Closed Bug 1049109 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: Panning and Zooming, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: botond, Assigned: botond)

References

Details

Attachments

(1 file)

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
Attached patch bug1049109.patchSplinter Review
This patch adds OverscrollHandoffChain::ClearOverscroll() which clears overscroll along the entire chain, and replaces the call to APZC::ClearOverscroll() in OnScaleEnd() with OverscrollHandoffChain::ClearOverscroll().
Assignee: nobody → botond
Attachment #8476883 - Flags: review?(bugmail.mozilla)
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++...
Attachment #8476883 - Flags: review?(bugmail.mozilla) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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!? :))
(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.)
Flags: needinfo?(botond)
https://hg.mozilla.org/mozilla-central/rev/1f7068f94021
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.