Open Bug 1545496 Opened 3 years ago Updated 2 years ago

Switch geckoview.xhtml to geckoview.html

Categories

(GeckoView :: General, task, P3)

Unspecified
Android

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: bgrins, Assigned: bdahl)

References

Details

Attachments

(2 files)

OS: All → Android
Priority: -- → P3
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED

Hi Botond, I'm seeing a foundRoot assertion failure (https://searchfox.org/mozilla-central/rev/4218cb868d8deed13e902718ba2595d85e12b86b/gfx/layers/composite/AsyncCompositionManager.cpp#1431) with the patch from this bug applied: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=267124571&repo=try&lineNumber=1634. I can reproduce this locally as well (it crashes at startup).

The exact change in this patch that causes the problem is switching from a <xul:window> to an <html:html> element for the root element in the geckoview document (https://searchfox.org/mozilla-central/source/mobile/android/chrome/geckoview/geckoview.xul). I'm wondering if you have any insight about why switching the document element would change the behavior at https://searchfox.org/mozilla-central/rev/4218cb868d8deed13e902718ba2595d85e12b86b/gfx/layers/composite/AsyncCompositionManager.cpp#1050.

Flags: needinfo?(botond)

The code that's responsible for making sure have root scrolling metadata (and therefore we don't trip that assertion) is generally nsLayoutUtils::GetRootMetadata(). If, for the page in question, we were getting here in that function, and with this patch we no longer are, that may indicate we need to update that function to account for this change, though it's not immediately clear how.

Flags: needinfo?(botond)

The relevant change here is that the XUL document didn't have a root scroll frame, but the HTML document does. Root scroll frames generate their own metadata, and as a result we don't get scroll metadata on the root layer.

I think that's fine from a correctness point of view, but the assertion in AsyncCompositionManager needs to be adjusted to expect that.

This patch should fix the assertion.

(In reply to Botond Ballo [:botond] from comment #5)

Created attachment 9093746 [details] [diff] [review]
Fix for AsyncCompositionManager assertion

This patch should fix the assertion.

Thank you! I will give this a try.

Blocks: 1579952
No longer blocks: 1540278
Depends on: 1603491
Assignee: bgrinstead → bdahl
No longer blocks: 1579952
Summary: Switch geckoview.xul to geckoview.html → Switch geckoview.xhtml to geckoview.html
You need to log in before you can comment on or make changes to this bug.