Bug 1123090 - Pass URLs loaded during CreateWindow to content process (r=bent)

dom/ipc/PBrowser.ipdl

@@ -132,7 +132,7 @@ parent:
                       nsString aName,
                       nsString aFeatures,
                       nsString aBaseURI)
-      returns (bool windowOpened, FrameScriptInfo[] frameScripts);
+      returns (bool windowOpened, FrameScriptInfo[] frameScripts, nsCString urlToLoad);
 
     sync SyncMessage(nsString aMessage, ClonedMessageData aData,
                      CpowEntry[] aCpows, Principal aPrincipal)

dom/ipc/TabChild.cpp

@@ -1520,6 +1520,7 @@ TabChild::ProvideWindowCommon(nsIDOMWindow* aOpener,
   nsString name(aName);
   nsAutoCString features(aFeatures);
   nsTArray<FrameScriptInfo> frameScripts;
+  nsCString urlToLoad;
 
   if (aIframeMoz) {
     newChild->SendBrowserFrameOpenWindow(this, url, name,
@@ -1559,7 +1560,8 @@ TabChild::ProvideWindowCommon(nsIDOMWindow* aOpener,
                           name, NS_ConvertUTF8toUTF16(features),
                           NS_ConvertUTF8toUTF16(baseURIString),
                           aWindowIsNew,
-                          &frameScripts)) {
+                          &frameScripts,
+                          &urlToLoad)) {
       return NS_ERROR_NOT_AVAILABLE;
     }
   }
@@ -1592,6 +1594,10 @@ TabChild::ProvideWindowCommon(nsIDOMWindow* aOpener,
     }
   }
 
+  if (!urlToLoad.IsEmpty()) {
+    newChild->RecvLoadURL(urlToLoad);
+  }
+
   nsCOMPtr<nsIDOMWindow> win = do_GetInterface(newChild->WebNavigation());
   win.forget(aReturn);
   return NS_OK;

dom/ipc/TabParent.cpp

@@ -273,7 +273,7 @@ TabParent::TabParent(nsIContentParent* aManager,
   , mChromeFlags(aChromeFlags)
   , mInitedByParent(false)
   , mTabId(aTabId)
-  , mSkipLoad(false)
+  , mCreatingWindow(false)
 {
   MOZ_ASSERT(aManager);
 }
@@ -454,17 +454,21 @@ TabParent::RecvEvent(const RemoteDOMEvent& aEvent)
 struct MOZ_STACK_CLASS TabParent::AutoUseNewTab MOZ_FINAL
 {
 public:
-  AutoUseNewTab(TabParent* aNewTab, bool* aWindowIsNew)
-   : mNewTab(aNewTab), mWindowIsNew(aWindowIsNew)
+  AutoUseNewTab(TabParent* aNewTab, bool* aWindowIsNew, nsCString* aURLToLoad)
+   : mNewTab(aNewTab), mWindowIsNew(aWindowIsNew), mURLToLoad(aURLToLoad)
   {
     MOZ_ASSERT(!TabParent::sNextTabParent);
+    MOZ_ASSERT(!aNewTab->mCreatingWindow);
+
     TabParent::sNextTabParent = aNewTab;
-    aNewTab->mSkipLoad = true;
+    aNewTab->mCreatingWindow = true;
+    aNewTab->mDelayedURL.Truncate();
   }
 
   ~AutoUseNewTab()
   {
-    mNewTab->mSkipLoad = false;
+    mNewTab->mCreatingWindow = false;
+    *mURLToLoad = mNewTab->mDelayedURL;
 
     if (TabParent::sNextTabParent) {
       MOZ_ASSERT(TabParent::sNextTabParent == mNewTab);
@@ -476,6 +480,7 @@ public:
 private:
   TabParent* mNewTab;
   bool* mWindowIsNew;
+  nsCString* mURLToLoad;
 };
 
 bool
@@ -489,7 +494,8 @@ TabParent::RecvCreateWindow(PBrowserParent* aNewTab,
                             const nsString& aFeatures,
                             const nsString& aBaseURI,
                             bool* aWindowIsNew,
-                            InfallibleTArray<FrameScriptInfo>* aFrameScripts)
+                            InfallibleTArray<FrameScriptInfo>* aFrameScripts,
+                            nsCString* aURLToLoad)
 {
   // We always expect to open a new window here. If we don't, it's an error.
   *aWindowIsNew = true;
@@ -530,7 +536,7 @@ TabParent::RecvCreateWindow(PBrowserParent* aNewTab,
     params->SetReferrer(aBaseURI);
     params->SetIsPrivate(isPrivate);
 
-    AutoUseNewTab aunt(newTab, aWindowIsNew);
+    AutoUseNewTab aunt(newTab, aWindowIsNew, aURLToLoad);
 
     nsCOMPtr<nsIFrameLoaderOwner> frameLoaderOwner;
     mBrowserDOMWindow->OpenURIInFrame(nullptr, params,
@@ -564,7 +570,7 @@ TabParent::RecvCreateWindow(PBrowserParent* aNewTab,
 
   nsCOMPtr<nsIDOMWindow> window;
 
-  AutoUseNewTab aunt(newTab, aWindowIsNew);
+  AutoUseNewTab aunt(newTab, aWindowIsNew, aURLToLoad);
 
   rv = pwwatch->OpenWindow2(parent, finalURIString.get(),
                             NS_ConvertUTF16toUTF8(aName).get(),
@@ -601,7 +607,7 @@ bool
 TabParent::SendLoadRemoteScript(const nsString& aURL,
                                 const bool& aRunInGlobalScope)
 {
-  if (mSkipLoad) {
+  if (mCreatingWindow) {
     mDelayedFrameScripts.AppendElement(FrameScriptInfo(aURL, aRunInGlobalScope));
     return true;
   }
@@ -615,11 +621,6 @@ TabParent::LoadURL(nsIURI* aURI)
 {
     MOZ_ASSERT(aURI);
 
-    if (mSkipLoad) {
-        // Don't send the message if the child wants to load its own URL.
-        return;
-    }
-
     if (mIsDestroyed) {
         return;
     }
@@ -627,6 +628,13 @@ TabParent::LoadURL(nsIURI* aURI)
     nsCString spec;
     aURI->GetSpec(spec);
 
+    if (mCreatingWindow) {
+        // Don't send the message if the child wants to load its own URL.
+        MOZ_ASSERT(mDelayedURL.IsEmpty());
+        mDelayedURL = spec;
+        return;
+    }
+
     if (!mShown) {
         NS_WARNING(nsPrintfCString("TabParent::LoadURL(%s) called before "
                                    "Show(). Ignoring LoadURL.\n",

dom/ipc/TabParent.h

@@ -144,7 +144,8 @@ public:
                                   const nsString& aFeatures,
                                   const nsString& aBaseURI,
                                   bool* aWindowIsNew,
-                                  InfallibleTArray<FrameScriptInfo>* aFrameScripts) MOZ_OVERRIDE;
+                                  InfallibleTArray<FrameScriptInfo>* aFrameScripts,
+                                  nsCString* aURLToLoad) MOZ_OVERRIDE;
     virtual bool RecvSyncMessage(const nsString& aMessage,
                                  const ClonedMessageData& aData,
                                  InfallibleTArray<CpowEntry>&& aCpows,
@@ -489,14 +490,23 @@ private:
     static TabParent* sNextTabParent;
 
     // When loading a new tab or window via window.open, the child is
-    // responsible for loading the URL it wants into the new
-    // TabChild. Simultaneously, though, the parent sends a LoadURL message to
-    // every new PBrowser (usually for about:blank). This message usually
-    // arrives after the child has started to load the URL it wants, and
-    // overrides it. To prevent this, we set mSkipLoad to true when creating the
-    // new tab. This flag prevents the unwanted LoadURL message from being sent
-    // by the parent.
-    bool mSkipLoad;
+    // responsible for loading the URL it wants into the new TabChild. When the
+    // parent receives the CreateWindow message, though, it sends a LoadURL
+    // message, usually for about:blank. It's important for the about:blank load
+    // to get processed because the Firefox frontend expects every new window to
+    // immediately start loading something (see bug 1123090). However, we want
+    // the child to process the LoadURL message before it returns from
+    // ProvideWindow so that the URL sent from the parent doesn't override the
+    // child's URL. This is not possible using our IPC mechanisms. To solve the
+    // problem, we skip sending the LoadURL message in the parent and instead
+    // return the URL as a result from CreateWindow. The child simulates
+    // receiving a LoadURL message before returning from ProvideWindow.
+    //
+    // The mCreatingWindow flag is set while dispatching CreateWindow. During
+    // that time, any LoadURL calls are skipped and the URL is stored in
+    // mSkippedURL.
+    bool mCreatingWindow;
+    nsCString mDelayedURL;
 
     // When loading a new tab or window via window.open, we want to ensure that
     // frame scripts for that tab are loaded before any scripts start to run in