967021 - JavaScript Error: "listenerWindow.location is null"

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Zac C (:zac)]

I've just encountered a 'hung' UI test running against Hamachi when launching an app using the atom.

In logcat the last activity from Gecko was this:

I/Gecko   ( 4484): 1391443823047	Marionette	DEBUG	Got request: executeWithCallback, data: {"to":"0","sessionId":"3-b2g","name":"executeAsyncScript","parameters":{"specialPowers":false,"scriptTimeout":null,"newSandbox":true,"args":[],"filename":"gaia_test.py","script":"GaiaApps.launchWithName('Usage')","line":82}}, id: {379a1fc7-b603-4f40-80b1-2266ae9e34b2}
E/GeckoConsole( 4484): Content JS LOG at dummy file:301 in GaiaApps.launch: launching app with name 'Usage'
I/Gonk    ( 4484): Setting nice for pid 4681 to 1
I/Gonk    ( 4484): Changed nice for pid 4681 from 18 to 1.
I/Gonk    ( 4484): Setting nice for pid 4598 to 18
I/Gonk    ( 4484): Changed nice for pid 4598 from 1 to 18.
I/Gecko   ( 4484): 1391443823570	Marionette	INFO	sendToClient: {"from":"0","value":{"frame":{"ELEMENT":"{e74be563-cb87-4960-a882-8438f5957e38}"},"src":"app://costcontrol.gaiamobile.org/index.html","name":"Usage","origin":"app://costcontrol.gaiamobile.org"}}, {379a1fc7-b603-4f40-80b1-2266ae9e34b2}, {379a1fc7-b603-4f40-80b1-2266ae9e34b2}
E/GeckoConsole( 4484): Content JS LOG at dummy file:292 in GaiaApps.launch/appOpen/<: app with origin 'app://costcontrol.gaiamobile.org' has launched
I/Gecko   ( 4484): 1391443823591	Marionette	DEBUG	Got request: switchToFrame, data: {"to":"0","sessionId":"3-b2g","name":"switchToFrame","parameters":{"focus":true,"element":"{e74be563-cb87-4960-a882-8438f5957e38}"}}, id: {67c4c3be-9e6a-4bac-b37c-62caac4acd99}
I/Gecko   ( 4484): 1391443823593	Marionette	INFO	Switched to frame: {"frameValue":"{e74be563-cb87-4960-a882-8438f5957e38}"}
I/Gecko   ( 4484): 1391443823597	Marionette	INFO	trying remote frame 0
E/GeckoConsole( 4484): [JavaScript Error: "listenerWindow.location is null" {file: "chrome://marionette/content/marionette-server.js" line: 2354}]

This resulted in a socket.timeout after 6+ minutes of waiting.

The test being run is tests/functional/cost_control/test_cost_control_reset_wifi.py

The replication frequency is low (maybe 10%) but the effect is bad as this does not fail quickly.

Comment 1

[David Burns :automatedtester]

Attachment: Add extra check on variable to prevent marionette crash

Comment 2

[David Burns :automatedtester]

I have done the obvious of adding another check but since this is OOP I am not sure if there is a better way so let me know what you suggest

Comment 3

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 8398834 [details] [diff] [review]
Add extra check on variable to prevent marionette crash

Review of attachment 8398834 [details] [diff] [review]:
-----------------------------------------------------------------

I can't reproduce this either, but what do you think of this comment?

::: testing/marionette/marionette-server.js
@@ +2370,5 @@
>                              Services.wm.getOuterWindowWithId(message.json.value);
>  
>          //go in here if we're already in a remote frame.
> +        if ((!listenerWindow || (listenerWindow.location &&
> +                                 listenerWindow.location.href != message.json.href)) &&

If I understand the problem correctly, I think the OOP frame exists but is still being initialized. so listenerWindow is an object, but its location is not yet a value, so what we should do here is:

((!listenerWindow || (!listenerWindow.location) || (listenerWindow.location &&
		listenerWindow.location.href != message.json.href)) &&

Comment 4

[David Burns :automatedtester]

(In reply to Malini Das [:mdas] from comment #3)

> ((!listenerWindow || (!listenerWindow.location) || (listenerWindow.location
> &&
> 		listenerWindow.location.href != message.json.href)) &&

I feel like I am missing something. The `(!listenerWindow.location) ||` feels unneeded since we are testing it in the next part of the conditional

Comment 5

[David Burns :automatedtester]

Attachment: Check that a window listener is not null in marionette

Comment 6

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 8405416 [details] [diff] [review]
Check that a window listener is not null in marionette

Review of attachment 8405416 [details] [diff] [review]:
-----------------------------------------------------------------

You're right in your comment that we don't need to do the (!listenerWindow.location) check. You can remove this and push

Comment 7

[David Burns :automatedtester]

Attachment: Check that a window listener is not null in marionette

Comment 8

[David Burns :automatedtester]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce8248d046d1

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ce8248d046d1