Switch geckoview.xhtml to geckoview.html
Categories
(GeckoView :: General, task, P3)
Tracking
(Not tracked)
People
(Reporter: bgrins, Unassigned)
References
Details
Attachments
(2 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
4.07 KB,
patch
|
Details | Diff | Splinter Review |
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=.
Updated•6 years ago
|
Reporter | ||
Updated•6 years ago
|
Reporter | ||
Comment 1•6 years ago
|
||
Reporter | ||
Comment 2•6 years ago
|
||
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•6 years ago
•
|
||
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•6 years ago
|
||
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•6 years ago
|
||
This patch should fix the assertion.
Reporter | ||
Comment 6•6 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #5)
Created attachment 9093746 [details] [diff] [review]
Fix for AsyncCompositionManager assertionThis patch should fix the assertion.
Thank you! I will give this a try.
Reporter | ||
Updated•6 years ago
|
Reporter | ||
Updated•5 years ago
|
Comment 7•3 years ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Updated•3 years ago
|
Description
•