For this to work, we'll need to change how isAppTab is set in tabbrowser.xml (currently it's disabled) and also ensure that nsIWebBrowserChrome3::OnBeforeLinkTraversal is called correctly somewhere. I'm not sure what the best way is to implement this, or whether it's best for isAppTab to live in the parent, the child, or both.

Ended up tinkering around in code related to this anyway so might as well take this.

This patch works but I'm not sure the architecture is right and I'd appreciate some feedback. We need something like nsContentTreeOwner's XULBrowserWindow but for content processes. I added a webBrowserChrome property to TabChild but with some tabs being remote and some not that leads to onBeforeLinkTraversal sometimes being called through TabChild and sometimes through nsContentTreeOwner. To avoid code duplication I added a way for the in-process frame script to register its WebBrowserChrome with the main XULBrowserWindow so it can pass off calls to the right object. It works but it seems a little hacky. Another option might be to just have nsDocShell call onBeforeLinkTraversal on something else before going to the treeOwner. Something at the level of sameTypeRootTreeItem would make sense, I could add the webBrowserChrome property to the docshell there but I'm guessing you folks might have a better idea for me.

I would really rather not add more possible codepaths for this stuff on the docshell level. Things are already complicated enough with the whole treeowner/windowprovider mess.

I guess the easier option to remove code duplication would be to move the onBeforeLinkTraversal code into a module that both browser.js and content.js import

Comment on attachment 8478477 [details] [diff] [review] WIP A cleaner patch is on the way here.

This patch allows something in a frame script to register a webBrowserChrome object with TabChild that it can pass calls to onBeforeLinkTraversal to. This is similar to how we register in the chrome side with XULBrowserWindow.

This passes the isAppTab setting through to the frame script and responds to onBeforeLinkTraversal as necessary. We can enable a bunch of tests in e10s mode with this :)

Comment on attachment 8480780 [details] [diff] [review] TabChild patch > [scriptable, uuid(2eb3bc54-78bf-40f2-b301-a5b5b70f7da0)] > interface nsITabChild : nsISupports > { > readonly attribute nsIContentFrameMessageManager messageManager; > >+ attribute nsIInterfaceRequestor webBrowserChrome; >+ Why nsIInterfaceRequestor? Why don't you just use nsIWebBrowserChrome3? > TabChild::GetInterface(const nsIID & aIID, void **aSink) > { >+ if (aIID.Equals(NS_GET_IID(nsIWebBrowserChrome3))) { >+ if (mWBC) >+ return mWBC->GetInterface(aIID, aSink); >+ return NS_ERROR_NO_INTERFACE; >+ } 2 spaces for indentation (I know the file isn't quite consistent), and use {} always with if. >+ nsCOMPtr<nsIInterfaceRequestor> mWBC; I'm rather surprised if this doesn't cause leaks. Rename the member variable to mWebBrowserChrome3 or some such and move it to TabChildBase and add it to cycle collection. (And please test on a debug build with XPCOM_MEM_LEAK_LOG=1 env variable to see you don't leak anything.)

(In reply to Olli Pettay [:smaug] from comment #10) > Comment on attachment 8480780 [details] [diff] [review] > TabChild patch > > > [scriptable, uuid(2eb3bc54-78bf-40f2-b301-a5b5b70f7da0)] > > interface nsITabChild : nsISupports > > { > > readonly attribute nsIContentFrameMessageManager messageManager; > > > >+ attribute nsIInterfaceRequestor webBrowserChrome; > >+ > Why nsIInterfaceRequestor? Why don't you just use nsIWebBrowserChrome3? Sorry I failed to explain my reasoning there. So over in bug 999239 I need to catch attempts to load URLs in remote tabs that shouldn't be remote. My proposal for doing so is to add a hook into docshell that let JS cancel attempts to load pages and then redirect them to the right process. I'm doing this through the same mechanism as here, XULBrowserWindow in the main process and mWebBrowserChrome in child processes. > >+ nsCOMPtr<nsIInterfaceRequestor> mWBC; > I'm rather surprised if this doesn't cause leaks. > Rename the member variable to mWebBrowserChrome3 or some such and move it to > TabChildBase and add it to cycle collection. > > (And please test on a debug build with XPCOM_MEM_LEAK_LOG=1 env variable to > see you don't leak anything.) Attempting to figure this out, but running in debug with this doesn't show any leaks. Not sure why.

(In reply to Dave Townsend [:mossop] from comment #11) > > Why nsIInterfaceRequestor? Why don't you just use nsIWebBrowserChrome3? > > Sorry I failed to explain my reasoning there. So over in bug 999239 I need > to catch attempts to load URLs in remote tabs that shouldn't be remote. My > proposal for doing so is to add a hook into docshell that let JS cancel > attempts to load pages and then redirect them to the right process. I'm > doing this through the same mechanism as here, XULBrowserWindow in the main > process and mWebBrowserChrome in child processes. That doesn't still answer to my question ;) Why you use type nsIInterfaceRequestor and not nsIWebBrowserChrome3? > Attempting to figure this out, but running in debug with this doesn't show > any leaks. Not sure why. Well, add the member variables to CC anyway.

(In reply to Olli Pettay [:smaug] from comment #12) > (In reply to Dave Townsend [:mossop] from comment #11) > > > > Why nsIInterfaceRequestor? Why don't you just use nsIWebBrowserChrome3? > > > > Sorry I failed to explain my reasoning there. So over in bug 999239 I need > > to catch attempts to load URLs in remote tabs that shouldn't be remote. My > > proposal for doing so is to add a hook into docshell that let JS cancel > > attempts to load pages and then redirect them to the right process. I'm > > doing this through the same mechanism as here, XULBrowserWindow in the main > > process and mWebBrowserChrome in child processes. > That doesn't still answer to my question ;) Why you use type > nsIInterfaceRequestor and not > nsIWebBrowserChrome3? Because I added the new function to a nsIWebBrowserChrome4 since that seems to have been the way to do things in the past so the object needed to have the option to implement both though. But maybe I should just add the function to nsIWebBrowserChrome3?

Even nsIWebBrowserChrome3 is rather bad. We should just merge all nsIWebBrowserChromeX, and then let scripts to provide some functionality. But ok for now, nsIWebBrowserChrome3 is fine, and if you want to add more functionality there, just add it.

It is just seeing nsIInterfaceRequestor as a property type which is really odd.

Landed with the requested changes: https://hg.mozilla.org/integration/fx-team/rev/52b4f9600a14

Backed out for bug 1062522 https://hg.mozilla.org/integration/fx-team/rev/1917953b1f89

I.. um.. put it back in: https://hg.mozilla.org/integration/fx-team/rev/8703c1895505 due to permanent bc-1 failures: https://tbpl.mozilla.org/php/getParsedLog.php?id=47418182&tree=Fx-Team 495 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/plugins/browser_CTP_drag_drop.js | Waited too long for click-to-play notification - 496 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/plugins/browser_CTP_drag_drop.js | Should have a click-to-play notification in the tab in the new window - 498 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/plugins/browser_CTP_drag_drop.js | Waited too long for click-to-play notification - 499 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/plugins/browser_CTP_drag_drop.js | Should have a click-to-play notification in the initial tab again - 505 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/plugins/browser_CTP_drag_drop.js | Waited too long for click-to-play notification - ...

This was re-landed.

This seems to have regressed recently.

(In reply to Chris AtLee [:catlee] from comment #21) > This seems to have regressed recently. Please file a new bug.

filed bug 1080055

Reproduced the initial issue on a Nightly build before and after it regressed, verified on Windows 7 64bit, Ubuntu 14.04 64bit and Mac OS X 10.9.5 using latest Nightly.