Closed Bug 1691348 Opened 3 years ago Closed 3 years ago

Browsing context has been discarded after "WebDriver:SwitchToFrame"

Categories

(Remote Protocol :: Marionette, defect, P2)

Product:
Remote Protocol
Component:
Marionette
Version:
Firefox 85
Type:
defect

Tracking

(firefox-esr78 unaffected, firefox85 wontfix, firefox86 wontfix, firefox87 fixed)

Status:
RESOLVED FIXED
Milestone:
87 Branch
Tracking Flags:
Tracking Status
firefox-esr78 --- unaffected
firefox85 --- wontfix
firefox86 --- wontfix
firefox87 --- fixed

People

(Reporter: leszek.skorczynski+mozilla, Assigned: whimboo)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Bug 1691348 - [marionette] Don't raise NoSuchWindow error when frame's content hasn't been finished loading after "WebDriver:SwitchToFrame".
48 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-release-
Details | Review
Bug 1691348 - [wdspec] Added test for accessing elements in slow loading frame.
48 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-release-
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:85.0) Gecko/20100101 Firefox/85.0

Steps to reproduce:

When running the following test with geckodriver it fails with the "error":"no such window","message":"Browsing context has been discarded" message:

describe('Upload', () => {
    it('Test', () => {
        browser.execute(function setBody(): void {
            document.documentElement.innerHTML = '<html><body><iframe src="https://mozilla.com/"></iframe></body></html>';
        });
        browser.switchToFrame($('iframe'));
        // browser.pause(300); // if this is uncommented then everything works
        $('#content').click(); // otherwise it fails here
    });
});

Reproduction repository:
https://github.com/RafalSkorka/bug1677975

error log:

1612508368692	webdriver::server	DEBUG	-> POST /session/0145e224-841a-40ad-a87e-bb0f5236d4ec/execute/sync {"script":"return (function setBody() {\n            document.documentElement.innerHTML = '<html><body><iframe src=\"https://mozilla.com/\"></iframe></body></html>';\n        }).apply(null, arguments)","args":[]}
1612508368694	Marionette	DEBUG	0 -> [0,2,"WebDriver:ExecuteScript",{"args":[],"script":"return (function setBody() {\n            document.documentElement.innerHTML = '<html><body><iframe src=\"https://mozilla.com/\"></iframe></body></html>';\n        }).apply(null, arguments)"}]
1612508368695	Marionette	TRACE	[15] MarionetteCommands actor created for window id 2147483649
1612508368699	Marionette	DEBUG	0 <- [1,2,null,{"value":null}]
1612508368692	webdriver::server	DEBUG	<- 200 OK {"value":null}
1612508368692	webdriver::server	DEBUG	-> POST /session/0145e224-841a-40ad-a87e-bb0f5236d4ec/element {"using":"css selector","value":"iframe"}
1612508368704	Marionette	DEBUG	0 -> [0,3,"WebDriver:FindElement",{"using":"css selector","value":"iframe"}]
1612508368708	Marionette	DEBUG	0 <- [1,3,null,{"value":{"element-6066-11e4-a52e-4f735466cecf":"09020d9b-7e6a-4f59-b3f4-f5524b4149ef"}}]
1612508368708	webdriver::server	DEBUG	<- 200 OK {"value":{"element-6066-11e4-a52e-4f735466cecf":"09020d9b-7e6a-4f59-b3f4-f5524b4149ef"}}
1612508368708	webdriver::server	DEBUG	-> POST /session/0145e224-841a-40ad-a87e-bb0f5236d4ec/frame {"id":{"sessionId":"0145e224-841a-40ad-a87e-bb0f5236d4ec","elementId":"09020d9b-7e6a-4f59-b3f4-f5524b4149ef","element-6066-11e4-a52e-4f735466cecf":"09020d9b-7e6a-4f59-b3f4-f5524b4149ef","selector":"iframe","parent":{"sessionId":"0145e224-841a-40ad-a87e-bb0f5236d4ec","capabilities":{"acceptInsecureCerts":false,"browserName":"firefox","browserVersion":"85.0","moz:accessibilityChecks":false,"moz:buildID":"20210118153634","moz:geckodriverVersion":"0.29.0","moz:headless":true,"moz:processID":15896,"moz:profile":"C:\\PATH\\rust_mozprofileUfLd2y","moz:shutdownTimeout":60000,"moz:useNonSpecCompliantPointerOrigin":false,"moz:webdriverClick":true,"pageLoadStrategy":"normal","platformName":"windows","platformVersion":"10.0","rotatable":false,"setWindowRect":true,"strictFileInteractability":false,"timeouts":{"implicit":0,"pageLoad":300000,"script":30000},"unhandledPromptBehavior":"dismiss and notify"},"config":{"specs":["./test/specs/**/*.js"],"suites":{},"exclude":[],"outputDir":".","logLevel":"trace","logLevels":{},"excludeDriverLogs":[],"bail":0,"waitforInterval":500,"waitforTimeout":5000,"framework":"mocha","reporters":[],"services":[],"maxInstances":100,"maxInstancesPerCapability":100,"filesToWatch":[],"connectionRetryTimeout":120000,"connectionRetryCount":3,"execArgv":[],"runnerEnv":{},"runner":"local","mochaOpts":{"timeout":60000,"ui":"bdd"},"jasmineNodeOpts":{"defaultTimeoutInterval":10000},"cucumberOpts":{"timeout":10000},"onPrepare":[],"onWorkerStart":[],"before":[],"beforeSession":[],"beforeSuite":[],"beforeHook":[],"beforeTest":[],"beforeCommand":[],"afterCommand":[],"afterTest":[],"afterHook":[],"afterSuite":[],"afterSession":[],"after":[],"onComplete":[],"onReload":[],"beforeFeature":[],"beforeScenario":[],"beforeStep":[],"afterStep":[],"afterScenario":[],"afterFeature":[],"_":["wdio.conf.js"],"$0":"node_modules\\@wdio\\cli\\bin\\wdio.js","ignoredWorkerServices":[],"specFileRetryAttempts":0},"_NOT_FIBER":true},"isReactElement":false}}
1612508368716	Marionette	DEBUG	0 -> [0,4,"WebDriver:SwitchToFrame",{"element":"09020d9b-7e6a-4f59-b3f4-f5524b4149ef"}]
1612508368717	Marionette	DEBUG	0 <- [1,4,null,{"value":null}]
1612508368708	webdriver::server	DEBUG	<- 200 OK {"value":null}
1612508368708	webdriver::server	DEBUG	-> POST /session/0145e224-841a-40ad-a87e-bb0f5236d4ec/element {"using":"css selector","value":"#content"}
1612508368720	Marionette	DEBUG	0 -> [0,5,"WebDriver:FindElement",{"using":"css selector","value":"#content"}]
1612508368720	Marionette	DEBUG	0 <- [1,5,{"error":"no such window","message":"Browsing context has been discarded","stacktrace":"WebDriverError@chrome://marionet ... t@chrome://marionette/content/server.js:241:9\n_onJSONObjectReady/<@chrome://marionette/content/transport.js:504:20\n"},null]
1612508368708	webdriver::server	DEBUG	<- 404 Not Found {"value":{"error":"no such window","message":"Browsing context has been discarded","stacktrace":"WebDriverError@chrome://marionette/content/error.js:181:5\nNoSuchWindowError@chrome://marionette/content/error.js:415:5\nassert.that/<@chrome://marionette/content/assert.js:460:13\nassert.open@chrome://marionette/content/assert.js:168:4\nGeckoDriver.prototype.findElement@chrome://marionette/content/driver.js:2109:10\ndespatch@chrome://marionette/content/server.js:297:40\nexecute@chrome://marionette/content/server.js:267:16\nonPacket/<@chrome://marionette/content/server.js:240:20\nonPacket@chrome://marionette/content/server.js:241:9\n_onJSONObjectReady/<@chrome://marionette/content/transport.js:504:20\n"}}

Actual results:

When driver tries to find element it gets the error message.

Expected results:

no error is expected. it worked in previous version of Firefox (84).

Thanks for filing the bug. Maybe this could be related to bug 1676665.

Would you mind trying the following two Nightly builds of Firefox?

Without the patch: https://archive.mozilla.org/pub/firefox/nightly/2020/11/2020-11-17-09-44-06-mozilla-central/
With the patch: https://archive.mozilla.org/pub/firefox/nightly/2020/11/2020-11-17-21-55-29-mozilla-central/

Does the one with the patch fail for you while the other one passes fine?

Flags: needinfo?(leszek.skorczynski+mozilla)
Summary: Browsing context has been discarded after SwitchToFrame → Browsing context has been discarded after "WebDriver:SwitchToFrame"

Also does the same happen when you are using a URL as frame source that loads faster like a data URL?

Status: UNCONFIRMED → NEW
status-firefox85: --- → wontfix
status-firefox86: --- → affected
status-firefox87: --- → affected
Ever confirmed: true

I confirm the version without the patch works fine and the one with the patch does not work. I haven't checked the data URL, but when the "Browsing context has been discarded" error is raised Firefox apparently does not load the src yet. I can tell it because when it works properly it fails on not being able to embed Mozilla's https url inside such crafted html (which is expected).

Flags: needinfo?(leszek.skorczynski+mozilla)

Thanks! When I find time later today I will see if I can reproduce it some way. It's interesting that we never hit this problem since then.

Keywords: regression
Regressed by: 1676665

The problem is the specific WebDriver command (Execute Script), which is used here to load the web page including the iframe. Running the code will indeed synchronously update the DOM, but it doesn't take care of the iframe's content to be finished or even started loaded. And that is by design. Only NavigateTo, Forward, Back, Refresh, and Element Click have an implicit wait for the page to be finished loading.

But actually I can see a difference in the type of error as reported between the old architecture of Marionette and the new actor based one. When running this test with the preference marionette.actors.enabled=false set the failure as reported is that #content cannot be found.

I will have a more closer look tomorrow.

Assignee: nobody → hskupin
Blocks: 1669172
Status: NEW → ASSIGNED
Component: geckodriver → Marionette
Priority: -- → P2

The changeset that is causing the different error is: https://hg.mozilla.org/mozilla-central/rev/311ceb127ade

The test case as provided looks like the following for Marionette:

    def test_usercase(self):
        self.marionette.execute_script("""
          document.documentElement.innerHTML = '<html><body><iframe src=\"https://mozilla.com/\"></iframe></body></html>';
        """)
        iframe = self.marionette.find_element(By.TAG_NAME, "iframe")
        self.marionette.switch_to_frame(iframe)
        self.marionette.find_element(By.ID, "content")

Note that https://mozilla.com denies to be loaded inside a frame because X-Frame-Options is set to DENY. But changing the iframe source to eg. https://example.org doesn't make a difference, but at least makes the browsing context being available earlier.

Running the page load through Navigate To works as expected:

1612859498307	Marionette	DEBUG	3 -> [0,2,"WebDriver:Navigate",{"url":"data:text/html;charset=utf-8,%3Ciframe%20src%3D%27https%3A//mozilla.com/%27%3E%3C/iframe%3E"}]
1612859498315	Marionette	TRACE	[10] MarionetteCommands actor created for window id 2147483649
1612859498318	Marionette	TRACE	[10] MarionetteEvents actor created for window id 2147483649
1612859498318	Marionette	TRACE	Received event beforeunload for about:blank
1612859498323	Marionette	TRACE	Received event pagehide for about:blank
1612859498325	Marionette	TRACE	[10] MarionetteEvents actor created for window id 2147483650
1612859498371	Marionette	TRACE	Received event DOMContentLoaded for data:text/html;charset=utf-8,%3Ciframe%20src%3D%27https%3A//mozilla.com/%27%3E%3C/iframe%3E
1612859498543	Marionette	TRACE	[19] MarionetteEvents actor created for window id 12
1612859498547	Marionette	TRACE	[20] MarionetteEvents actor created for window id 14
1612859498549	Marionette	TRACE	[21] MarionetteEvents actor created for window id 16
1612859498551	Marionette	TRACE	[22] MarionetteEvents actor created for window id 18
1612859498605	Marionette	TRACE	[19] MarionetteEvents actor created for window id 4294967297
1612859498612	Marionette	TRACE	[20] MarionetteEvents actor created for window id 4294967298
1612859498642	Marionette	TRACE	[22] MarionetteEvents actor created for window id 4294967300
1612859498648	Marionette	TRACE	[21] MarionetteEvents actor created for window id 4294967299
1612859499556	Marionette	TRACE	[2147483649] MarionetteEvents actor created for window id 2147483652
1612859499556	Marionette	WARN	Ignoring event 'pageshow' because document has an invalid readyState of 'uninitialized'.
1612859499638	Marionette	TRACE	[2147483649] MarionetteEvents actor created for window id 2147483653
1612859499643	Marionette	TRACE	Received event pageshow for data:text/html;charset=utf-8,%3Ciframe%20src%3D%27https%3A//mozilla.com/%27%3E%3C/iframe%3E
1612859499644	Marionette	DEBUG	3 <- [1,2,null,{"value":null}]

Note that for the frame we get an invalid pageshow event because the document's readyState is uninitialized. That is causing a reload of the frame's content. Waiting this extra time is not done when running the code through Execute Script.

Interestingly I can get rid of the problem when simply accessing browsingContext.window in the child Actor right before returning from Switch To Frame:

diff --git a/testing/marionette/actors/MarionetteCommandsChild.jsm b/testing/marionette/actors/MarionetteCommandsChild.jsm
--- a/testing/marionette/actors/MarionetteCommandsChild.jsm
+++ b/testing/marionette/actors/MarionetteCommandsChild.jsm
@@ -520,6 +520,8 @@ class MarionetteCommandsChild extends JS
       browsingContext = context;
     }

+    browsingContext.window;
+
     return { browsingContextId: browsingContext.id };
   }

That actually maps to what we did previously in the framescript code:
https://hg.mozilla.org/mozilla-central/rev/311ceb127ade#l2.76

It feels to me that the currentWindowGlobal is not correctly getting created, without accessing the window in the child actor first.

Nika, could you please help us to understand this behavior? Is it maybe a bug in the JSWindowActor or BrowsingContext implementation?

Flags: needinfo?(nika)

I will put up a patch that we could temporarily land until we got an answer from Nika, and maybe a possible platform bug fixed.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #6)

It feels to me that the currentWindowGlobal is not correctly getting created, without accessing the window in the child actor first.

When loading an in-process subframe, there is an old optimization which kicks in where we try to avoid creating the initial about:blank document and its associated window global in order to avoid unnecessary creation of resources. This initial window global and initial about:blank document are only created if some script, whether it be chrome or content, accesses the relevant properties of the frame element. If they aren't, the first global in the context ends up being the one loaded from the network. Because the window global doesn't exist yet (and may never exist if we never try to access it), we don't create the WindowGlobalParent actor either, and so you see a null currentWindowGlobal in the parent process.

Given that this is marionette-only code, the approach you have in that bug of accessing the property to force it to be created early is probably as good as we're going to get for now, as it basically disables the lazy window global optimization in that case. You might want to wrap it in a try { ... } catch though.

Flags: needinfo?(nika)

Talked to Nika on Elements and the try/catch should not be needed.

Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4fcf6d43457b
[marionette] Don't raise NoSuchWindow error when frame's content hasn't been finished loading after "WebDriver:SwitchToFrame". r=marionette-reviewers,jgraham
https://hg.mozilla.org/integration/autoland/rev/8209c454ea9b
[wdspec] Added test for accessing elements in slow loading frame. r=webdriver-reviewers,jgraham
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/27594 for changes under testing/web-platform/tests

https://hg.mozilla.org/mozilla-central/rev/4fcf6d43457b
https://hg.mozilla.org/mozilla-central/rev/8209c454ea9b

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox87: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
Upstream PR merged by moz-wptsync-bot

Comment on attachment 9202295 [details]
Bug 1691348 - [marionette] Don't raise NoSuchWindow error when frame's content hasn't been finished loading after "WebDriver:SwitchToFrame".

Beta/Release Uplift Approval Request

  • User impact if declined: Users of WebDriver can run into this failure when switching between frames that haven't been finished loading yet, and causing an unexpected error.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): An additional check for the browsing context's browerId has been added to check for equality of top-level browsing contexts.
  • String changes made/needed:
Attachment #9202295 - Flags: approval-mozilla-beta?
Attachment #9202593 - Flags: approval-mozilla-beta?
Blocks: 1677975

Why is the change risky/not risky? (and alternatives if risky): An additional check for the browsing context's browerId has been added to check for equality of top-level browsing contexts.

Sorry that was wrong. To prevent an internal optimization routine to kick in (see comment 9), manually accessing browsingContext.window will trigger the creation of the required window global for Marionette's browsing context checks.

We build RC later today, is that low risk enough for an RC build?

Flags: needinfo?(hskupin)

Yes, it's just adding a single line to access browsingContext.window here:
https://hg.mozilla.org/integration/autoland/rev/4fcf6d43457b#l1.15

All the other changes are just test related.

Flags: needinfo?(hskupin) → needinfo?(pascalc)
Flags: needinfo?(pascalc)
Attachment #9202593 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Attachment #9202295 - Flags: approval-mozilla-beta? → approval-mozilla-release?

Comment on attachment 9202593 [details]
Bug 1691348 - [wdspec] Added test for accessing elements in slow loading frame.

Too late for non-critical patches, sorry.

Attachment #9202593 - Flags: approval-mozilla-release? → approval-mozilla-release-
Attachment #9202295 - Flags: approval-mozilla-release? → approval-mozilla-release-

The patch landed in nightly and beta is affected.
:whimboo, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(hskupin)

As given by the denied approval request this will be a wontfix for 86.

status-firefox86: affected → wontfix
Flags: needinfo?(hskupin)
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: