1545496 - Switch geckoview.xhtml to geckoview.html

Status: NEW

(GeckoView :: General, task, P3)

Description

[Brian Grinstead [:bgrins]]

I believe this file https://searchfox.org/mozilla-central/rev/ee3905439acbf81e9c829ece0b46d09d2fa26c5c/mobile/android/chrome/geckoview/geckoview.xul can be migrated to HTML as-is.

• The <browser> creation will need to be updated to use createXULElement: https://searchfox.org/mozilla-central/rev/ee3905439acbf81e9c829ece0b46d09d2fa26c5c/mobile/android/chrome/geckoview/geckoview.js#380
• A few hardcoded references to the path will need to be updated: https://searchfox.org/mozilla-central/search?q=geckoview.xul&path=.

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: Bug 1545496 - Migrate geckoview.xul to geckoview.html

Comment 2

[Brian Grinstead [:bgrins]]

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.

Comment 3

[Botond Ballo [: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.

Comment 4

[Botond Ballo [: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.

Comment 5

[Botond Ballo [:botond]]

Attachment: Fix for AsyncCompositionManager assertion

This patch should fix the assertion.

Comment 6

[Brian Grinstead [:bgrins]]

(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.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 8

[Chris Peterson [:cpeterson]]

Tasks should have severity N/A.