Remove the explicit remoteness switch that happens during the restore process
Categories
(Firefox :: Session Restore, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox91 | --- | fixed |
People
(Reporter: u608768, Assigned: annyG)
References
(Blocks 3 open bugs)
Details
Attachments
(2 files, 3 obsolete files)
For restores that require remoteness changes, we'll currently call SessionStore._sendRestoreHistory(), then switch the browser's remoteness (with updateBrowserRemotenessByURL()), and then call _sendRestoreHistory() again on the post-remoteness-flip browser.
This causes complications for SSTabRestoring timing (bug 1702055 required a few hacks to fix these). We should remove the first restoreHistory() call and the updateBrowserRemotenessByURL() call.
Fission:M8 but this is mostly just clean-up, so can be pushed further if need be.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
Assignee | ||
Comment 2•3 years ago
|
||
There has been several issues I have uncovered so far that stem from not executing the explicit remoteness switch in addition to changing the SetCurrentURI
call within SessionStoreUtils::RestoreDocShellState
to pass false
for aFireOnLocationChange
argument.
-
In the case when we open a new tab, scroll on it, duplicate the tab, and then force reload, the scroll position wasn't being reset. I tracked it down to two flush calls, where the first flush call was supposed to reset the session store, but because it's epoch value was not considered recent (the epoch has been deleted from sessionstore's set), we ignored the update. Instead, the flush call from
checkScroll
function went through with the scroll values before the force reload. This was happening because when we reset the epoch upon XULFrameLoaderCreated event, we didn't update it in other places, namely we didn't update the session store listener's value. A simple call torequestEpochUpdate
fixed this problem. -
When the
aFireOnLocationChange
argument isfalse
, then we get the wrong value here. When I check the session history entry's uri, it is about:rights, as expected. So this means that thecurrentURI
of the browser doesn't get updated becauseupdateForLocationChange
doesn't get called, which eventually can be traced down to the nsDocShell not sending the onLocationChange event. However, if I setaFireOnLocationChange
to true, then I have a test like browser/components/sessionstore/test/browser_restore_container_tabs_oa.js failing (as well as some others) with a bunch of assertions. We end up firingXULFrameLoaderCreated
event more often than we want, sometimes we get the remote type wrong ("web" instead of the "webIsolated=https://example.com"). This leads me to believe that settingaFireOnLocationChange
totrue
just fires off the event too early.
This situation makes bug 1709132 more pertinent - the solution here might be just to remove SetCurrentURI call as kashav pointed out in the comments there.
Kashav and I talked about this, he came up with preliminary patches for removing SetCurrentURI, and figured that the patches probably need to land at the same time as patches for this current bug. Sounds good. -
And there is the ever persistent bug with the http redirect. [Here] we keep getting the pre-redirect url. The current SH entry also shows the wrong url. I am currently debugging this.
Updated•3 years ago
|
Assignee | ||
Comment 3•3 years ago
|
||
Depends on D116209
Assignee | ||
Comment 4•3 years ago
|
||
Depends on D117238
Assignee | ||
Comment 5•3 years ago
|
||
Depends on D117239
Assignee | ||
Comment 6•3 years ago
|
||
Depends on D117240
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Comment 8•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7289a07292f6
https://hg.mozilla.org/mozilla-central/rev/90b17c352a43
Description
•