Make window handles of top-level browsing contexts unique (no change for browsing context swaps)
Categories
(Remote Protocol :: Marionette, enhancement, P2)
Tracking
(Fission Milestone:M7a, firefox90 fixed)
Tracking | Status | |
---|---|---|
firefox90 | --- | fixed |
People
(Reporter: whimboo, Assigned: jdescottes)
References
(Blocks 1 open bug)
Details
(Whiteboard: [bidi-m1-mvp], [wptsync upstream])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
With the fix on bug 1674329 we update the handle of the currently selected window. But if other windows are open in the background, and a navigation gets triggered in one of those, we will miss that the top-level browsing context might get changed.
Due to those top-level browsing context swaps the window handle will change, and as such tests won't be able to identify the formerly window anymore.
The only constant value is the browsingContext.browserId
, which references the actual tab. If we cannot use the browserId
directly, maybe it would be good to have a new mapping for browserId => unique id
, which can be updated whenever a top-level browsing context changes.
Reporter | ||
Comment 2•4 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [βοΈUTC+1] from comment #0)
The only constant value is the
browsingContext.browserId
, which references the actual tab. If we cannot use thebrowserId
directly, maybe it would be good to have a new mapping forbrowserId => unique id
, which can be updated whenever a top-level browsing context changes.
With what we just have seen on bug 1682062 browserId
is also not a feasible replacement. While it is unique and that is what we want, it depends on a valid content browser being attached to the tab. But as long as a tab is unloaded there is no such content browser, and the browserId stays at 0
for all those tabs.
We will have to check how to best identify tabs after bug 1669172 has been landed. Maybe we have to map permanentKey
to a uuid to really have stable and unique entries.
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Comment 3•3 years ago
|
||
While working on bug 1704697 to get BFCache working with Fission in Marionette I noticed a couple of failures that are related to a missing unique identifier for tabs. Loading web pages can cause a process switch and as such the top-level browsing context is swapped / replaced. After that switching windows will fail for cached window handles because the handle is no longer valid. With BFCache + Fission this scenario is now much more likely.
Here a Wdspec test result:
https://treeherder.mozilla.org/logviewer?job_id=338030756&repo=try&lineNumber=9289-9345
[task 2021-04-28T13:11:44.267Z] 13:11:44 INFO - PID 2959 | 1619615504265 Marionette DEBUG 0 -> [0,19,"WebDriver:SwitchToWindow",{"handle":"41","name":"41"}]
[..]
[task 2021-04-28T13:11:44.348Z] 13:11:44 INFO - PID 2959 | 1619615504345 Marionette DEBUG 0 -> [0,20,"WebDriver:Navigate",{"url":"http://web-platform.test:8000/webdriver/tests/support/inline.py?doc=%3C%21doctype+html%3E%0A%3Cmeta+charset%3DUTF-8%3E%0A%3Cp+id%3D%27b%27%3Eother&mime=text%2Fhtml&charset=UTF-8"}]
[task 2021-04-28T13:11:44.356Z] 13:11:44 INFO - PID 2959 | 1619615504354 Marionette TRACE Received event beforeunload for about:blank
[task 2021-04-28T13:11:44.395Z] 13:11:44 INFO - PID 2959 | 1619615504392 Marionette TRACE Remoteness change detected. Set new top-level browsing context to 45
[task 2021-04-28T13:11:44.408Z] 13:11:44 INFO - PID 2959 | 1619615504405 Marionette TRACE Received event pagehide for about:blank
[task 2021-04-28T13:11:44.418Z] 13:11:44 INFO - PID 2959 | 1619615504410 Marionette TRACE [45] MarionetteEvents actor created for window id 34
[task 2021-04-28T13:11:44.423Z] 13:11:44 INFO - PID 2959 | 1619615504421 Marionette TRACE Received event beforeunload for about:blank
[task 2021-04-28T13:11:44.487Z] 13:11:44 INFO - PID 2959 | 1619615504485 Marionette TRACE Received event pagehide for about:blank
[task 2021-04-28T13:11:44.504Z] 13:11:44 INFO - PID 2959 | 1619615504502 Marionette TRACE [45] MarionetteEvents actor created for window id 6442450946
[task 2021-04-28T13:11:44.553Z] 13:11:44 INFO - PID 2959 | 1619615504550 Marionette TRACE Received event DOMContentLoaded for http://web-platform.test:8000/webdriver/tests/support/inline.py?doc=%3C%21doctype+html%3E%0A%3Cmeta+charset%3DUTF-8%3E%0A%3Cp+id%3D%27b%27%3Eother&mime=text%2Fhtml&charset=UTF-8
[task 2021-04-28T13:11:44.556Z] 13:11:44 INFO - PID 2959 | 1619615504551 Marionette TRACE Received event pageshow for http://web-platform.test:8000/webdriver/tests/support/inline.py?doc=%3C%21doctype+html%3E%0A%3Cmeta+charset%3DUTF-8%3E%0A%3Cp+id%3D%27b%27%3Eother&mime=text%2Fhtml&charset=UTF-8
[task 2021-04-28T13:11:44.567Z] 13:11:44 INFO - PID 2959 | 1619615504564 Marionette DEBUG 0 <- [1,20,null,{"value":null}]
[..]
[task 2021-04-28T13:11:44.655Z] 13:11:44 INFO - PID 2959 | 1619615504653 Marionette DEBUG 0 -> [0,22,"WebDriver:SwitchToWindow",{"handle":"39","name":"39"}]
[task 2021-04-28T13:11:44.686Z] 13:11:44 INFO - PID 2959 | 1619615504684 Marionette TRACE Received DOM event TabSelect for [object XULElement]
[task 2021-04-28T13:11:44.709Z] 13:11:44 INFO - PID 2959 | 1619615504706 Marionette DEBUG 0 <- [1,22,null,{"value":null}]
[..]
[task 2021-04-28T13:11:44.846Z] 13:11:44 INFO - PID 2959 | 1619615504840 Marionette DEBUG 0 -> [0,25,"WebDriver:SwitchToWindow",{"handle":"41","name":"41"}]
[task 2021-04-28T13:11:44.848Z] 13:11:44 INFO - PID 2959 | 1619615504843 Marionette DEBUG 0 <- [1,25,{"error":"no such window","message":"Unable to locate window: 41","stacktrace":"WebDriverError@chrome://marionette/cont ... t@chrome://marionette/content/server.js:238:9\n_onJSONObjectReady/<@chrome://marionette/content/transport.js:504:20\n"},null]
Julian, would you be interested to work on this bug or should I take it?
Reporter | ||
Comment 4•3 years ago
|
||
Required preferences to enable BFCache and Fission are fission.autostart=true
and fission.bfcacheInParent=true
.
Assignee | ||
Comment 5•3 years ago
|
||
I will take the bug.
Quick note: I can reproduce the issue with
./mach wpt /webdriver/tests/close_window/close.py --setpref fission.autostart=true --setpref fission.bfcacheInParent=true
But not with
./mach wpt /webdriver/tests/close_window/close.py --enable-fission --setpref fission.bfcacheInParent=true
Somehow --enable-fission doesn't seem to have the same effect as --setpref fission.autostart=true ?
Assignee | ||
Updated•3 years ago
|
Reporter | ||
Comment 6•3 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #5)
Somehow --enable-fission doesn't seem to have the same effect as --setpref fission.autostart=true ?
Yes, that is bug 1680963 that James is working on to get fixed soon.
Btw. I briefly spoke with Olli and maybe we can indeed use the browserId
for the unique window handle of tabs. Clients would have to make sure to really use chrome window handles separately, given that there can be identical ids between them.
Assignee | ||
Comment 7•3 years ago
|
||
Updated•3 years ago
|
Assignee | ||
Comment 8•3 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [βοΈUTC+1] from comment #6)
Btw. I briefly spoke with Olli and maybe we can indeed use the
browserId
for the unique window handle of tabs. Clients would have to make sure to really use chrome window handles separately, given that there can be identical ids between them.
As discussed on chat.mozilla.org, I use uuids for now, because of what was mentioned in comment 2:
(In reply to Henrik Skupin (:whimboo) [βοΈUTC+1] from comment #2)
With what we just have seen on bug 1682062
browserId
is also not a feasible replacement. While it is unique and that is what we want, it depends on a valid content browser being attached to the tab. But as long as a tab is unloaded there is no such content browser, and the browserId stays at0
for all those tabs.
Comment 9•3 years ago
|
||
Tracking this bug for Fission M7a because this bug blocks Fission bfcache (meta bug 1671510).
Comment 10•3 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c15da7dafcab [marionette] Use stable unique ids as window handles r=marionette-reviewers,webdriver-reviewers,whimboo
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29016 for changes under testing/web-platform/tests
Comment 12•3 years ago
|
||
bugherder |
Reporter | ||
Comment 13•3 years ago
|
||
(In reply to Web Platform Test Sync Bot (Matrix: #interop:mozilla.org) from comment #11)
Created web-platform-tests PR
https://github.com/web-platform-tests/wpt/pull/29016 for changes under
testing/web-platform/tests
James, could you please merge? Thanks.
Upstream PR merged by moz-wptsync-bot
Upstream PR merged by moz-wptsync-bot
Reporter | ||
Updated•3 years ago
|
Updated•2 years ago
|
Description
•