1275485 - Regression: can't close container that have been separated from the main window (can't close fx either and need to kill process)

Status: VERIFIED FIXED

(Core :: DOM: Security, defect, P1)

Description

[Kamil Jozwiak [:kjozwiak]]

It looks like bug # 1271792 caused a nasty regression. When you drag a container tab out of the main window into it's own window, you won't be able to close it via the UI. The only way I managed to close it was to kill the entire process. I've reproduced this on all three platforms:

* Win 10 x64 VM - Reproduced
* Ubuntu 14.04.4 x64 VM - Reproduced
* OSX 10.11.15 x64 - Reproduced

I think this should be a P1 as it breaks normal functionality which is important for a broader m-c release.

STR: (very easy to reproduce)

* launch the latest version of m-c
* go into about:config and change privacy.userContext.enabled;true
* open a new container via File -> New Container Tab -> Personal
* once the tab opens, drag it into it's own separate window
* Now try closing the container window via:
** the "X" at the top right/left corner of the window (depending on the OS)
** Hamburger Menu -> Quit Button
** the "X" next to the container tab

You'll notice that you can't close the window/tab using the above methods. The only way that I managed to close the window/tab was to kill the entire process.

Regression range:

16:22.18 INFO: Last good revision: fe1a7608cfd77794732ce6d1f9a2ee7cc33e213f
16:22.18 INFO: First bad revision: 4df69b84d9e5d0ab1324523c1346ed8f2dd1439e
16:22.18 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fe1a7608cfd77794732ce6d1f9a2ee7cc33e213f&tochange=4df69b84d9e5d0ab1324523c1346ed8f2dd1439e

Comment 1

[Kamil Jozwiak [:kjozwiak]]

I've also attached a video to illustrate the issue:

* https://youtu.be/hy1pno8eGcs

Comment 2

[Jonathan Hao (inactive) [:jhao]]

Thanks, Kamil, for catching this regression. I'll investigate the reason.

Comment 3

[Jonathan Hao (inactive) [:jhao]]

If I load a normal web page in the tab before dragging it, I can still close it normally. The regression happens when the tab is in about:newtab. It also happens on about:config, so probably there's something wrong with chrome pages.

In fact [1] could have solve bug 1271792. We make [2] because we thought some of baku's previous changes can be reverted. I tried backing out [2], and the regression is gone.

[1] https://hg.mozilla.org/integration/mozilla-inbound/rev/7cba66ab3465
[2] https://hg.mozilla.org/integration/mozilla-inbound/rev/8532f8ec8acd

Comment 4

[Jonathan Hao (inactive) [:jhao]]

Attachment: Put back changes in bug 1260766 that sets userContextId in frame loader

Not only dragging about:xxx pages breaks, dragging normal webpages in non-e10s mode also breaks, so I'm going to put baku's changes in nsFrameLoader back. Sorry for causing this regression.

baku, could you review this patch? It is exactly the same as the one you wrote in 1260766.

Comment 5

[Jonathan Hao (inactive) [:jhao]]

Attachment: Put back changes in bug 1260766 that sets userContextId in frame loader

Comment 6

[Jonathan Hao (inactive) [:jhao]]

Attachment: Put back changes in bug 1260766 that sets userContextId in frame loader

Comment 7

[Jonathan Hao (inactive) [:jhao]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b29d31d32644

Comment 8

[Jonathan Hao (inactive) [:jhao]]

Attachment: Put back changes in bug 1260766 that sets userContextId in frame loader.

Edit commit message.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/959b300c171e

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/959b300c171e

Comment 11

[Ethan Tseng [:ethan]]

Jonathan, thanks for fixing this regression quickly!

Kamil, thank you for filing this bug.
Could you help to verify the fix? :)

Comment 12

[Kamil Jozwiak [:kjozwiak]]

Went through verification using the following build:
* https://archive.mozilla.org/pub/firefox/nightly/2016/06/2016-06-02-03-02-20-mozilla-central/
* fx49.0a1 buildID: 20160602030220, changeset: 34a8be4346a9

* Opened all the different containers via several different methods:
** closed the container tab via the "X" next to the tab
** closed the container window via the "X" at the top right hand side of the window
** closed the FX via the Hamburger Menu
** closed the container tab via "File --> Close Tab"
** closed the container window via "File --> Close Window"

* ensured that I can drag several about:xxx pages into their own separate windows and close them via:
** closed the tab via the "X" next to the tab
** closed the window via the "X" at the top right hand side of the window
** closed the FX via the Hamburger Menu
** closed the tab via "File --> Close Tab"
** closed the window via "File --> Close Window"

* ensured that I can drag several web pages into their own separate windows via non-e10s and closed them via:
** closed the tab via the "X" next to the tab
** closed the window via the "X" at the top right hand side of the window
** closed the FX via the Hamburger Menu
** closed the tab via "File --> Close Tab"
** closed the window via "File --> Close Window"

I also went through the above test cases using a m-c debug build to ensure that there wasn't any assertions or crashes. Build Used:
* fx49.0a1 buildID: 20160602112746 changeset: 92e0c73391e7 tip