Closed Bug 1650868 Opened 4 years ago Closed 4 years ago

Fix APZ assertion about root content scroll metadata being inside async zoom containers

Categories

(Core :: Panning and Zooming, task)

task

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox80 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(2 files)

Attached file Layer tree dump (deleted) —

On the devtools/client/framework/test/browser_toolbox_toolbar_reorder_with_extension.js test run with apz.allow_zooming=true, the assertion here intermittently fails. (May need to run the test with --verify to repro locally).

The layer tree in question is attached. There are two RefLayers, and the async zoom container is in one while the root content the assertion is picking up is in the other one. The code needs updating to allow this scenario since it seems perfectly legitimate.

We've run into a similar assertion before in bug 1610657; I made a suggestion in bug 1610657 comment 4 for how this could be handled (basically, tweak the logic for the rootContent annotation so that it only appears on the layer which also has the AZC).

Hm. I don't think that approach would work in this case, since the two documents are siblings rather than nested. So there is no "outer" document on which to suppress the RCD flag. I just finished writing a patch which changes mUsingAsyncZoomContainer to actually be a Maybe<LayersId> so that we can track which subtree has the async zoom container, and then ignore RCD things that are in other subtrees. That seems to fix the problem.

I think in general there's nothing that restricts the browser to have a single top-level RCD. I think it's conceivable that the chrome process could e.g. host two browser tabs side-by-side and both would be RCDs. I guess this testcase probably does something similar, but at least here only one is zoomable. In the general case though there might be multiple sibling documents, all of which are zoomable. For bugs like this one where we need to make changes to this code I prefer to move in the direction of more generality (allow multiple RCDs and zoom containers) rather than forcing additional requirements on consumers.

Ah, interesting. I think additional work would likely be needed for zooming to work in the case where two side-by-side RCDs are zoomable, but I'm happy to move in the direction of supporting that, sure.

Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bbc213aa30a3 Make the async zoom containment checks subtree-specific. r=botond
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: