Based on bug 1020801 comment 19, create this bug to add scroll start/stop notification from apz and then pass these events to scroll observers.

This patch tries to add scroll start/stop notification from apz. And these notification will be used to show/hide the selection bubble from bug 1020801

Comment on attachment 8474428 [details] [diff] [review] WIP Review of attachment 8474428 [details] [diff] [review]: ----------------------------------------------------------------- ::: docshell/base/nsIScrollObserver.h @@ +21,5 @@ > * Called when the scroll position of some element has changed. > */ > virtual void ScrollPositionChanged() = 0; > + virtual void ScrollPositionChangeStarted(){}; > + virtual void ScrollPositionChangeStopped(){}; Have empty implementation here because not every scroll observer want to listen scrollStart/Stop events. Or it requires to declare all functions as pure virtual function.

Note that the APZStateChange::TransformBegin|End notifications are fired any time there is a change to the transform. This doesn't necessarily mean that the scroll position is changing; it might be the zoom that is changing while the scroll position stays the same. So it might be better to rename your functions accordingly.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #3) > Note that the APZStateChange::TransformBegin|End notifications are fired any > time there is a change to the transform. This doesn't necessarily mean that > the scroll position is changing; it might be the zoom that is changing while > the scroll position stays the same. So it might be better to rename your > functions accordingly. Thanks for the note. I also noticed that APZStateChange::TransformEnd is received but no scroll position changed events for scroll snapback case. How about the ScrollViewChangeStarted/ScrollViewChangeStopped?

Comment on attachment 8474428 [details] [diff] [review] WIP Review of attachment 8474428 [details] [diff] [review]: ----------------------------------------------------------------- Will work on the v2 to rename ScrollPositionChangeStarted/Stopped

That sounds ok to me, but I'm not a layout peer so I'm not the one to make a decision on this.

Rename funcation to ScrollViewChangeStart/Stop based on comment 3. roc, this patch is trying to add the notification of scroll view changes(transform changes) from APZ and dispatch to scroll oberservers. The selection bubble in bug 1020801 will use the new ScrollViewChangeStart/Stop callback to show/hide itself.

Attached to correct patch. Rename funcation to ScrollViewChangeStart/Stop based on comment 3. roc, this patch is trying to add the notification of scroll view changes(transform changes) from APZ and dispatch to scroll oberservers. The selection bubble in bug 1020801 will use the new ScrollViewChangeStart/Stop callback to show/hide itself.

Comment on attachment 8474955 [details] [diff] [review] WIP v2 Review of attachment 8474955 [details] [diff] [review]: ----------------------------------------------------------------- ::: docshell/base/nsIScrollObserver.h @@ +24,5 @@ > + > + /** > + * Called when the scroll view starts to change, i.e. like the transform > + */ > + virtual void ScrollViewChangeStarted(){}; This isn't really clear enough. Let's call the functions AsyncPanZoomStarted/Stopped and the comment should say "Called when an async panning/zooming transform has started being applied." "Called when an async panning/zooming transform is no longer applied."

Address review's comment. Wait for try server result. https://tbpl.mozilla.org/?tree=Try&rev=6aa9caceae7e

The updated patch didn't rename the functions as roc requested: (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9) > Let's call the functions AsyncPanZoomStarted/Stopped and ...

Comment on attachment 8476420 [details] [diff] [review] WIP v3 Review of attachment 8476420 [details] [diff] [review]: ----------------------------------------------------------------- ::: docshell/base/nsIScrollObserver.h @@ +29,5 @@ > + > + /** > + * Called when an async panning/zooming transform is no longer applied. > + */ > + virtual void ScrollViewChangeStopped(){}; Note that you need to change NS_ISCROLLOBSERVER_IID because of the functions you've added to this interface.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11) > The updated patch didn't rename the functions as roc requested: > > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9) > > Let's call the functions AsyncPanZoomStarted/Stopped and ... (In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #12) > Comment on attachment 8476420 [details] [diff] [review] > WIP v3 > > Review of attachment 8476420 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: docshell/base/nsIScrollObserver.h > @@ +29,5 @@ > > + > > + /** > > + * Called when an async panning/zooming transform is no longer applied. > > + */ > > + virtual void ScrollViewChangeStopped(){}; > > Note that you need to change NS_ISCROLLOBSERVER_IID because of the functions > you've added to this interface. Thanks for the note, I will re-upload patch again.

Change the uuid and update func name to AsyncPanZoomStarted/Stopped.

(In reply to peter chang[:pchang][:peter] from comment #10) > Created attachment 8476420 [details] [diff] [review] > WIP v3 > > Address review's comment. > > Wait for try server result. > https://tbpl.mozilla.org/?tree=Try&rev=6aa9caceae7e Previous try is all green. And he difference between v3 and v4 are the uuid and function name changes. It won't impact the testing result.