Make browser_test_shentry_wireframe.js work with parent controlled loads
Categories
(Core :: DOM: Navigation, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox104 | --- | fixed |
People
(Reporter: mccr8, Assigned: mccr8)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
This test fails with browser.tabs.documentchannel.parent-controlled set to true. The feature seems to include some interaction with SHEntry stuff, so it is possible that this is an actual issue with the feature, and not just an issue with the test (some tests have to be fixed to wait for different events when run with parent controlled loads).
Running with
./mach mochitest --headless --setpref browser.tabs.documentchannel.parent-controlled=true browser_test_shentry_wireframe.js
results in these failures:
docshell/test/navigation/browser_test_shentry_wireframe.js
FAIL A wireframe was captured for the first entry. - null == true - JS frame :: chrome://mochitests/content/browser/docshell/test/navigation/browser_test_shentry_wireframe.js :: <TOP_LEVEL> :: line 34
Stack trace:
chrome://mochitests/content/browser/docshell/test/navigation/browser_test_shentry_wireframe.js:null:34
UNEXPECTED-PASS No wireframe for the loaded entry. -
Stack trace:
chrome://mochitests/content/browser/docshell/test/navigation/browser_test_shentry_wireframe.js:null:52
FAIL A wireframe was captured for the first entry. - null == true - JS frame :: chrome://mochitests/content/browser/docshell/test/navigation/browser_test_shentry_wireframe.js :: <TOP_LEVEL> :: line 63
Stack trace:
chrome://mochitests/content/browser/docshell/test/navigation/browser_test_shentry_wireframe.js:null:63
FAIL A wireframe was captured for the first entry. - null == true - JS frame :: chrome://mochitests/content/browser/docshell/test/navigation/browser_test_shentry_wireframe.js :: <TOP_LEVEL> :: line 79
Stack trace:
chrome://mochitests/content/browser/docshell/test/navigation/browser_test_shentry_wireframe.js:null:79
Assignee | ||
Comment 1•2 years ago
|
||
It looks like PersistLayoutHistoryState() (which is where CollectWireframe is called) is called from nsDocShell::DoURILoad when the pref is false, but not when it is true.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 2•2 years ago
|
||
Saving wireframes happens in the same method as saving scroll positions, so maybe this is related to bug 1743636. I'll have to read over Anny's comments there.
Assignee | ||
Comment 3•2 years ago
|
||
Yeah, it sounds like the issue is very similar to what Anny described in bug 1743636 comment 4: the test loads one page from the parent, then a second from the parent, and then somehow it doesn't save the layout history stuff, which includes both the scroll position (the other test) and the wireframe.
Assignee | ||
Comment 4•2 years ago
|
||
I managed to get at least a prototype of something that at least passes 1 test that wasn't passing before. At some point, somebody pointed me at bug 1766131, where Peter added SynchronizeLayoutHistoryState(). This saves the current session history entry, requests some layout state from the child, then saves that layout data into the entry. This avoids the issue of the wireframe data being sent up from the child not having an active entry.
After far too much time spent wrestling with C++ compilation errors, I managed to a wireframe to the data that is returned. Once you have it along with the prior active entry, it is easy to save it. Then, I added a call to SynchronizeLayoutHistoryState() into CanonicalBrowsingContext::ReplacedBy(), which is where the active entry is changed in this case.
Assignee | ||
Comment 5•2 years ago
|
||
That also seems to fix browser_scrollPositions.js, so that's nice.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 6•2 years ago
|
||
Here's my WIP patch for this issue: https://phabricator.services.mozilla.com/differential/diff/578590/
While it does make part of the test pass, it doesn't fix everything.
Assignee | ||
Comment 7•2 years ago
|
||
Assignee | ||
Comment 8•2 years ago
|
||
The updated version of the patch passes all subtests, with and without parent controlled navigation. It even passes the two "todo" tests. I still need to go through and see if the existing CollectWireframe is still necessary, as it feels like it might be redundant.
Assignee | ||
Comment 9•2 years ago
|
||
Updated•2 years ago
|
Comment 11•2 years ago
|
||
Comment 12•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8d966ca8f0bd
https://hg.mozilla.org/mozilla-central/rev/61c86361ac7c
Description
•