This process takes about 100ms in current code. Short of juggling JS options, which will probably *increase* compilation time, we should just get this off the critical path. Playing off of bug 781724, we can preload and precompile BrowserElementChild.js in the intervening time between process launch and loading the first PBrowser. Since this is currently on the critical path, I expect all the time be knocked off startup, but I haven't written the patch to measure yet.

There are a few things going on in this patch. Sorry, couldn't split into smaller chunks, they're all interdependent. This patch builds up several smaller parts in order to let us achieve this goal - when a content process is prelaunched, pre-fetch and pre-compile BrowserElementChild.js. - incidentally, also pre-create a TabChild instance to get JSContext etc. startup off the critical path We need to create a TabChild instance in order to prefetch and precompile BrowserElementChild.js. Here are the sub-parts First I've split nsFrameMessageManager script-loading into separate fetch-and-compile and execute steps. This is pretty trivial. Second, TabChild now initializes its nsWebBrowser from Init(). That means we create a fake widget for it and so forth. We need this initialization in order to initialize the TabChildGlobal. This also fixes the bug dup'd to this one, something jlebar stepped on with the browser-element work. Third, we aggressively initialize TabChildGlobal. I added a flag requesting whether or not to execute BrowserElementChild.js when appropriate. For precreated TabChild, we don't execute immediately. Whenever we defer execution though, we always ensure BrowserElementChild.js has been executed before needed. Fourth, we delay initialization of IME state until where we previously initialized the widget et al., from RecvShow(). Initializing IME requires an IPC connection, which we don't have for precreated TabChild. And finally, I put all this together in TabChild::PreloadSlowThings. Let me know if you have further questions or there are parts here you'd like someone else to review.

Comment on attachment 651867 [details] [diff] [review] Refactor TabChild to allow pre-created instances, and then use a pre-created instance to pre-load and compile BrowserElementChild.js > TabChild::TabChild(PRUint32 aChromeFlags, bool aIsBrowserElement, > PRUint32 aAppId) > : mRemoteFrame(nullptr) > , mTabChildGlobal(nullptr) > , mChromeFlags(aChromeFlags) > , mOuterRect(0, 0, 0, 0) > , mLastBackgroundColor(NS_RGB(255, 255, 255)) >- , mDidFakeShow(false) >+ , mAppId(aAppId) > , mIsBrowserElement(aIsBrowserElement) >- , mAppId(aAppId) >+ , mTriedBrowserInit(false) > { > printf("creating %d!\n", NS_IsMainThread()); > } (At some point we should remove those printfs) > nsresult > TabChild::Init() > { >- nsCOMPtr<nsIWebBrowser> webBrowser = do_CreateInstance(NS_WEBBROWSER_CONTRACTID); >- if (!webBrowser) { >- NS_ERROR("Couldn't create a nsWebBrowser?"); >- return NS_ERROR_FAILURE; >- } >+ nsCOMPtr<nsIWebBrowser> webBrowser = >+ do_CreateInstance(NS_WEBBROWSER_CONTRACTID); >+ if (!webBrowser) { >+ NS_ERROR("Couldn't create a nsWebBrowser?"); >+ return NS_ERROR_FAILURE; >+ } > >- webBrowser->SetContainerWindow(this); >- mWebNav = do_QueryInterface(webBrowser); >- NS_ASSERTION(mWebNav, "nsWebBrowser doesn't implement nsIWebNavigation?"); >+ webBrowser->SetContainerWindow(this); >+ mWebNav = do_QueryInterface(webBrowser); >+ NS_ASSERTION(mWebNav, "nsWebBrowser doesn't implement nsIWebNavigation?"); > >- nsCOMPtr<nsIDocShellTreeItem> docShellItem(do_QueryInterface(mWebNav)); >- docShellItem->SetItemType(nsIDocShellTreeItem::typeContentWrapper); >+ nsCOMPtr<nsIDocShellTreeItem> docShellItem(do_QueryInterface(mWebNav)); >+ docShellItem->SetItemType(nsIDocShellTreeItem::typeContentWrapper); > >- nsCOMPtr<nsIObserverService> observerService = >- do_GetService(NS_OBSERVERSERVICE_CONTRACTID); >+ nsCOMPtr<nsIObserverService> observerService = >+ do_GetService(NS_OBSERVERSERVICE_CONTRACTID); This is somewhat odd. The whole file is for some reason using non-Gecko-coding-style indentation. I'd prefer to change the code which uses wrong coding style to use right one, not other way around. But for this patch, I'd just remove coding style changes. >@@ -274,22 +276,36 @@ protected: > nsEventStatus DispatchWidgetEvent(nsGUIEvent& event); > > virtual PIndexedDBChild* AllocPIndexedDB(const nsCString& aASCIIOrigin, > bool* /* aAllowed */); > > virtual bool DeallocPIndexedDB(PIndexedDBChild* aActor); > >+ enum FrameScriptLoading { DONT_LOAD_SCRIPTS, DEFAULT_LOAD_SCRIPTS }; >+ bool InitTabChildGlobal(FrameScriptLoading aScriptLoading=DEFAULT_LOAD_SCRIPTS); Space before and after =

(In reply to Olli Pettay [:smaug] from comment #4) > Comment on attachment 651867 [details] [diff] [review] > Refactor TabChild to allow pre-created instances, and then use a pre-created > instance to pre-load and compile BrowserElementChild.js > > > TabChild::TabChild(PRUint32 aChromeFlags, bool aIsBrowserElement, > > PRUint32 aAppId) > > : mRemoteFrame(nullptr) > > , mTabChildGlobal(nullptr) > > , mChromeFlags(aChromeFlags) > > , mOuterRect(0, 0, 0, 0) > > , mLastBackgroundColor(NS_RGB(255, 255, 255)) > >- , mDidFakeShow(false) > >+ , mAppId(aAppId) > > , mIsBrowserElement(aIsBrowserElement) > >- , mAppId(aAppId) > >+ , mTriedBrowserInit(false) > > { > > printf("creating %d!\n", NS_IsMainThread()); > > } > (At some point we should remove those printfs) > > > > > nsresult > > TabChild::Init() > > { > >- nsCOMPtr<nsIWebBrowser> webBrowser = do_CreateInstance(NS_WEBBROWSER_CONTRACTID); > >- if (!webBrowser) { > >- NS_ERROR("Couldn't create a nsWebBrowser?"); > >- return NS_ERROR_FAILURE; > >- } > >+ nsCOMPtr<nsIWebBrowser> webBrowser = > >+ do_CreateInstance(NS_WEBBROWSER_CONTRACTID); > >+ if (!webBrowser) { > >+ NS_ERROR("Couldn't create a nsWebBrowser?"); > >+ return NS_ERROR_FAILURE; > >+ } > > > >- webBrowser->SetContainerWindow(this); > >- mWebNav = do_QueryInterface(webBrowser); > >- NS_ASSERTION(mWebNav, "nsWebBrowser doesn't implement nsIWebNavigation?"); > >+ webBrowser->SetContainerWindow(this); > >+ mWebNav = do_QueryInterface(webBrowser); > >+ NS_ASSERTION(mWebNav, "nsWebBrowser doesn't implement nsIWebNavigation?"); > > > >- nsCOMPtr<nsIDocShellTreeItem> docShellItem(do_QueryInterface(mWebNav)); > >- docShellItem->SetItemType(nsIDocShellTreeItem::typeContentWrapper); > >+ nsCOMPtr<nsIDocShellTreeItem> docShellItem(do_QueryInterface(mWebNav)); > >+ docShellItem->SetItemType(nsIDocShellTreeItem::typeContentWrapper); > > > >- nsCOMPtr<nsIObserverService> observerService = > >- do_GetService(NS_OBSERVERSERVICE_CONTRACTID); > >+ nsCOMPtr<nsIObserverService> observerService = > >+ do_GetService(NS_OBSERVERSERVICE_CONTRACTID); > This is somewhat odd. The whole file is for some reason using > non-Gecko-coding-style indentation. The local style is 4-space indent. Patches landed with 2-space indent, despite the modelines, which led to the mismatched style. Functions added which ignored the modeline are very hard to hack on in editors that honor the modeline. > But for this patch, I'd just remove coding style changes. > Fine. > > >@@ -274,22 +276,36 @@ protected: > > nsEventStatus DispatchWidgetEvent(nsGUIEvent& event); > > > > virtual PIndexedDBChild* AllocPIndexedDB(const nsCString& aASCIIOrigin, > > bool* /* aAllowed */); > > > > virtual bool DeallocPIndexedDB(PIndexedDBChild* aActor); > > > > >+ enum FrameScriptLoading { DONT_LOAD_SCRIPTS, DEFAULT_LOAD_SCRIPTS }; > >+ bool InitTabChildGlobal(FrameScriptLoading aScriptLoading=DEFAULT_LOAD_SCRIPTS); > Space before and after = Changed.

This is failing on try, on the in-process(!!!) dom/browser-element tests, apparently because of 10 INFO TEST-START | /tests/dom/browser-element/mochitest/test_browserElement_inproc_Alert.html [snip] JavaScript error: chrome://browser/content/tabbrowser.xml, line 1988: aTab is null but nothing else suspicious around. That means it must be the nsFrameMessageManager change, but I have absolutely no clue what the bug might be ... :/

Oh, you break data: url scripts

Or, hmm, maybe not.

Which is <method name="getBrowserForTab"> <parameter name="aTab"/> <body> <![CDATA[ return aTab.linkedBrowser;

(In reply to Olli Pettay [:smaug] from comment #7) > Oh, you break data: url scripts Feels like a GC / ownership problem.

Although yeah you're right, var script = 'data:,\ testState = 0; \ content.alert("Hello, world!"); \ testState = 1; \ '; looks pretty darned suspicious ...

Comment on attachment 651867 [details] [diff] [review] Refactor TabChild to allow pre-created instances, and then use a pre-created instance to pre-load and compile BrowserElementChild.js >@@ -851,17 +860,16 @@ nsFrameScriptExecutor::LoadFrameScriptIn > if (!scheme.EqualsLiteral("data")) { > nsFrameJSScriptExecutorHolder* holder = > new nsFrameJSScriptExecutorHolder(script); > // Root the object also for caching. > JS_AddNamedScriptRoot(mCx, &(holder->mScript), > "Cached message manager script"); > sCachedScripts->Put(aURL, holder); > } >- (void) JS_ExecuteScript(mCx, global, script, nullptr); Ah this part. Could become else { (void) JS_ExecuteScript(mCx, global, script, nullptr); }

Yep, it was data:! Thanks for the quick pointer. There's an OOP test failing now but it seems unrelated.

Reverting the fake show stuff made the failure go away :(. Now seeing another OOP failure with use-after-__delete__, but might be able to kill 763602 after all ...

This patch ended up being a barrel of monkeys. I fixed up three more bugs, going through try now. Will probably need review one of the bugfixes.

The first set of fixes here restored jlebar's FakeShow() hack, which is still necessary. Not requesting review on that. This fixes a problem in which TabChild still alive around XPCOM shutdown would not destroy their DOM window state properly. I don't know why this is happening with these patches, but it's a problem that could have happened without them.

Comment on attachment 655943 [details] [diff] [review] 781725, followup: don't register for UI-only events until we have rendering state, and always destroy the window on shutdown. I'm not too familiar with "cancel-default-pan-zoom" and "browser-zoom-to-rect" (and looks like those are pretty horrible notifications), but this change doesn't make the situation any worse.