Closed Bug 1158424 Opened 6 years ago Closed 6 years ago

Clean up FrameMetrics::mIsRoot and related notions

Categories

(Core :: Panning and Zooming, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox40 --- affected
firefox41 --- fixed

People

(Reporter: botond, Assigned: botond)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(9 files)

39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
kats
: review+
Details
39 bytes, text/x-review-board-request
kats
: review+
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
FrameMetrics currently has a boolean flag mIsRoot, described as being true if it represents the root scroll frame of the root content document (RCD-RSF). 

Having mIsRoot = true makes a FrameMetrics special in the following ways:

 (1) It's always created (and triggers the creation of an APZC), even 
     if the scroll frame it represents doesn't have a scroll range.
     This is to ensure that the entire content area is covered by an 
     APZC for event handling purposes.

 (2) Repaint requests for it go to APZCCallbackHelper::UpdateRootFrame
     rather than APZCCallbackHelper::UpdateSubFrame. UpdateRootFrame
     does additional processing needed for zooming.

     So mIsRoot is potentially zoomable. (APZ will only actually zoom
     it if the meta-viewport tag allows it.)

 (3) Scrolling it causes 'mozbrowserasyncscroll' events to be fired.

 (4) Wheel events sent to it are affected by the
     "mousewheel.system_scroll_override_on_root_content.enabled"
     pref.

This was conceptually sound at one point, but is starting not to be:

  - We now use APZ for both content *and* chrome, so having an APZC
    cover the entire content area isn't enough - we need the chrome
    area covered as well.

    We are currently dealing with this by creating another special
    APZC for the root document of the parent process (or the only
    process, for non-e10s platforms), to cover the chrome area.
    This root document is not a content document (it's either a
    XUL document or a chrome HTML document), so this APZC isn't
    mIsRoot, but it serves purpose (1) just like mIsRoot serves
    purpose (1) for content processes.

  - On Fennec, the root content document doesn't necessarily get
    an APZC, so we may not have an APZC with mIsRoot = true.

I propose cleaning this up by splitting mIsRoot into two flags: 

  - mIsProcessRoot would serve purpose (1), and be true for the
    root APZC in every process. Every process must have such a
    root APZC, even if its root scroll frame doesn't have a
    scroll range (or if it doesn't have a root scroll frame, as
    is the case for processes with XUL root documents).

  - mIsRootContent would serve purposes (2), (3), and (4). It
    would be set for the APZC corresponding to the root scroll
    frame of the root content document *if* such APZC already
    exists (either because the root scroll frame has a scroll
    range, or it's also the process-root). Having no APZC with
    mIsRootContent is OK.

Note that some places in APZCTreeManager (for example, GetMultiTouchTarget()) use IsRootForLayersId() (which effectively means mIsProcessRoot, as each process has its own layers id) when they really mean mIsRootContent. These will have to be updated.
(In reply to Botond Ballo [:botond] from comment #0)
> Having mIsRoot = true makes a FrameMetrics special in the following ways:

Oh, I forgot an important one: the composition bounds are calculated differently for it. I believe this behaviour belongs with mIsRootContent as well.
Thinking some more about this:

  - No one needs to check a FrameMetrics for mIsProcessRoot,
    so we don't need to store that flag.

  - The existing usage of mIsRoot basically corresponds to
    mIsRootContent, so that's basically just a rename.

The only actual changes that I think are involved are:

  - Any Layout code that special-cases the RCD-RSF for the
    purpose of making sure it gets FrameMetrics, should
    instead special-case the root scroll frame of the root 
    document of the process (or use the document element if 
    there is no root scroll frame).

    Note that special-casing the RCD-RSF for other purposes,
    such as computing the composition bounds differently,
    continues to be fine.

  - Any APZ code that uses IsRootForLayersId() when it means
    mIsRootContent should be changed accordingly.
Whiteboard: [gfx-noted]
Duplicate of this bug: 1108574
Kevin, did you mean to make bug 1108574 a duplicate of a different bug? I don't see why it would be a duplicate of this one.
Flags: needinfo?(kbrosnan)
Ah, I see you've already fixed it :)
Flags: needinfo?(kbrosnan)
I realized, during a discussion with Timothy where this bug came up, that "process root" is an inaccurate term. For example, with e10s, multiple tabs can share a single process, but presumably each tab would have its own "process-root" APZC. Perhaps "LayerManager root" might be more accurate?
Assignee: nobody → botond
Bug 1158424 - Rename FrameMetrics::mIsRoot to mIsRootContent. r=kats
Bug 1158424 - Remove FrameMetrics::IsRootScrollable() (it just duplicated IsRootContent()). r=kats
The WIP patches so far rename FrameMetrics::mIsRoot to mIsRootContent, and fold IsRootScrollable() into IsRootContent() since they return the same thing.

The remaining work in this bug is:

  - Clean up the uses of APZCTreeManager::IsRootForLayersId(apzc).
    I'm going to defer this part until after bug 1168630 lands, because
    what we choose to do there may be relevant to some of the uses.

  - Split out an mIsZoomable flag from mIsRootContent for readability
    (and ultimately anticipating subframe zooming).
(In reply to Botond Ballo [:botond] from comment #9)
>   - Clean up the uses of APZCTreeManager::IsRootForLayersId(apzc).
>     I'm going to defer this part until after bug 1168630 lands, because
>     what we choose to do there may be relevant to some of the uses.

The patches that are now posted in bug 1168630 make it clear what needs to be done in this bug. Marking as dependency.
Depends on: 1168630
(In reply to Botond Ballo [:botond] from comment #9)
>   - Clean up the uses of APZCTreeManager::IsRootForLayersId(apzc).
>     I'm going to defer this part until after bug 1168630 lands, because
>     what we choose to do there may be relevant to some of the uses.

Bug 1168630 renamed IsRootForLayersId() to HasNoParentWithSameLayersId().

Here's my analysis of its callers:

  - When logging APZ test data, we mark APZCs with this flag as "roots" so
    the APZC tree reconstruction code in the APZ tests, which reconstructs a
    subtree corresponding to a layers id, knows not to expect them to have a
    parent.

    I think this use is appropriate and can stand.

  - When assigning zoom constraints to APZCs, we use this flag to decide
    whether to give the APZC the "root zoom constraints", or whether to
    have the APZC inherit zoom constraints from its parent.

    The root zoom constraints (GeckoCC::GetRootZoomConstraints()) applies
    to the root-content APZC. My proposal is:

      - If the APZC is the root-content, give it the root zoom constraints.

      - Otherwise if it has a parent, give it the parent's constraints.

      - Otherwise, leave it with the default constraints.

  - When setting zoom constraints in UpdateZoomConstraints, we use this flag 
    to check whether we should apply them recursively. Here we should be
    checking for root-content instead.

      - We also use it in UpdateZoomConstraintsRecursively() to detect a subtree 
        with a different layers id to skip it. That use should stand as it 
        concerns the tree structure.

  - In BuildOverscrollHandoffChain():

      - We issue a warning if an APZC without this flag fails to have a 
        scroll-parent-id. Here I think we should be checking for
        IsLayersIdRoot() instead, as Layout should have enough info
        to set scroll-parent-ids for other APZC. In particular, APZCs
        with HasNoParentWithSameLayersId() = true but IsLayersIdRoot() = false
        (such as APZCs inside toplevel fixed-pos layers) should still have
        scroll-parent-ids.

      - In an optimization that attempts to finds the APZC that matches the
        scroll-parent-id quickly by walking up the layer, we use this flag
        to determine where to stop. This use should stand as it concerns the
        tree structure.

  - The final use is in RootAPZCForLayersId(), called by GetMultitouchTarget().
    I believe GetMultitouchTarget() is going to need to be rewritten altogether.

    Consider a page with a root APZC R, and a fixed-pos layer F with a scrollable
    subframe S. Suppose the user has one finger over R and one finger over S,
    and tries to pinch.

    With containerless-root, F and R will be siblings in the layer tree, so 
    S and R will be siblings in the APZC tree. If we call CommonAncestor() on
    S and R, that's going to return an APZC in the *parent* layer tree (if any).

    I propose doing the following instead:

       - If the two APZCs have the same layers id, return FindRootAPZCForLayersId()
         for that layers id.

       - Otherwise, call CommonAncestor() and return FindRootAPZCForLayersId() for
         the layers id of the result.

Finally, since some calls to HasNoParentWithSameLayersId() should stand, after changing the callers that should change I will remove the deprecation notice that was added to this function in bug 1168630.

Any feedback is welcome.
(In reply to Botond Ballo [:botond] from comment #11)
>   - In BuildOverscrollHandoffChain():
> 
>       - We issue a warning if an APZC without this flag fails to have a 
>         scroll-parent-id. Here I think we should be checking for
>         IsLayersIdRoot() instead, as Layout should have enough info
>         to set scroll-parent-ids for other APZC. In particular, APZCs
>         with HasNoParentWithSameLayersId() = true but IsLayersIdRoot() =
> false
>         (such as APZCs inside toplevel fixed-pos layers) should still have
>         scroll-parent-ids.

For APZC's inside fixed pos elements it's not technically correct to set their scroll parent as the root scroll frame, since the root scroll frame doesn't scroll fixed pos elements. But that is what we currently do. It might be something we want to change at some point though. Presumably we can just change this warning if we do that?
(In reply to Timothy Nikkel (:tn) from comment #12)
> For APZC's inside fixed pos elements it's not technically correct to set
> their scroll parent as the root scroll frame, since the root scroll frame
> doesn't scroll fixed pos elements. But that is what we currently do. It
> might be something we want to change at some point though. Presumably we can
> just change this warning if we do that?

Good point. As discussed briefly on IRC, we'll have the warning match what Layout does for now, and we can change it when needed.
(In reply to Botond Ballo [:botond] from comment #11)
>     I propose doing the following instead:
> 
>        - If the two APZCs have the same layers id, return FindRootAPZCForLayersId()
>          for that layers id.
> 
>        - Otherwise, call CommonAncestor() and return FindRootAPZCForLayersId() for
>          the layers id of the result.

Erm, that's wrong - we want to be zooming the root-content APZC, not the root-for-layers-id one.
Comment on attachment 8611558 [details]
MozReview Request: Bug 1158424 - Rename FrameMetrics::mIsRoot to mIsRootContent. r=kats

Bug 1158424 - Rename FrameMetrics::mIsRoot to mIsRootContent. r=kats
Attachment #8611558 - Flags: review?(bugmail.mozilla)
Comment on attachment 8611559 [details]
MozReview Request: Bug 1158424 - Remove FrameMetrics::IsRootScrollable() (it just duplicated IsRootContent()). r=kats

Bug 1158424 - Remove FrameMetrics::IsRootScrollable() (it just duplicated IsRootContent()). r=kats
Attachment #8611559 - Flags: review?(bugmail.mozilla)
Bug 1158424 - Expose IsRootContent() in AsyncPanZoomController. r=kats
Attachment #8613778 - Flags: review?(bugmail.mozilla)
Bug 1158424 - Clean up uses of HasNoParentWithSameLayersId() related to zoom constraints. r=kats
Attachment #8613779 - Flags: review?(bugmail.mozilla)
Bug 1158424 - Clean up uses of HasNoParentWithSameLayersId() in overscroll handoff chain building. r=kats
Attachment #8613780 - Flags: review?(bugmail.mozilla)
Bug 1158424 - Extract a BreadthFirstSearch() helper function. r=kats
Attachment #8613781 - Flags: review?(bugmail.mozilla)
Bug 1158424 - Add APZCTreeManager::FindRootContentApzcForLayersId(). r=kats
Attachment #8613782 - Flags: review?(bugmail.mozilla)
Bug 1158424 - Fix APZCTreeManager::GetMultitouchTarget(). r=kats
Attachment #8613783 - Flags: review?(bugmail.mozilla)
Bug 1158424 - Undeprecate HasNoParentWithSameLayersId(). r=kats
Attachment #8613784 - Flags: review?(bugmail.mozilla)
Updated patch series to implement what I described in comment 11.
(In reply to Botond Ballo [:botond] from comment #9)
>   - Split out an mIsZoomable flag from mIsRootContent for readability
>     (and ultimately anticipating subframe zooming).

I'm wondering how mIsZoomazble fits with how I implemented GetMultiTouchTarget() (finding a common layers id, and then getting the root-content APZC for that layers id). Does it make sense to find the first APZC (in breadth-first search) that has mIsZoomable = true?
Comment on attachment 8611558 [details]
MozReview Request: Bug 1158424 - Rename FrameMetrics::mIsRoot to mIsRootContent. r=kats

https://reviewboard.mozilla.org/r/9515/#review8667

Ship It!
Attachment #8611558 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8611559 [details]
MozReview Request: Bug 1158424 - Remove FrameMetrics::IsRootScrollable() (it just duplicated IsRootContent()). r=kats

https://reviewboard.mozilla.org/r/9517/#review8669

Ship It!
Attachment #8611559 - Flags: review?(bugmail.mozilla) → review+
Attachment #8613778 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8613778 [details]
MozReview Request: Bug 1158424 - Expose IsRootContent() in AsyncPanZoomController. r=kats

https://reviewboard.mozilla.org/r/9795/#review8673

Ship It!
(In reply to Botond Ballo [:botond] from comment #11)
>   - When assigning zoom constraints to APZCs, we use this flag to decide
>     whether to give the APZC the "root zoom constraints", or whether to
>     have the APZC inherit zoom constraints from its parent.
> 
>     The root zoom constraints (GeckoCC::GetRootZoomConstraints()) applies
>     to the root-content APZC. My proposal is:
> 
>       - If the APZC is the root-content, give it the root zoom constraints.
> 
>       - Otherwise if it has a parent, give it the parent's constraints.
> 
>       - Otherwise, leave it with the default constraints.

I'm not sure I agree with this. I think what we want (and what the original code tried to accomplish) was that all the APZCs with a given layers ID have the same zoom constraints. In the context of this change this would map to putting the GeckoCC::GetRootZoomConstraints on any APZC for which HasNoParentWithSameLayersId is true, and then for all the others, copying down the parent's constraints. In particular I don't think that leaving something with the default constraints is the right thing to do. That being said, I'm also re-thinking how the zoom constraints should be propagated, because of the trouble the current implementation is causing over in bug 1055557. So for now we can go with what you have and then I might change it up as part of bug 1055557.

>   - The final use is in RootAPZCForLayersId(), called by
> GetMultitouchTarget().
>     I believe GetMultitouchTarget() is going to need to be rewritten
> altogether.
> 
>     Consider a page with a root APZC R, and a fixed-pos layer F with a
> scrollable
>     subframe S. Suppose the user has one finger over R and one finger over S,
>     and tries to pinch.
> 
>     With containerless-root, F and R will be siblings in the layer tree, so 
>     S and R will be siblings in the APZC tree. If we call CommonAncestor() on
>     S and R, that's going to return an APZC in the *parent* layer tree (if
> any).

In this layer tree, if we apply an async zoom on R, will the fixed-pos layer F also get visually bigger? I might be missing something but since it's outside the subtree rooted at F the async transform won't apply to F. (I expect it will get larger on repaint since the resolutionAndScale will apply to all layers in the presShell).
Comment on attachment 8613779 [details]
MozReview Request: Bug 1158424 - Clean up uses of HasNoParentWithSameLayersId() related to zoom constraints. r=kats

https://reviewboard.mozilla.org/r/9797/#review8675

::: gfx/layers/apz/src/APZCTreeManager.cpp:474
(Diff revision 1)
> -      } else {
> +      } else  if (!apzc->HasNoParentWithSameLayersId()) {

nit: spacing in |else  if|

::: gfx/layers/apz/src/APZCTreeManager.cpp:477
(Diff revision 1)
> -        // user-scalable=no was specified, none of the APZCs allow double-tap
> +        // user-scalable=no was specified on thr oot, none of the APZCs allow

s/thr oot/the root/
Attachment #8613779 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8613780 [details]
MozReview Request: Bug 1158424 - Clean up uses of HasNoParentWithSameLayersId() in overscroll handoff chain building. r=kats

https://reviewboard.mozilla.org/r/9799/#review8677

Ship It!
Attachment #8613780 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8613781 [details]
MozReview Request: Bug 1158424 - Extract a BreadthFirstSearch() helper function. r=kats

https://reviewboard.mozilla.org/r/9801/#review8679

::: gfx/layers/apz/src/APZCTreeManager.cpp
(Diff revision 1)
> -}

The diff is showing a deleted brace at the end of the file (presumably the one corresponding to "namespace layers"). Compile error? Fix as needed.

::: gfx/layers/apz/src/APZCTreeManager.cpp:1463
(Diff revision 1)
> +template <typename Node, typename Condition>

Haven't looked at the rest of the patches but making "Node" a template parameter seems like an unnecessary generalization. I assume the only thing it ever is going to be is a HitTestingTreeNode. I'd rather not make that a template parameter if we can avoid it.
Attachment #8613781 - Flags: review?(bugmail.mozilla) → review+
Attachment #8613782 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8613782 [details]
MozReview Request: Bug 1158424 - Add APZCTreeManager::FindRootContentApzcForLayersId(). r=kats

https://reviewboard.mozilla.org/r/9803/#review8681

Ship It!
Comment on attachment 8613783 [details]
MozReview Request: Bug 1158424 - Fix APZCTreeManager::GetMultitouchTarget(). r=kats

https://reviewboard.mozilla.org/r/9805/#review8687

Ship It!
Attachment #8613783 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8613784 [details]
MozReview Request: Bug 1158424 - Undeprecate HasNoParentWithSameLayersId(). r=kats

https://reviewboard.mozilla.org/r/9807/#review8689

Thanks, splitting up the patches made it much easier to review than I thought it would be! :)
Attachment #8613784 - Flags: review?(bugmail.mozilla) → review+
(In reply to Botond Ballo [:botond] from comment #25)
> I'm wondering how mIsZoomazble fits with how I implemented
> GetMultiTouchTarget() (finding a common layers id, and then getting the
> root-content APZC for that layers id). Does it make sense to find the first
> APZC (in breadth-first search) that has mIsZoomable = true?

Hm, that's a good question. Maybe we should hold off on introducing the mIsZoomable flag for now. It was a little speculative anyway, in anticipation of eventually supporting subframe zooming but I don't know when (or if) that will happen, so it's better to not add that flag now than to add it and possibly get it wrong.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #29)
> >     Consider a page with a root APZC R, and a fixed-pos layer F with a
> > scrollable
> >     subframe S. Suppose the user has one finger over R and one finger over S,
> >     and tries to pinch.
> > 
> >     With containerless-root, F and R will be siblings in the layer tree, so 
> >     S and R will be siblings in the APZC tree. If we call CommonAncestor() on
> >     S and R, that's going to return an APZC in the *parent* layer tree (if
> > any).
> 
> In this layer tree, if we apply an async zoom on R, will the fixed-pos layer
> F also get visually bigger? I might be missing something but since it's
> outside the subtree rooted at F the async transform won't apply to F. (I
> expect it will get larger on repaint since the resolutionAndScale will apply
> to all layers in the presShell).

Hmm, good point! This may be an issue with containerless-root.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #32)
> ::: gfx/layers/apz/src/APZCTreeManager.cpp:1463
> (Diff revision 1)
> > +template <typename Node, typename Condition>
> 
> Haven't looked at the rest of the patches but making "Node" a template
> parameter seems like an unnecessary generalization. I assume the only thing
> it ever is going to be is a HitTestingTreeNode. I'd rather not make that a
> template parameter if we can avoid it.

Given that the function only assumes that the tree is navigable via GetLastChild() / GetPrevSibling() calls, it can be used for any tree supporting that interface, such as the layer tree. I was thinking of eventually moving the function into a header where it can be used by multiple files.
(In reply to Botond Ballo [:botond] from comment #38)
> Given that the function only assumes that the tree is navigable via
> GetLastChild() / GetPrevSibling() calls, it can be used for any tree
> supporting that interface, such as the layer tree. I was thinking of
> eventually moving the function into a header where it can be used by
> multiple files.

Ok, fair enough.
Renaming bug to reflect what the patches currently do a bit more accurately.
Summary: Split FrameMetrics::mIsRoot into mIsProcessRoot and mIsRootContent → Clean up FrameMetrics::mIsRoot and related notions
The "Extract a BreadthFirstSearch() helper function" patch ran afoul of the static analysis added in bug 1153304, but that's a bug in the static analysis.

I filed bug 1170388 and have a fix there; rather than work around the bug I'll just wait for that to land.
Depends on: 1170388
(In reply to Botond Ballo [:botond] from comment #38)
> Given that the function only assumes that the tree is navigable via
> GetLastChild() / GetPrevSibling() calls, it can be used for any tree
> supporting that interface, such as the layer tree. I was thinking of
> eventually moving the function into a header where it can be used by
> multiple files.

Filed bug 1171312 for this.
Try push that includes these patches, and the fix for bug 1170388:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ff606ee785a
(In reply to Botond Ballo [:botond] from comment #43)
> Try push that includes these patches, and the fix for bug 1170388:
> 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ff606ee785a

Hmm, the static analysis is still firing for these patches, even though it passes locally with clang 3.5, and the tests I added that were supposed to cover this scenario pass in infra with clang 3.3.

Clearly, the tests that I wrote fail to exercise a case that these patches do...
(In reply to Botond Ballo [:botond] from comment #44)
> (In reply to Botond Ballo [:botond] from comment #43)
> > Try push that includes these patches, and the fix for bug 1170388:
> > 
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ff606ee785a
> 
> Hmm, the static analysis is still firing for these patches, even though it
> passes locally with clang 3.5, and the tests I added that were supposed to
> cover this scenario pass in infra with clang 3.3.
> 
> Clearly, the tests that I wrote fail to exercise a case that these patches
> do...

I've been trying to get clang 3.3 to run locally to repro the problem, but had no success so far because clang 3.3 chokes on gcc 4.9's libstdc++ headers, and I can't seem to be able to get the prebuilt package to look at older libstdc++ headers. I'll try building it from source so I can bake in the older header paths.
(In reply to Botond Ballo [:botond] from comment #45)
> (In reply to Botond Ballo [:botond] from comment #44)
> > (In reply to Botond Ballo [:botond] from comment #43)
> > > Try push that includes these patches, and the fix for bug 1170388:
> > > 
> > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ff606ee785a
> > 
> > Hmm, the static analysis is still firing for these patches, even though it
> > passes locally with clang 3.5, and the tests I added that were supposed to
> > cover this scenario pass in infra with clang 3.3.
> > 
> > Clearly, the tests that I wrote fail to exercise a case that these patches
> > do...
> 
> I've been trying to get clang 3.3 to run locally to repro the problem, but
> had no success so far because clang 3.3 chokes on gcc 4.9's libstdc++
> headers, and I can't seem to be able to get the prebuilt package to look at
> older libstdc++ headers. I'll try building it from source so I can bake in
> the older header paths.

I was able to configure clang 3.3 to use gcc 4.8's libstdc++, but it's ICE'ing on me when trying to build.

At this point I don't want to spend any more time on this, so I'll just revise my patches to not use lambdas to avoid tripping the static analysis.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #32)
> Comment on attachment 8613781 [details]
> MozReview Request: Bug 1158424 - Extract a BreadthFirstSearch() helper
> function. r=kats
> 
> https://reviewboard.mozilla.org/r/9801/#review8679
> 
> ::: gfx/layers/apz/src/APZCTreeManager.cpp
> (Diff revision 1)
> > -}
> 
> The diff is showing a deleted brace at the end of the file (presumably the
> one corresponding to "namespace layers"). Compile error? Fix as needed.

This was a patch-splitting error; a later patch in the series deleted the last function in the file without deleting its closing brace, balancing things out :) Fixed locally.
Comment on attachment 8611558 [details]
MozReview Request: Bug 1158424 - Rename FrameMetrics::mIsRoot to mIsRootContent. r=kats

Bug 1158424 - Rename FrameMetrics::mIsRoot to mIsRootContent. r=kats
Attachment #8611558 - Flags: review+
Comment on attachment 8611559 [details]
MozReview Request: Bug 1158424 - Remove FrameMetrics::IsRootScrollable() (it just duplicated IsRootContent()). r=kats

Bug 1158424 - Remove FrameMetrics::IsRootScrollable() (it just duplicated IsRootContent()). r=kats
Attachment #8611559 - Flags: review+
Attachment #8613778 - Flags: review+
Comment on attachment 8613778 [details]
MozReview Request: Bug 1158424 - Expose IsRootContent() in AsyncPanZoomController. r=kats

Bug 1158424 - Expose IsRootContent() in AsyncPanZoomController. r=kats
Comment on attachment 8613779 [details]
MozReview Request: Bug 1158424 - Clean up uses of HasNoParentWithSameLayersId() related to zoom constraints. r=kats

Bug 1158424 - Clean up uses of HasNoParentWithSameLayersId() related to zoom constraints. r=kats
Attachment #8613779 - Flags: review+
Comment on attachment 8613780 [details]
MozReview Request: Bug 1158424 - Clean up uses of HasNoParentWithSameLayersId() in overscroll handoff chain building. r=kats

Bug 1158424 - Clean up uses of HasNoParentWithSameLayersId() in overscroll handoff chain building. r=kats
Attachment #8613780 - Flags: review+
Attachment #8613781 - Flags: review+ → review?(bugmail.mozilla)
Comment on attachment 8613781 [details]
MozReview Request: Bug 1158424 - Extract a BreadthFirstSearch() helper function. r=kats

Bug 1158424 - Extract a BreadthFirstSearch() helper function. r=kats
Attachment #8613782 - Flags: review+ → review?(bugmail.mozilla)
Comment on attachment 8613782 [details]
MozReview Request: Bug 1158424 - Add APZCTreeManager::FindRootContentApzcForLayersId(). r=kats

Bug 1158424 - Add APZCTreeManager::FindRootContentApzcForLayersId(). r=kats
Comment on attachment 8613783 [details]
MozReview Request: Bug 1158424 - Fix APZCTreeManager::GetMultitouchTarget(). r=kats

Bug 1158424 - Fix APZCTreeManager::GetMultitouchTarget(). r=kats
Attachment #8613783 - Flags: review+
Comment on attachment 8613784 [details]
MozReview Request: Bug 1158424 - Undeprecate HasNoParentWithSameLayersId(). r=kats

Bug 1158424 - Undeprecate HasNoParentWithSameLayersId(). r=kats
Attachment #8613784 - Flags: review+
Reworked the Part 6 and Part 7 patches to use non-lambdas. Re-requesting review for them, carrying r+ for the rest.
Comment on attachment 8613781 [details]
MozReview Request: Bug 1158424 - Extract a BreadthFirstSearch() helper function. r=kats

https://reviewboard.mozilla.org/r/9801/#review9435

Oh, C++ - a bajillion different ways to do anything!
Attachment #8613781 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8613782 [details]
MozReview Request: Bug 1158424 - Add APZCTreeManager::FindRootContentApzcForLayersId(). r=kats

https://reviewboard.mozilla.org/r/9803/#review9437
Attachment #8613782 - Flags: review?(bugmail.mozilla) → review+
You need to log in before you can comment on or make changes to this bug.