Closed
Bug 1351940
Opened 7 years ago
Closed 6 years ago
tabbar.py TabBar.get_handle_for_tab can throw "TypeError: win.outerWindowID is undefined"
Categories
(Remote Protocol :: Marionette, defect, P2)
Tracking
(firefox-esr60 fixed, firefox64 fixed, firefox65 fixed)
RESOLVED
FIXED
mozilla65
People
(Reporter: Silne30, Assigned: whimboo)
References
Details
(Keywords: pi-marionette-firefox-puppeteer)
Attachments
(2 files, 2 obsolete files)
I performing a search via the location bar in puppeteer and it opens a new tab. When I try to close that tab, I get the above error. I attached my test script for your review.
Reporter | ||
Comment 1•7 years ago
|
||
Comment 2•7 years ago
|
||
This issue is blocking us from landing a telemetry test. Henrik, mind taking a look?
Flags: needinfo?(hskupin)
Assignee | ||
Comment 3•7 years ago
|
||
Can you also please attach the log? This is more helpful than the testcase actually. Thanks.
Flags: needinfo?(hskupin) → needinfo?(jdorlus)
Assignee | ||
Comment 4•7 years ago
|
||
Maybe this is related to bug 1352984, but I cannot say this without a log.
Reporter | ||
Comment 6•7 years ago
|
||
The gecko log did not have any additional information.
Assignee | ||
Comment 7•7 years ago
|
||
The stack looks a bit different than the other test failures we currently have but also fails in tabbar.py. Here specifically in `get_handle_for_tab()`. Lets see if bug 1352984 will fix it, otherwise we will need a separate patch. Maybe you can workaround by specifically focusing the newly opened tab.
Depends on: 1352984
Flags: needinfo?(hskupin)
Reporter | ||
Comment 9•7 years ago
|
||
Just checked. No. This is not still present.
Flags: needinfo?(jdorlus)
Assignee | ||
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
Assignee | ||
Comment 10•7 years ago
|
||
So this can still be an issue as seen on bug 1219725 comment 117: 1497299333048 Marionette TRACE 2 -> [0,125,"findElement",{"using":"id","value":"tabbrowser-tabs","element":"0486c14b-f539-3f42-94fe-1ee8a8ebbaf3"}] 1497299333051 Marionette TRACE 2 <- [1,125,null,{"value":{"element-6066-11e4-a52e-4f735466cecf":"586c8c1e-b204-cc43-a0b0-f038aaee8534","ELEMENT":"586c8c1e-b204-cc43-a0b0-f038aaee8534"}}] 1497299333053 Marionette TRACE 2 -> [0,126,"findElements",{"using":"tag name","value":"tab","element":"586c8c1e-b204-cc43-a0b0-f038aaee8534"}] 1497299333058 Marionette TRACE 2 <- [1,126,null,[{"element-6066-11e4-a52e-4f735466cecf":"dc47494a-e346-0a47-ad55-2ab979c222a8","ELEMENT":"dc47494a-e346-0a47-ad55-2ab979c222a8"},{"element-6066-11e4-a52e-4f735466cecf":"ae20660d-cb85-1a40-9659-66a388e61d1f","ELEMENT":"ae20660d-cb85-1a40-9659-66a388e61d1f"},{"element-6066-11e4-a52e-4f735466cecf":"c13249c5-918f-d54f-b160-902ef6dc0e6f","ELEMENT":"c13249c5-918f-d54f-b160-902ef6dc0e6f"}]] 1497299333063 Marionette TRACE 2 -> [0,127,"executeScript",{"scriptTimeout":null,"newSandbox":true,"args":[{"element-6066-11e4-a52e-4f735466cecf":"dc47494a-e346-0a47-ad55-2ab979c222a8","ELEMENT":"dc47494a-e346-0a47-ad55-2ab979c222a8"}],"filename":"tabbar.py","script":"\n let win = arguments[0].linkedBrowser;\n if (!win) {\n return null;\n }\n return win.outerWindowID.toString();\n ","sandbox":"default","line":218}] 1497299333067 Marionette TRACE 2 <- [1,127,{"error":"javascript error","message":"TypeError: win.outerWindowID is undefined","stacktrace":"execute_script @tabbar.py, line 218\ninline javascript, line 5\nsrc: \" return win.outerWindowID.toString();\"\nde:\n@tabbar.py:5:11\n@tabbar.py:0:2\nevaluate.sandbox/promise<@chrome://marionette/content/evaluate.js:154:13\nevaluate.sandbox@chrome://marionette/content/evaluate.js:105:17\nGeckoDriver.prototype.execute_@chrome://marionette/content/driver.js:894:29\nGeckoDriver.prototype.executeScript@chrome://marionette/content/driver.js:793:27\nTaskImpl_run@resource://gre/modules/Task.jsm:321:42\nTaskImpl@resource://gre/modules/Task.jsm:279:3\nasyncFunction@resource://gre/modules/Task.jsm:254:14\nTask_spawn@resource://gre/modules/Task.jsm:168:12\nTaskImpl_handleResultValue@resource://gre/modules/Task.jsm:391:16\nTaskImpl_run@resource://gre/modules/Task.jsm:329:15\nTaskImpl@resource://gre/modules/Task.jsm:279:3\nasyncFunction@resource://gre/modules/Task.jsm:254:14\nTask_spawn@resource://gre/modules/Task.jsm:168:12\nexecute@chrome://marionette/content/server.js:510:15\nonPacket@chrome://marionette/content/server.js:481:7\n_onJSONObjectReady/<@chrome://marionette/content/transport.js:480:9\n"},null] The affected code here is: https://dxr.mozilla.org/mozilla-central/source/testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/browser/tabbar.py#196-202 So as we also noticed on bug 1332122 and bug 1363368 the `linkedBrowser` can be null in case the browser hasn't done its initialization. So we shouldn't simply return null, but wait until win is valid. Then I'm not sure why `outerWindowID` is undefined. Maybe best is to use the patch / test on bug 1219725 and trying to reproduce.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Summary: Puppeteer throws TypeError: win.outerWindowID is null → AfPuppeteer throws TypeError: win.outerWindowID is null
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → hskupin
Status: REOPENED → ASSIGNED
Summary: AfPuppeteer throws TypeError: win.outerWindowID is null → tabbar.py TabBar.get_handle_for_tab can throw "TypeError: win.outerWindowID is undefined"
Assignee | ||
Comment 11•7 years ago
|
||
The problem here is that the underlying tab is gone, or got replaced. This will/can happen for sessionrestore after a restart. So the underlying issue here is in Marionette and will be fixed with bug 1373191.
Assignee | ||
Comment 12•7 years ago
|
||
The problem here only persists after a restart of Firefox with sessionrestore enabled. Especially in such a condition when a background tab hasn't been restored yet. In such a situation there is no valid content browser and window available. Attached a testcase for it.
Assignee | ||
Comment 13•7 years ago
|
||
Andreas, do you have an idea how we should handled not yet restored tabs? In all those cases the tab will be displayed but each of those won't have a contentBrowser yet. It means we cannot interact with the tabs, but users should be able to switch to those. Once that is done, the content browser is initialized. Is it something you can fold into your patch for the refactoring, or should I take care of it separately?
Flags: needinfo?(ato)
Comment 14•7 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #13) > Andreas, do you have an idea how we should handled not yet > restored tabs? In all those cases the tab will be displayed but > each of those won't have a contentBrowser yet. It means we cannot > interact with the tabs, but users should be able to switch to > those. Once that is done, the content browser is initialized. That’s an interesting case. Like you say, the user should definitely be able to interact with it and Marionette needs to handle that a tab’s content browser may not yet have initialised/appeared. > Is it something you can fold into your patch for the refactoring, > Ior should take care of it separately? I’m trying hard not to make any feature changes or conformance fixes in https://bugzilla.mozilla.org/show_bug.cgi?id=1311041, so I think it would be a good idea to address this separately and write a good test case for it. If there’s a test case, I will try my best to not regress that later.
Flags: needinfo?(ato)
Assignee | ||
Comment 15•7 years ago
|
||
Ok, so I will keep this bug for puppeteer, and file a new one which will take care of necessary changes in Marionette code.
Keywords: ateam-marionette-firefox-puppeteer
Assignee | ||
Updated•7 years ago
|
Attachment #8852753 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8854288 -
Attachment is obsolete: true
Updated•7 years ago
|
Priority: -- → P3
Assignee | ||
Comment 18•6 years ago
|
||
I'm not actually working on this bug. Also we most likely need bug 1140237 fixed first.
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 21•6 years ago
|
||
Actually this is not dependent on bug 1311041! I was just able to reproduce it constantly with a minimize testcase I was using to reproduce bug 1510181. Instead it's a plain problem in an execute_script call when we try to access `.toString()` on `null`. It's an easy fix, so taking...
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
No longer depends on: marionette-window-tracking
Priority: P3 → P2
Assignee | ||
Comment 22•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f1392125e694cbc5e4fc068c324a075ff8ac029
Assignee | ||
Comment 23•6 years ago
|
||
Comment 24•6 years ago
|
||
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/95c294286e66 [marionette] Only convert a valid outerWindowID to a string. r=ato
Comment 25•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/95c294286e66
Status: ASSIGNED → RESOLVED
Closed: 7 years ago → 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Assignee | ||
Comment 26•6 years ago
|
||
Silly mistake in a library for testing. Please uplift to beta too to reduce test failures on that branch.
status-firefox64:
--- → affected
status-firefox-esr60:
--- → affected
Whiteboard: [checkin-needed-beta]
Comment 27•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/f65cadcba4bd
Whiteboard: [checkin-needed-beta]
Assignee | ||
Updated•6 years ago
|
Whiteboard: [checkin-needed-esr60]
Comment 28•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr60/rev/3e8c9f8c5307
Whiteboard: [checkin-needed-esr60]
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•