If you do var w = window.open(...); w.close(); OOP, the close() happens before the new window's BrowserElementChild is created, because the BrowserElementChild is created only once the frame loader shows the window. The "fake show" I added to TabChild as part of the window.open dance isn't good enough. It's not as simple as creating the BrowserElementChild at the time we do the fake show, either; the message manager doesn't exist until we do the frame loader show. Fun.

My attempts to move the loadFrameScript("BrowserElementChild.js") earlier have not been successful. Or at least, I can indeed load the frame script earlier, but it's not early enough. The problem is that the parent sends the child a message saying "load this frame script". I think I'll need to move to a system where the child loads the script itself.

This is kind of ugly -- it would be cleaner to load BrowserElementChild.js exactly once -- but it's a much simpler change than I think I'd have to make otherwise.

Comment on attachment 633681 [details] [diff] [review] Patch v1 This is just too ugly. I'm not sure how to keep a list of scripts to load asap, and how to prevent parent to send them afterwards, but that is what should happen.

I can't see how to do this in a simple manner. The best thing I can think of is: For in-process iframes, do what we currently do. For OOP iframes created from the frameloader, set a flag on the tabchild before we call show, saying "you're a browser frame". For OOP iframes opened via window.open, manually set that flag during the window.open. Then when we do TabChild::RecvShow, check the flag, and if it's set, instantiate the BrowserElementChild. That's much more complicated than this current patch, and the benefits are small, if they exist at all. I don't think this is worth wasting more time on. The approach here isn't fragile -- it's not going to cause regressions down the line. Code complete is in a month; I simply can't spend time on things that already work.

> Code complete is in a month I should say, feature complete is in a month, code complete in two months.

Comment on attachment 633681 [details] [diff] [review] Patch v1 Still makes no sense. A temporary hack I could accept is to add some boolean parameter to PBrowser constructor. If it is true, TabChild would automatically load "chrome://global/content/BrowserElementChild.js" asap after creation. Then remove the explicit http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementParent.js#163 and parent->CreateTab(chromeFlags, OwnerIsBrowserFrame()) in nsFrameLoader.

I feel that hack is much uglier than what I have here, so I'll try to come up with something that we both agree is more elegant.

What do you think of this?

smaug wants me to send the in-process load-script from C++ too, since he doesn't like sending in JS sometimes and C++ other times. He also wants me to pass a boolean parameter for is-browser-frame in to the PBrowser constructor, instead of sending a message, because sending a message is "slow".

This landed and bounced because of a wide range of intermittent orange; see bug 764718 comment 26. My guess is that the orange came from this bug, rather than bug 764718, but I need to investigate.

But then this was backed out: https://hg.mozilla.org/mozilla-central/rev/1da4a83db7b2

I can't reproduce this locally on Mac or Linux, so I've pushed to try with some extra debugging information enabled: https://tbpl.mozilla.org/?tree=Try&rev=e6d95c9eb1f5 https://tbpl.mozilla.org/?tree=Try&rev=2120ae253449

Gah, I pushed those with bad trychooser syntax, so no builds were run. Trying again: https://tbpl.mozilla.org/?tree=Try&rev=98321ce6f771 https://tbpl.mozilla.org/?tree=Try&rev=8afcbcae9f7d

Interesting! > JavaScript error: resource:///components/BrowserElementParent.js, line 176: can't access > dead object From https://tbpl.mozilla.org/php/getParsedLog.php?id=12913528&tree=Try#error0 (full queue, this bug plus bug 764718).

More try pushes: https://tbpl.mozilla.org/?tree=Try&rev=a3cd3e86d48b <-- maybe fixes some tests https://tbpl.mozilla.org/?tree=Try&rev=967dc9b52b4b <-- with patch v1 from this bug, to see if that has the same oranges

I now think the problem is in bug 764718.

I was wrong in comment 13: This was never backed out.