Mir

Code review comment for lp:~robertcarr/mir/rebuild-input-targeting

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Overall, this looks like a nice simplification but there's a lot to it and (due to the poor test coverage of the 3rd_party input code) I don't feel confident that nothing breaks as a result.

It isn't easy to get the code under test (especially without reworking it - which is what this MP does), so I can't insist on "more tests".

Anyway, there's a bit of tidy up needed...

226 + synthesizeCancelationEventsForInputChannelLocked(
227 + focusedInputChannel, options);

Strange indentation

~~~~

2845 + virtual ~InputRegistrar() noexcept(true) {}

2978 + virtual ~InputTargetEnumerator() noexcept(true) {}

These don't gain by being inline

~~~~

2853 +protected:
2854 + InputRegistrar(const InputRegistrar&) = delete;
2855 + InputRegistrar& operator=(const InputRegistrar&) = delete;

2982 +protected:
2983 + InputTargetEnumerator(const InputTargetEnumerator&) = delete;
2984 + InputTargetEnumerator& operator=(const InputTargetEnumerator&) = delete;

No need for a protected section for these - and aren't they deleted in the base anyway?

~~~~~

2858 + droidinput::sp<droidinput::InputDispatcherInterface> input_dispatcher;

should be const

review: Needs Fixing

« Back to merge proposal