According to the discussion in bug 1020801 comment 14, we fire two sets of TransformBegin/TransformEnd notifications from APZ in the case where we pan into overscroll and do a snapback. We should avoid doing this since conceptually it is a single user operation and there should be a single TransformBegin/TransformEnd that encompasses the whole thing.

This probably happens when we switch from the fling to the snapback animation. As the fling animation terminates, it schedules the snabpack as a deferred task and returns false for the continueAnimation return value. This will result in the state getting to NOTHING temporarily at [1] before it is set to SNAP_BACK at [2]. This state glitch is likely what results in the TransformEnd/TransformBegin notifications. [1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?rev=70b5978c6fb6#2124 [2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?rev=70b5978c6fb6#1807

A possible approach is to place Transform[Begin|End] notifications into a "queue" of size 1 (with a newer one replacing an older one), and flush the "queue" at the end of conceptual operations. Looking at the code, flushing them at the end of the following functions should be sufficient: SampleContentTransformForFrame HandleInputEvent CancelAnimation Destroy ZoomToRect Kats, does this approach sound reasonable?

While reviewing this part of your patch on bug 1039992: + // Note: + // This needs to be a deferred task even though it can safely run + // while holding mMonitor, because otherwise, if the overscrolled APZC + // is this one, then the SetState(NOTHING) in UpdateAnimation will + // stomp on the SetState(SNAP_BACK) it does. + mDeferredTasks.append(NewRunnableMethod(mOverscrollHandoffChain.get(), + &OverscrollHandoffChain::SnapBackOverscrolledApzc)); return false; I was thinking it be simpler to just do what you said in the note (i.e. run while holding mMonitor instead of posting a deferred task) and then return true instead of returning false. The return true will indicate that there's still an animation going, and we won't flip into state NONE. Would that work?

Bug 1046159 contains a similar case of going to the NOTHING state and then back into the transforming state.

I believe your latest patches for flinging will also introduce a NOTHING state after the pan but before the fling kicks in. (Because of how we need to find the scrollgrabbing APZC to start the fling).

Try push https://tbpl.mozilla.org/?tree=Try&rev=d797b92aebb0

Comment on attachment 8488290 [details] [diff] [review] Part 1 - Introduce the state change notification blocker Review of attachment 8488290 [details] [diff] [review]: ----------------------------------------------------------------- Nice approach :) More concise than what I suggested in comment 2.

Try push has some build failures. These I'm definitely blaming on nested enum class thingy :p

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11) > Try push has some build failures. These I'm definitely blaming on nested > enum class thingy :p I'm not opposed to changing APZStateChange to be a regular enum if that makes things easier.

Made the change to a regular enum in bug 1066421. New try push is clean: https://tbpl.mozilla.org/?tree=Try&rev=6768aa6af709 Landed: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/6f015a84b5e6 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/e0d05b3f7f2c remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/e208feba9fd1

https://hg.mozilla.org/mozilla-central/rev/6f015a84b5e6 https://hg.mozilla.org/mozilla-central/rev/e0d05b3f7f2c https://hg.mozilla.org/mozilla-central/rev/e208feba9fd1