The devtools plan is that although webconsole and debugging will occur remotely, the inspector panel (tree view) needs to be implemented in the same process as the content it inspects. My current thought is something like: <xul:browser type="content-primary" remote="true" id="idofremotebrowser"/> ... <xul:iframe type="chrome" remote="true" related="idofremotebrowser" src="chrome://inspector/content/inspectorpanel.xul" /> And the content scripts for the iframe would have the following extra properties: * relatedContent (the window of the content specified by "linked") * relatedDocshell (the docshell of the content specified by "linked")

This is a good start. Currently that iframe lives in a xul:panel. Probably not an issue. What *may* be an issue is that the code that populates this iframe doesn't live in the iframe itself. It's a jsm imported into chrome. We may need a way to host these jsms in the content process as well and give them access to both the content process' browser document and the iframe's document.

That's not a problem, you can use JSMs from the content script to hook it all up.

Olli, can you take this? This blocks the inspector/highlighter work that the devtools team needs to do in early Q4. Let me know if you can attach a date when you think it might be done, and I'll add it to the short-term goal list.

I should be able to do this next week, if not this weekend.

Oh, hmm, we don't support message managers for non-remote chrome browsers. But I guess I could change that, so that in case of "inspection" browsers, there could be mm. Also, so far we haven't supported chrome remote browsers. I guess the syntax/API needs to look like: <browser type="content-primary"/> <browser type="chrome" forcemessagemanager="true" needsrelated="true"> (The fact that browsers are remote or not doesn't change anything. It is just that they both need to be remote="true" or both need to be remote="false". Also, it doesn't matter whether one uses <browser> or <iframe>.) needsrelated attribute is need to prevent automatic process and docshell creation. Then, when needed one does in the code something like var browsers = document.getElementsByTagName("browser"); browsers[1].relatedBrowser = browsers[0]; Setting .relatedBrowser ensures that both browsers use the same process.

(In reply to Olli Pettay [:smaug] from comment #5) > Then, when needed one does in the code something like > var browsers = document.getElementsByTagName("browser"); > browsers[1].relatedBrowser = browsers[0]; Hrm, this may need to be browsers[1].frameLoader.relatedBrowser = browsers[0].frameLoader;

There's no particular reason why these browsers need to be type="chrome", is there? Wouldn't a type="content" browser work just fine, as long as it isn't content-primary?

I thought there would be some js running in the inspector frame, and it should be able to access the related frame. If the inspector doesn't run using chrome privileges, that access would be blocked. (And btw, type="chrome" should be easy to implement.)

The inspector is running with chrome privileges, but as far as I know that's no reason why we couldn't load it into a content-type docshell.

Actually, inspector should use xul:iframe since xul:browser has already some assumptions when and how message manager works. I have WIP patch for this. Need to write some more tests and clean up the code a bit.

Uploaded to tryserver.

Rob, would be great if you could try the patch a bit. Basically, create <xul:iframe needsrelated="true">, add it to document and set its relatedFrame to point some frameloader iframe.QueryInterface(Components.interfaces.nsIFrameLoaderOwner).frameLoader.relatedFrame = _frame_loader_of_some_browser; At that point the messagemanager of iframe is created and you can send scripts and add message listeners and send messages. In the tabchildglobal of iframe relatedDocShell and relatedContent should now point to the other tab. iframe is out-of-process if related browser is out-of-process, otherwise same-process. Also, it should be possible to re-set .relatedFrame. That re-creates message manager.

Tryserver shows still some problem.

Linux64 only problems o_O

Comment on attachment 558269 [details] [diff] [review] patch New patch coming (without support for chrome docshells in content process).

The patch is still causing apparently content process crash in tryserver/linux64. Can't reproduce locally.

Comment on attachment 558443 [details] [diff] [review] patch Benjamin, what do you think about this approach?

Comment on attachment 558443 [details] [diff] [review] patch I'd like bz or somebody to review this. I think the API is ok, but perhaps the devtools team should provide feedback on that.

Yes, anyone from devtools team, please give feedback. This is the first step, we can sure add more to the API later.

Comment on attachment 558443 [details] [diff] [review] patch I *think* this API looks fine. I have a couple of comment nits for you, but otherwise, f+. since you're in nsIFrameLoader.idl anyway: /** * With this event mode, it's the application's responsability to * convert and forward events to the content process change responsability->responsibility. in nsFrameLoader.cpp: + // We don't fire any errors or throw exceptions if we're + // still waiting that .relatedFrame will be set. + if (NeedsRelated() && !mRelatedFrame) { ITYM, "...if we're still waiting for .relatedFrame to be set". thanks! :)

Comment on attachment 558443 [details] [diff] [review] patch >+++ b/content/base/public/nsIFrameLoader.idl >+ attribute nsIFrameLoader relatedFrame; This needs documentation. The interaction between LoadURI and the setter here is ... non-obvious at best. Thank you for adding that builtinclass annotation! >+++ b/content/base/public/nsIFrameMessageManager.idl >+ * The current top level window in the related frame or null. "current" makes it sounds like it's an inner window. I assume it's actually an outer window, right? Certainly the getter returns an outer window. In that case, what makes it "current"? Worth documenting whether "null" means no related frame or no window in it or either one. >+ * The related docshell or null. Again, document when null can happen. >+++ b/content/base/src/nsFrameLoader.cpp >+ // still waiting that .relatedFrame will be set. still waiting to be told what our related frame is. >+ mShown = PR_TRUE; Please document the mShown member. And maybe call it mIsShowing? >+nsFrameLoader::SetRelatedFrame(nsIFrameLoader* aRelated) Why do we enforce that aRelated != this but not lack of other sorts of loops in the related-frame chain? >+++ b/content/base/src/nsFrameLoader.h >+ nsresult DestroyInternal(PRBool aReuse); Document, please. Especially the invariants aReuse values assume. >+ bool NeedsRelated(); Document. >+++ b/dom/ipc/PBrowser.ipdl >+ RelatedFrame(PBrowser related); Document. >+++ b/dom/ipc/TabChild.h >+ nsWeakPtr mRelatedFrame; It would be good to document what you can ask for from that nsWeakPtr. You seem to put a docshell in but ask for a webnavigation back out, then getInterface a docshell from that... which is confusing. I suspect you want to change some part of that code too. r=me with the above issues fixed.

Um, nice, the test case crashes now on Windows try. Crash reason: EXCEPTION_BREAKPOINT Crash address: 0x6a0c111b Thread 0 (crashed) 0 mozalloc.dll!mozalloc_abort(char const * const) [mozalloc_abort.cpp:1131ea691369 : 77 + 0x0] eip = 0x6a0c111b esp = 0x0030c804 ebp = 0x0030c8a4 ebx = 0x00000040 esi = 0x6bb7e457 edi = 0x6bbba392 eax = 0x00000000 ecx = 0x6bbba4c8 edx = 0x00000003 efl = 0x00000202 Found by: given as instruction pointer in context 1 xul.dll!NS_DebugBreak_P [nsDebugImpl.cpp:1131ea691369 : 345 + 0x9] eip = 0x6abf00eb esp = 0x0030c810 ebp = 0x0030c8a4 Found by: call frame info 2 xul.dll!mozilla::layers::SurfaceDescriptor::MaybeDestroy(mozilla::layers::SurfaceDescriptor::Type) [PLayers.cpp:1131ea691369 : 326 + 0x17] eip = 0x6abb8899 esp = 0x0030cc34 ebp = 0x0030d0ec ebx = 0x00000002 Found by: call frame info 3 xul.dll!mozilla::layers::SurfaceDescriptor::operator=(mozilla::layers::SurfaceDescriptor const &) [PLayers.cpp:1131ea691369 : 437 + 0x6] eip = 0x6abb895c esp = 0x0030cc4c ebp = 0x0030d0ec Found by: call frame info 4 xul.dll!mozilla::layers::ShadowThebesLayerD3D10::Swap(mozilla::layers::ThebesBuffer const &,nsIntRegion const &,mozilla::layers::OptionalThebesBuffer *,nsIntRegion *,mozilla::layers::OptionalThebesBuffer *,nsIntRegion *) [ThebesLayerD3D10.cpp:1131ea691369 : 485 + 0xa] eip = 0x6ac4ec7f esp = 0x0030cc64 ebp = 0x0030cc78 ebx = 0x0de86820 Found by: call frame info 5 xul.dll!mozilla::layers::ShadowLayersParent::RecvUpdate(InfallibleTArray<mozilla::layers::Edit> const &,InfallibleTArray<mozilla::layers::EditReply> *) [ShadowLayersParent.cpp:1131ea691369 : 326 + 0x21] eip = 0x6ac51e5b esp = 0x0030cc80 ebp = 0x0030d0e0 Found by: call frame info 6 xul.dll!mozilla::layers::PLayersParent::OnMessageReceived(IPC::Message const &,IPC::Message * &) [PLayersParent.cpp:1131ea691369 : 220 + 0x17] eip = 0x6ab98274 esp = 0x0030d158 ebp = 0x0030d174 ebx = 0xfffffffc Found by: call frame info

One way how to simplify Firebug's e10s adoption is utilizing the linked remote browser (just like the built-in inspector panel) In fact the entire Firebug UI (except of the Firebug start button now available on Firefox toolbar and Firefox Web Developer menu overly) is embedded inside a <xul:browser> element, see the following overlay: <!-- Firebug panel --> <vbox id="appcontent"> <splitter id="fbContentSplitter" collapsed="true"/> <vbox persist="height" id="fbMainFrame" collapsed="true"> <browser id="fbMainContainer" flex="2" src="chrome://firebug/content/firefox/firebugFrame.xul" disablehistory="true"/> </vbox> </vbox> That's how Firebug puts itself into the browser window (and btw. using type="chrome"). Firebug has been forced to use <browser> since there were some font-zoom issues when using <iframe> Are there still any expected problems with using <browser> ? Also, Firebug is importing several JSMs, mostly simple stuff but there is firebug-service.js module implementing Firebug's debugger API based on JSD. Is it possible to load such JSM into the content process as well? Also, if I understand correctly, all overlays applied on the linked <browser> element will also run in the same process, correct? (I am thinking about Firebug extensions here) Honza

marking this as assigned since Olli's deep into it. Honza, not sure I understand your question. You have overlays *inside* your browser element or containing the <browser>? If containing, then no, I don't think they'll be in the same process. is there a good reason you can't move from <browser> to <iframe>? I was under the impression browsers were used when Firebug was created because iframes weren't really a viable option inside xul.

(In reply to Rob Campbell [:rc] (robcee) from comment #27) > marking this as assigned since Olli's deep into it. > > Honza, not sure I understand your question. You have overlays *inside* your > browser element or containing the <browser>? If containing, then no, I don't > think they'll be in the same process. The overlays are inside the browser element. > is there a good reason you can't move from <browser> to <iframe>? I was > under the impression browsers were used when Firebug was created because > iframes weren't really a viable option inside xul. We had couple of problems with iframe in the past. One related to a security issue (bug 665369) and the type attribute of the iframe. The second was related to the text-zoom, since content of the iframe is XUL and it behaves differently from using <browser> element These issues could be perhaps fixed, but it would be easier for Firebug to continue running in <browser>. Honza

I tried to apply the patch on fx-team repo (cloned now), but there are same failures. Should I use different repository? Honza $ patch -p1 --dry-run < linked_browsers_4.diff patching file content/base/public/nsIFrameLoader.idl patching file content/base/public/nsIFrameMessageManager.idl patching file content/base/src/nsFrameLoader.cpp Hunk #2 FAILED at 324. Hunk #3 FAILED at 415. Hunk #4 succeeded at 456 (offset -3 lines). Hunk #5 FAILED at 768. Hunk #7 FAILED at 943. Hunk #8 FAILED at 1260. Hunk #9 succeeded at 1340 (offset -6 lines). Hunk #11 succeeded at 1816 (offset -6 lines). Hunk #13 succeeded at 2073 (offset -6 lines). Hunk #14 succeeded at 2195 with fuzz 1. 5 out of 14 hunks FAILED -- saving rejects to file content/base/src/nsFrameLoade r.cpp.rej patching file content/base/src/nsFrameLoader.h Hunk #2 succeeded at 163 with fuzz 1. Hunk #4 FAILED at 250. 1 out of 6 hunks FAILED -- saving rejects to file content/base/src/nsFrameLoader .h.rej patching file content/base/src/nsFrameMessageManager.cpp patching file content/base/src/nsGkAtomList.h Hunk #1 succeeded at 595 (offset 3 lines). patching file content/base/src/nsInProcessTabChildGlobal.cpp Hunk #2 FAILED at 391. 1 out of 2 hunks FAILED -- saving rejects to file content/base/src/nsInProcessTa bChildGlobal.cpp.rej patching file content/base/src/nsInProcessTabChildGlobal.h patching file content/base/test/chrome/Makefile.in patching file content/base/test/chrome/file_bug669698.xul patching file content/base/test/chrome/test_bug669698.xul patching file dom/ipc/PBrowser.ipdl patching file dom/ipc/TabChild.cpp Hunk #1 FAILED at 823. 1 out of 2 hunks FAILED -- saving rejects to file dom/ipc/TabChild.cpp.rej patching file dom/ipc/TabChild.h patching file layout/generic/nsSubDocumentFrame.h