Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Description • 10 years ago

Load the attached page in the B2G browser. Fling the page and then while it is in kinetic scroll, put your finger down.

Expected results:
- The page stops moving immediately when the finger goes down

Actual results:
- The page continues moving for 300ms before it stops

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 1 • 10 years ago

With this second case things are even worse. If you fling the top-level page and then put your finger down on a subframe the top-level page just keeps going. (See instructions at the top of the test page for precise STR).

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 5 • 10 years ago

Because we now want the overscroll handoff chain anytime we get a touchstart, we need it in ApzcTap, ApzcPinchWithTouchInput, ApzcDoubleTap, etc. I figured it was easier to just move it into the tester setup/teardown. If we don't have the handoff chain the NS_WARNING from part 2 gets printed.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 8 • 10 years ago

I think this is a cleaner solution to the stop-fast-fling problem (bug 1022956) since it doesn't reach backwards into the GEL to throw away an already-processed touchstart.

	Botond Ballo [:botond]
	Comment 10 • 10 years ago

Comment on attachment 8457868 [details] [diff] [review]
Part 2 - Fix the second test case

Review of attachment 8457868 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/apz/src/APZCTreeManager.cpp
@@ +905,5 @@
> +{
> +  MonitorAutoLock lock(mTreeLock);  // to access mOverscrollHandoffChain
> +  if (mOverscrollHandoffChain.length() == 0) {
> +    return false;
> +  }

We could omit this 'if' block and return 'mOverscrollHandoffChain.length() > 0' instead.

	Botond Ballo [:botond]
	Comment 11 • 10 years ago

Comment on attachment 8457943 [details] [diff] [review]
Part 5 - Move the stop-fast-fling code to deal with above changes

Review of attachment 8457943 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ +837,5 @@
>    TouchBlockState* block = nullptr;
>    if (aEvent.AsMultiTouchInput().mType == MultiTouchInput::MULTITOUCH_START) {
>      block = StartNewTouchBlock(false);
>      APZC_LOG("%p started new touch block %p\n", this, block);
> +    if (block == CurrentTouchBlock()) {

Please add a comment explaining this condition. (For reference, on IRC you said "so that future events don't interfere with animations from a previous block and cause some quantum wormhole" :-)).

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 12 • 10 years ago

(In reply to Botond Ballo [:botond] from comment #10)
> 
> We could omit this 'if' block and return 'mOverscrollHandoffChain.length() >
> 0' instead.

Good point. I'll do that, and update the corresponding flush-repaint code that I copied as well :)

(In reply to Botond Ballo [:botond] from comment #11)
> Please add a comment explaining this condition. (For reference, on IRC you
> said "so that future events don't interfere with animations from a previous
> block and cause some quantum wormhole" :-)).

Will do.

Also I did some try pushes with these patches:
https://tbpl.mozilla.org/?tree=Try&rev=4ad71eff0b94
https://tbpl.mozilla.org/?tree=Try&rev=3c760d184c0d

looks like we're getting a lot of infra errors right now but things that are running look green.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 13 • 10 years ago

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/71c26f7faa40
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/46964bbe3c3f
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/7d3bd5fccafd
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/086886096791
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/74cd20833f9d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/1709c5fdd182

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/71c26f7faa40
https://hg.mozilla.org/mozilla-central/rev/46964bbe3c3f
https://hg.mozilla.org/mozilla-central/rev/7d3bd5fccafd
https://hg.mozilla.org/mozilla-central/rev/086886096791
https://hg.mozilla.org/mozilla-central/rev/74cd20833f9d
https://hg.mozilla.org/mozilla-central/rev/1709c5fdd182