1049109 - Warning about handing off overscroll across a layer tree boundary is firing

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Botond Ballo [:botond]]

In bug 1031067, we added a warning when scroll is handed off across a layer tree boundary [1], saying that we may need to revise our approach to that bug.

With the rocketbar now in place, that warning is firing, which makes sense since we hand off scroll between the scrollgrabbing div in the parent process and the browser iframe in the child process. Accordingly, we need to revise our approach for bug 1031067.

A possible approach is to replace the calls to ClearOverscroll() at [2] with something like ClearOverscrollForHandoffChain().

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/APZCTreeManager.cpp?rev=cf0c9fbd6cb2#850
[2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?rev=217c103a4b75#1325

Comment 1

[Botond Ballo [:botond]]

Attachment: bug1049109.patch

This patch adds OverscrollHandoffChain::ClearOverscroll() which clears overscroll along the entire chain, and replaces the call to APZC::ClearOverscroll() in OnScaleEnd() with OverscrollHandoffChain::ClearOverscroll().

Comment 2

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

Comment on attachment 8476883 [details] [diff] [review]
bug1049109.patch

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

::: gfx/layers/apz/src/OverscrollHandoffChain.h
@@ +105,5 @@
>    bool CanBePanned(const AsyncPanZoomController* aApzc) const;
>  private:
>    std::vector<nsRefPtr<AsyncPanZoomController>> mChain;
> +
> +  typedef void (AsyncPanZoomController::*APZCMethod)();

Does this generate a compile-time error if the arg to ForEachApzc is not in AsyncPanZoomController? That's amazing! Little dark corners of C++...

Comment 3

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)
> ::: gfx/layers/apz/src/OverscrollHandoffChain.h
> @@ +105,5 @@
> >    bool CanBePanned(const AsyncPanZoomController* aApzc) const;
> >  private:
> >    std::vector<nsRefPtr<AsyncPanZoomController>> mChain;
> > +
> > +  typedef void (AsyncPanZoomController::*APZCMethod)();
> 
> Does this generate a compile-time error if the arg to ForEachApzc is not in
> AsyncPanZoomController? That's amazing! Little dark corners of C++...

It does indeed. (What kind of compiler would let you call a function that's not a method of AsyncPanZoomController, on an AsyncPanZoomController instance!? :))

Comment 4

[Botond Ballo [:botond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/36a60b15c104

Comment 5

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

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/a808b7283dbc for gtest failures: 
https://tbpl.mozilla.org/php/getParsedLog.php?id=46505096&tree=Mozilla-Inbound

Comment 6

[Botond Ballo [:botond]]

(In reply to Wes Kocher (:KWierso) from comment #5)
> Backed out in
> https://hg.mozilla.org/integration/mozilla-inbound/rev/a808b7283dbc for
> gtest failures: 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=46505096&tree=Mozilla-
> Inbound

Sorry! I should have run gtests locally before pushing. (Bug 932562 would help a lot in this regard.)

Comment 7

[Botond Ballo [:botond]]

Fixed tests and relanded:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f7068f94021

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/1f7068f94021