1311657 - switch_to_frame() can prevent close_chrome_window() from closing the chrome window

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I have seen this issue while working on bug 1259055. The XUL dialog (test_anonymous_content.xul) which gets opened by "test_anonymous_content.py TestAnonymousContent.test_switch_to_anonymous_iframe" cannot be closed after `switch_to_frame()` has been called.

A switch_to_window() call to actually switch to the chrome window succeeds, but the call to close_chrome_window() right after fails. I cannot 

1476962103221	Marionette	TRACE	conn10 -> [0,21,"switchToFrame",{"focus":true,"element":"2aa734d6-973b-b641-92ee-10fcded402cb"}]
1476962103223	Marionette	TRACE	conn10 <- [1,21,null,{}]
1476962103225	Marionette	TRACE	conn10 -> [0,22,"getChromeWindowHandles",null]
1476962103226	Marionette	TRACE	conn10 <- [1,22,null,["3","90"]]
1476962103227	Marionette	TRACE	conn10 -> [0,23,"getContext",null]
1476962103228	Marionette	TRACE	conn10 <- [1,23,null,{"value":"chrome"}]
1476962103229	Marionette	TRACE	conn10 -> [0,24,"setContext",{"value":"chrome"}]
1476962103230	Marionette	TRACE	conn10 <- [1,24,null,{}]
1476962103234	Marionette	TRACE	conn10 -> [0,25,"switchToWindow",{"name":"90"}]
1476962103235	Marionette	TRACE	conn10 <- [1,25,null,{}]
1476962103237	Marionette	TRACE	conn10 -> [0,26,"getCurrentChromeWindowHandle",null]
1476962103237	Marionette	TRACE	conn10 <- [1,26,null,{"value":"90"}]
1476962103239	Marionette	TRACE	conn10 -> [0,27,"closeChromeWindow",null]
1476962103240	Marionette	TRACE	conn10 <- [1,27,null,{}]
1476962103243	Marionette	TRACE	conn10 -> [0,28,"getChromeWindowHandles",null]
1476962103244	Marionette	TRACE	conn10 <- [1,28,null,["3","90"]]
[..]
1476962108247	Marionette	TRACE	conn10 -> [0,77,"setContext",{"value":"chrome"}]
1476962108247	Marionette	TRACE	conn10 <- [1,77,null,{}]
1476962108249	Marionette	TRACE	conn10 -> [0,78,"getContext",null]
1476962108250	Marionette	TRACE	conn10 <- [1,78,null,{"value":"chrome"}]
1476962108251	Marionette	TRACE	conn10 -> [0,79,"setContext",{"value":"chrome"}]
1476962108252	Marionette	TRACE	conn10 <- [1,79,null,{}]
1476962108253	Marionette	TRACE	conn10 -> [0,80,"takeScreenshot",{"highlights":null,"full":true,"hash":false,"id":null}]
1476962108254	Marionette	TRACE	conn10 <- [1,80,null,{"value":""}]

I will disable this test as part of my patch on bug 1259055 given that it actually can cause a regression in following tests.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Same problem exists in test_switch_frame_chrome.py TestSwitchFrameChrome.* when calling switch_to_frame(0).

So it's not only anonymous iframe related.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Lets wait for bug 1259055 before tackling this.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The problem here is similar to endSession that we have to switch to the top-most frame. Patch upcoming.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1311657 - Switch to top-most frame first before closing chrome window.

Review commit: https://reviewboard.mozilla.org/r/98324/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98324/

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8818194 [details]
Bug 1311657 - Switch to top-most frame first before closing chrome window.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98324/diff/1-2/

Comment 6

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8818194 [details]
Bug 1311657 - Switch to top-most frame first before closing chrome window.

https://reviewboard.mozilla.org/r/98324/#review98918

This is excellent.  Thanks for fixing this!

Comment 7

[Andreas Tolfsen ❲:ato❳]

If possible, I’d quite like to uplift this all the way.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/24bf5850aefe
Switch to top-most frame first before closing chrome window. r=ato

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/24bf5850aefe

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Test-only patch we would like to have for the next ESR release. Please uplift to aurora. Thanks.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6db2fdca32d1