MarionetteException in switch_to_window() with chrome scope enabled: "this.browser is undefined"

RESOLVED FIXED in Firefox 39

Status

--
major
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: whimboo, Assigned: chmanchester)

Tracking

({regression})

39 Branch
mozilla41
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed, firefox40 fixed, firefox41 fixed)

Details

(Whiteboard: [qa-automation-blocked])

Attachments

(3 attachments, 1 obsolete attachment)

Created attachment 8613090 [details]
test_windows.py

In our Firefox UI Tests I see constant test failures on Windows machines when opening and closing chrome windows. The root cause is located somewhere in switch_to_window().

Please see the attachment for a testcase. The failure might not always occur, so please run the test multiple times. When commenting out the chrome context switch at the beginning, the test will pass for me. Keep also in mind that this exception does only occur on Windows but not on OS X and Linux.

Here the stack:

Full message: TypeError: this.browser is undefined
Full stack: get@chrome://marionette/content/driver.js:3022:5
BrowserObj.prototype.switchToTab@chrome://marionette/content/driver.js:3123:3
GeckoDriver.prototype.switchToWindow@chrome://marionette/content/driver.js:1520:
9
TaskImpl_run@resource://gre/modules/Task.jsm:330:41
TaskImpl@resource://gre/modules/Task.jsm:275:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14
Task_spawn@resource://gre/modules/Task.jsm:164:12
TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:381:1
TaskImpl_run@resource://gre/modules/Task.jsm:322:13
TaskImpl@resource://gre/modules/Task.jsm:275:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14
Task_spawn@resource://gre/modules/Task.jsm:164:12
CommandProcessor.prototype.execute@chrome://marionette/content/command.js:153:13

Dispatcher.prototype.onPacket@chrome://marionette/content/dispatcher.js:80:5
DebuggerTransport.prototype._onJSONObjectReady/<@resource://gre/modules/devtools
/dbg-client.jsm -> resource://gre/modules/devtools/transport/transport.js:471:9
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:/
/gre/modules/devtools/DevToolsUtils.js:82:14
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:/
/gre/modules/devtools/DevToolsUtils.js:82:14

Code in question here:

Object.defineProperty(BrowserObj.prototype, "browserForTab", {
  get() {
    return this.browser.getBrowserForTab(this.tab);
  }
});


This misbehavior is blocking us from having green tests, and are also causing test failures in the update tests. That's why it is blocking us.
Chris mentioned that this could be a regression. So I did a regression test and it's indeed a regression. Specifically in Nightly between 03/11 and 03/12. Here the pushlog:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fd8e079d6335&tochange=58c9d079f318

I strongly believe that this is a fallout from bug 1096488.
Blocks: 1096488
status-firefox39: --- → affected
status-firefox40: --- → affected
Keywords: regression
Version: 41 Branch → 39 Branch
Created attachment 8613150 [details]
log.txt

Log output with marionette logging turned on.
Created attachment 8613154 [details]
log.txt

The non-cut off version of the log.
Attachment #8613150 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Depends on: 1169874
(Assignee)

Comment 4

4 years ago
Looking at the log, I think there's a timing issue where we end up with a reference to a window that's no longer valid. Bug 1096488 added a bunch of interactions with gBrowser when we switch between windows and makes us lean more on the validity of BrowserObj.window, both of which seem relevant.

I'm not entirely sure so I'd like to debug on one of the machines where this is reproducible (bug 1169874 is about getting the access to do that).
(Assignee)

Comment 5

4 years ago
I have a possible fix on try (not sure if it reproduces there though). There isn't a build environment on the test machine, so debugging there wasn't especially informative.
Assignee: nobody → cmanchester
(Assignee)

Comment 6

4 years ago
The problem doesn't reproduce on try, but I downloaded a build with my fix from try to the test machine and confirmed it works. 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=de268bb3c082
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9cb5e03d834a
(Assignee)

Comment 7

4 years ago
Created attachment 8613245 [details]
MozReview Request: Bug 1169798 - Refresh the marionette server's window reference when switching between windows to avoid intermittent exception.;r=ato

Bug 1169798 - Refresh the marionette server's window reference when switching between windows to avoid intermittent exception.;r=ato
Attachment #8613245 - Flags: review?(ato)
(Assignee)

Comment 9

4 years ago
Comment on attachment 8613245 [details]
MozReview Request: Bug 1169798 - Refresh the marionette server's window reference when switching between windows to avoid intermittent exception.;r=ato

Bug 1169798 - Refresh the marionette server's window reference when switching between windows to avoid intermittent exception.;r=ato
(Assignee)

Comment 10

4 years ago
Comment on attachment 8613245 [details]
MozReview Request: Bug 1169798 - Refresh the marionette server's window reference when switching between windows to avoid intermittent exception.;r=ato

There's a problem with e10s in that last try run, I'll re-request review when I figure out what's up.
Attachment #8613245 - Flags: review?(ato)
Great progress Chris! Thanks a lot for digging into this problem that quickly. Do you have any idea why only Windows boxes are showing this problem? Maybe they are a bit slower, which is causing this race condition? Let me know if you need something else or getting tested.
Status: NEW → ASSIGNED
Duplicate of this bug: 1169180
(Assignee)

Comment 13

4 years ago
I've seen bugs before that only surfaced on old windows versions, so I wouldn't be surprised if their slowness was causing us to hit this. Unfortunately I can't point to a definitive root cause for this, so the patch I have is addressing mostly the symptom. 

I fixed the issue in comment 10, I'm just waiting or a try build to verify on the test machine.
(Assignee)

Comment 14

4 years ago
Comment on attachment 8613245 [details]
MozReview Request: Bug 1169798 - Refresh the marionette server's window reference when switching between windows to avoid intermittent exception.;r=ato

Bug 1169798 - Refresh the marionette server's window reference when switching between windows to avoid intermittent exception.;r=ato
Attachment #8613245 - Flags: review?(ato)
(Assignee)

Comment 15

4 years ago
The new try build is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b2bb52fad6f and works on the test machine with this test case.
Attachment #8613245 - Flags: review?(ato) → review+
Comment on attachment 8613245 [details]
MozReview Request: Bug 1169798 - Refresh the marionette server's window reference when switching between windows to avoid intermittent exception.;r=ato

https://reviewboard.mozilla.org/r/9731/#review8575

Ship It!
https://hg.mozilla.org/mozilla-central/rev/f8ec794163af
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
(Assignee)

Comment 20

4 years ago
According to Henrik this has fixed the test failures in question so we'd like to uplift it to aurora and beta.
You need to log in before you can comment on or make changes to this bug.