Bug 1133077 - [e10s] window.open broken with vertical tabs add-on

https://github.com/darrinhenein/VerticalTabs/blob/master/VerticalTabs.xpi

^-- that's the add-on

Cloned the source of it.

Ok, I understand what's going on. Ugh, it's a sad story.

set_selectedPanel@chrome://global/content/bindings/tabbox.xml:671:13

set_selectedIndex@chrome://global/content/bindings/tabbox.xml:399:15

tabs_XBL_Constructor@chrome://global/content/bindings/tabbox.xml:263:11

VerticalTabs.prototype.rearrangeXUL@resource://verticaltabs/verticaltabs.jsm:168:9

VerticalTabs.prototype.init@resource://verticaltabs/verticaltabs.jsm:80:9

VerticalTabs@resource://verticaltabs/verticaltabs.jsm:59:5

startup/<@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/mikeconley/Library/Application%20Support/Firefox/Profiles/32tsfewj.e10s/extensions/verticaltabs@mozilla.com.xpi!/bootstrap.js:113:14

watcher@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/mikeconley/Library/Application%20Support/Firefox/Profiles/32tsfewj.e10s/extensions/verticaltabs@mozilla.com.xpi!/bootstrap.js -> jar:file:///Users/mikeconley/Library/Application%20Support/Firefox/Profiles/32tsfewj.e10s/extensions/verticaltabs@mozilla.com.xpi!/utils.js:150:9

runOnce@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/mikeconley/Library/Application%20Support/Firefox/Profiles/32tsfewj.e10s/extensions/verticaltabs@mozilla.com.xpi!/bootstrap.js -> jar:file:///Users/mikeconley/Library/Application%20Support/Firefox/Profiles/32tsfewj.e10s/extensions/verticaltabs@mozilla.com.xpi!/utils.js:160:7

First thing to know is that VerticalTabs rearranges the XUL of every window that opens - including pop up windows.

When it does this, it removes the tabbrowser-tabs element from the DOM, and injects it elsewhere, causing the binding to be re-attached.

When the tabs binding is reattached, it re-runs the constructor, and runs this code:

if (this.tabbox && this.tabbox.hasAttribute("selectedIndex")) {

let selectedIndex = parseInt(this.tabbox.getAttribute("selectedIndex"));

this.selectedIndex = selectedIndex > 0 ? selectedIndex : 0;

return;

}

Setting the selectedIndex also sets the selectedIndex on our special async tabbrowser-tabpanels binding. Unfortunately, our tabbrowser-tabpanels binding has a bug in it, whereby if the requested selectedIndex matches the currently selectedIndex, we still do all of the work to remove the content-type="primary" attribute from the browser tab, fire down the MozAfterRemotePaint message, wait for the message to return, and then set content-type="primary" back.

The problem is, however, that this VerticalTabs XUL re-arrangement happens on the window load event, and once it's done running (and we're still waiting for the content process to tell us that the selected tab is ready to paint), we're left in this half-switched state where there is no browser with type="content-primary", thus, no mPrimaryContentShell, and thus, we fail the assertion, and we crash.

I think the easy fix is to make it so that our async binding will not attempt to switch the tabs if the requested index is the same as the current index.

Long term, this will be fixed once gw280 moves us to async tabbrowser-tabs instead of the async tabbrowser-tabpanels.

Try push.

Looks good - let's see what billm thinks.

Hrm. He asks a damn good question:

"I still don't quite understand everything though. If the tabs are the same, then I would expect shouldWait to be false here:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#3405

So _prepare should return a resolved promise. Consequently, _finalizeTabSwitch should run immediately and set content-primary for the new tab:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#3494

Which of these inferences is wrong?"

None - the problem is that in finalizeTabSwitch, the toTab equals the fromTab, so in the last lines of _finalizeTabSwitch, we set the type of the tab to content-targetable, which clears out the mPrimaryContentShell.