1374682 - Allow having non-cancelable blocks in InputQueue

Status: RESOLVED FIXED

(Core :: Panning and Zooming, enhancement, P3)

Description

[Kartikaya Gupta (email:[e-mail])]

The InputBlockState and subclasses were originally designed to allow two classes of blocks: cancelable and non-cancelable. Right now all the blocks in existence derive from CancelableBlockState because they are cancelable. However we would like to add a non-cancelable block for keyboard inputs. So we should make sure the code can properly handle adding a subclass of InputBlockState that is not a subclass of CancelableBlockState.

This basically means QueuedInput should store a InputBlockState* instead of a CancelableBlockState* and propagate those changes outwards. Some helper functions might also need to be modified.

I have a WIP locally that I need to clean up.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1374682 - Clean up processing of non-cancelable input blocks in InputQueue.

The main changes here are:
- Make QueuedInput store a InputBlockState instead of a CancelableBlockState,
  and propagate these changes outwards as necessary.
- Hoist some virtual functions from CancelableBlockState up into InputBlockState.
  These are mostly downcast-type functions but also a couple of others
  (DispatchEvent and MustStayActive) that are generally applicable to input
  blocks and not specific to block cancellation.
- Modify the signatures of a few other functions in InputQueue (such as CanDiscardBlock)
  to take a InputBlockState even though all the current call sites pass in a
  CancelableBlockState. This will make it easier to plumb in a new non-cancelable
  block because the function can still be used without modification.

Review commit: https://reviewboard.mozilla.org/r/150938/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150938/

Comment 2

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=93c7a59d631a6540e95afe2705aea9b5e2e9a2cd

Comment 3

[Botond Ballo [:botond]]

Comment on attachment 8879603 [details]
Bug 1374682 - Clean up processing of non-cancelable input blocks in InputQueue.

https://reviewboard.mozilla.org/r/150938/#review156314

::: gfx/layers/apz/src/InputBlockState.h:68
(Diff revision 1)
> +    return nullptr;
> +  }
> +  virtual DragBlockState* AsDragBlock() {
> +    return nullptr;
> +  }
> +  virtual PanGestureBlockState *AsPanGestureBlock() {

Fix positioning of * while you're at it.

::: gfx/layers/apz/src/InputBlockState.cpp:229
(Diff revision 1)
>    }
>    return mContentResponded || mContentResponseTimerExpired;
>  }
>  
>  void
> -CancelableBlockState::DispatchEvent(const InputData& aEvent) const
> +InputBlockState::DispatchEvent(const InputData& aEvent) const

Can we move it to be next to the other InputBlockState method definitions?

::: gfx/layers/apz/src/InputQueue.cpp:582
(Diff revision 1)
>    APZThreadUtils::AssertOnControllerThread();
>  
>    INPQ_LOG("got a content response; block=%" PRIu64 "\n", aInputBlockId);
>    bool success = false;
> -  CancelableBlockState* block = FindBlockForId(aInputBlockId, nullptr);
> -  if (block) {
> +  InputData* firstInput = nullptr;
> +  InputBlockState* inputBlock = FindBlockForId(aInputBlockId, &firstInput);

Why are we passing in |firstInput| if we don't use it?

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8879603 [details]
Bug 1374682 - Clean up processing of non-cancelable input blocks in InputQueue.

https://reviewboard.mozilla.org/r/150938/#review156314

> Fix positioning of * while you're at it.

Fixed

> Can we move it to be next to the other InputBlockState method definitions?

Done

> Why are we passing in |firstInput| if we don't use it?

Whoops, copy-paste error. Good catch! fixed also.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8879603 [details]
Bug 1374682 - Clean up processing of non-cancelable input blocks in InputQueue.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150938/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b6fe4dc69e3
Clean up processing of non-cancelable input blocks in InputQueue. r=botond

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4b6fe4dc69e3