Bug 1261842 - Refactor nsWindowWatcher to not have so much duplication. r?smaug draft
authorMike Conley <mconley@mozilla.com>
Fri, 15 Jul 2016 11:20:53 -0400
changeset 392020 756d1a613d7051b5a77c112e11e1a07a6917f2a2
parent 392019 c13edf06c5842d7c02ef22892e4e89c0b921eb98
child 392021 af5820e5f3258615a4cdd4cfcad17b05bee367b2
push id23919
push usermconley@mozilla.com
push dateFri, 22 Jul 2016 20:54:03 +0000
reviewerssmaug
bugs1261842
milestone50.0a1
Bug 1261842 - Refactor nsWindowWatcher to not have so much duplication. r?smaug MozReview-Commit-ID: 8ItRHmJxcLA
embedding/components/windowwatcher/nsWindowWatcher.cpp
embedding/components/windowwatcher/nsWindowWatcher.h
--- a/embedding/components/windowwatcher/nsWindowWatcher.cpp
+++ b/embedding/components/windowwatcher/nsWindowWatcher.cpp
@@ -489,16 +489,82 @@ CheckUserContextCompatibility(nsIDocShel
 }
 
 NS_IMETHODIMP
 nsWindowWatcher::OpenWindowWithoutParent(nsITabParent** aResult)
 {
   return OpenWindowWithTabParent(nullptr, EmptyCString(), true, 1.0f, aResult);
 }
 
+nsresult
+nsWindowWatcher::CreateChromeWindow(const nsACString& aFeatures,
+                                    nsIWebBrowserChrome* aParentChrome,
+                                    uint32_t aChromeFlags,
+                                    uint32_t aContextFlags,
+                                    nsITabParent* aOpeningTabParent,
+                                    nsIWebBrowserChrome** aResult)
+{
+  nsCOMPtr<nsIWindowCreator2> windowCreator2(do_QueryInterface(mWindowCreator));
+  if (NS_WARN_IF(!windowCreator2)) {
+    return NS_ERROR_UNEXPECTED;
+  }
+
+  // B2G multi-screen support. mozDisplayId is returned from the
+  // "display-changed" event, it is also platform-dependent.
+#ifdef MOZ_WIDGET_GONK
+  int retval = WinHasOption(aFeatures, "mozDisplayId", 0, nullptr);
+  windowCreator2->SetScreenId(retval);
+#endif
+
+  bool cancel = false;
+  nsCOMPtr<nsIWebBrowserChrome> newWindowChrome;
+  nsresult rv =
+    windowCreator2->CreateChromeWindow2(aParentChrome, aChromeFlags, aContextFlags,
+                                        aOpeningTabParent, &cancel,
+                                        getter_AddRefs(newWindowChrome));
+
+  if (NS_SUCCEEDED(rv) && cancel) {
+    newWindowChrome = nullptr;
+    return NS_ERROR_ABORT;
+  }
+
+  newWindowChrome.forget(aResult);
+  return NS_OK;
+}
+
+/**
+ * Disable persistence of size/position in popups (determined by
+ * determining whether the features parameter specifies width or height
+ * in any way). We consider any overriding of the window's size or position
+ * in the open call as disabling persistence of those attributes.
+ * Popup windows (which should not persist size or position) generally set
+ * the size.
+ *
+ * @param aFeatures
+ *        The features string that was used to open the window.
+ * @param aTreeOwner
+ *        The nsIDocShellTreeOwner of the newly opened window. If null,
+ *        this function is a no-op.
+ */
+void
+nsWindowWatcher::MaybeDisablePersistence(const nsACString& aFeatures,
+                                         nsIDocShellTreeOwner* aTreeOwner)
+{
+  if (!aTreeOwner) {
+    return;
+  }
+
+ // At the moment, the strings "height=" or "width=" never happen
+ // outside a size specification, so we can do this the Q&D way.
+  if (PL_strcasestr(aFeatures.BeginReading(), "width=") ||
+      PL_strcasestr(aFeatures.BeginReading(), "height=")) {
+    aTreeOwner->SetPersistence(false, false, false);
+  }
+}
+
 NS_IMETHODIMP
 nsWindowWatcher::OpenWindowWithTabParent(nsITabParent* aOpeningTabParent,
                                          const nsACString& aFeatures,
                                          bool aCalledFromJS,
                                          float aOpenerFullZoom,
                                          nsITabParent** aResult)
 {
   MOZ_ASSERT(XRE_IsParentProcess());
@@ -557,42 +623,27 @@ nsWindowWatcher::OpenWindowWithTabParent
   }
 
   uint32_t contextFlags = 0;
   if (parentWindowOuter->IsLoadingOrRunningTimeout()) {
     contextFlags |=
             nsIWindowCreator2::PARENT_IS_LOADING_OR_RUNNING_TIMEOUT;
   }
 
-  // B2G multi-screen support. mozDisplayId is returned from the
-  // "display-changed" event, it is also platform-dependent.
-#ifdef MOZ_WIDGET_GONK
-  int retval = WinHasOption(features, "mozDisplayId", 0, nullptr);
-  windowCreator2->SetScreenId(retval);
-#endif
-
   uint32_t chromeFlags = CalculateChromeFlagsForChild(aFeatures);
 
   // A content process has asked for a new window, which implies
   // that the new window will need to be remote.
   chromeFlags |= nsIWebBrowserChrome::CHROME_REMOTE_WINDOW;
 
-  bool cancel = false;
   nsCOMPtr<nsIWebBrowserChrome> parentChrome(do_GetInterface(parentTreeOwner));
   nsCOMPtr<nsIWebBrowserChrome> newWindowChrome;
 
-  nsresult rv =
-    windowCreator2->CreateChromeWindow2(parentChrome, chromeFlags, contextFlags,
-                                        aOpeningTabParent, &cancel,
-                                        getter_AddRefs(newWindowChrome));
-
-  if (NS_SUCCEEDED(rv) && cancel) {
-    newWindowChrome = nullptr; // just in case
-    return NS_ERROR_ABORT;
-  }
+  CreateChromeWindow(aFeatures, parentChrome, chromeFlags, contextFlags,
+                     aOpeningTabParent, getter_AddRefs(newWindowChrome));
 
   if (NS_WARN_IF(!newWindowChrome)) {
     return NS_ERROR_UNEXPECTED;
   }
 
   nsCOMPtr<nsIDocShellTreeItem> chromeTreeItem = do_GetInterface(newWindowChrome);
   if (NS_WARN_IF(!chromeTreeItem)) {
     return NS_ERROR_UNEXPECTED;
@@ -610,20 +661,17 @@ nsWindowWatcher::OpenWindowWithTabParent
   }
 
   chromeContext->SetPrivateBrowsing(isPrivateBrowsingWindow);
 
   // Tabs opened from a content process can only open new windows
   // that will also run with out-of-process tabs.
   chromeContext->SetRemoteTabs(true);
 
-  if (PL_strcasestr(aFeatures.BeginReading(), "width=") ||
-      PL_strcasestr(aFeatures.BeginReading(), "height=")) {
-    chromeTreeOwner->SetPersistence(false, false, false);
-  }
+  MaybeDisablePersistence(aFeatures, chromeTreeOwner);
 
   SizeSpec sizeSpec;
   CalcSizeSpec(aFeatures, sizeSpec);
   SizeOpenedWindow(chromeTreeOwner, parentWindowOuter, false, sizeSpec,
                    &aOpenerFullZoom);
 
   nsCOMPtr<nsITabParent> newTabParent;
   chromeTreeOwner->GetPrimaryTabParent(getter_AddRefs(newTabParent));
@@ -956,32 +1004,19 @@ nsWindowWatcher::OpenWindowInternal(mozI
           popupConditions = !isCallerChrome;
         }
 
         if (popupConditions) {
           contextFlags |=
             nsIWindowCreator2::PARENT_IS_LOADING_OR_RUNNING_TIMEOUT;
         }
 
-        // B2G multi-screen support. mozDisplayId is returned from the
-        // "display-changed" event, it is also platform-dependent.
-#ifdef MOZ_WIDGET_GONK
-        int retval = WinHasOption(features, "mozDisplayId", 0, nullptr);
-        windowCreator2->SetScreenId(retval);
-#endif
-
+        rv = CreateChromeWindow(features, parentChrome, chromeFlags, contextFlags,
+                                nullptr, getter_AddRefs(newChrome));
 
-        bool cancel = false;
-        rv = windowCreator2->CreateChromeWindow2(parentChrome, chromeFlags,
-                                                 contextFlags, nullptr,
-                                                 &cancel, getter_AddRefs(newChrome));
-        if (NS_SUCCEEDED(rv) && cancel) {
-          newChrome = 0; // just in case
-          rv = NS_ERROR_ABORT;
-        }
       } else {
         rv = mWindowCreator->CreateChromeWindow(parentChrome, chromeFlags,
                                                 getter_AddRefs(newChrome));
       }
 
       if (newChrome) {
         nsCOMPtr<nsIXULWindow> xulWin = do_GetInterface(newChrome);
         if (xulWin) {
@@ -1028,35 +1063,20 @@ nsWindowWatcher::OpenWindowInternal(mozI
     }
   }
 
   rv = ReadyOpenedDocShellItem(newDocShellItem, parentWindow, windowIsNew, aResult);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  /* disable persistence of size/position in popups (determined by
-     determining whether the features parameter specifies width or height
-     in any way). We consider any overriding of the window's size or position
-     in the open call as disabling persistence of those attributes.
-     Popup windows (which should not persist size or position) generally set
-     the size. */
   if (isNewToplevelWindow) {
-    /* at the moment, the strings "height=" or "width=" never happen
-       outside a size specification, so we can do this the Q&D way. */
-
-    if (PL_strcasestr(features.get(), "width=") ||
-        PL_strcasestr(features.get(), "height=")) {
-
-      nsCOMPtr<nsIDocShellTreeOwner> newTreeOwner;
-      newDocShellItem->GetTreeOwner(getter_AddRefs(newTreeOwner));
-      if (newTreeOwner) {
-        newTreeOwner->SetPersistence(false, false, false);
-      }
-    }
+    nsCOMPtr<nsIDocShellTreeOwner> newTreeOwner;
+    newDocShellItem->GetTreeOwner(getter_AddRefs(newTreeOwner));
+    MaybeDisablePersistence(features, newTreeOwner);
   }
 
   if ((aDialog || windowIsModalContentDialog) && aArgv) {
     // Set the args on the new window.
     nsCOMPtr<nsPIDOMWindowOuter> piwin(do_QueryInterface(*aResult));
     NS_ENSURE_TRUE(piwin, NS_ERROR_UNEXPECTED);
 
     rv = piwin->SetArguments(aArgv);
--- a/embedding/components/windowwatcher/nsWindowWatcher.h
+++ b/embedding/components/windowwatcher/nsWindowWatcher.h
@@ -113,16 +113,26 @@ protected:
                                const SizeSpec& aSizeSpec,
                                float* aOpenerFullZoom);
   static void GetWindowTreeItem(mozIDOMWindowProxy* aWindow,
                                 nsIDocShellTreeItem** aResult);
   static void GetWindowTreeOwner(nsPIDOMWindowOuter* aWindow,
                                  nsIDocShellTreeOwner** aResult);
 
 private:
+  nsresult CreateChromeWindow(const nsACString& aFeatures,
+                              nsIWebBrowserChrome* aParentChrome,
+                              uint32_t aChromeFlags,
+                              uint32_t aContextFlags,
+                              nsITabParent* aOpeningTabParent,
+                              nsIWebBrowserChrome** aResult);
+
+  void MaybeDisablePersistence(const nsACString& aFeatures,
+                               nsIDocShellTreeOwner* aTreeOwner);
+
   static uint32_t CalculateChromeFlagsHelper(uint32_t aInitialFlags,
                                              const nsACString& aFeatures,
                                              bool &presenceFlag,
                                              bool aDialog = false,
                                              bool aHasChromeParent = false,
                                              bool aChromeURL = false);
   static uint32_t EnsureFlagsSafeForContent(uint32_t aChromeFlags,
                                             bool aChromeURL = false);