Bug 1267339 part 2. Push maintenance of the popup spam count down into the window watcher. r=mconley

dom/base/nsGlobalWindow.cpp

@@ -1502,9 +1502,20 @@ void
 nsGlobalWindow::MaybeForgiveSpamCount()
 {
   if (IsOuterWindow() &&
-      IsPopupSpamWindow())
+      IsPopupSpamWindow()) {
-  {
+    SetIsPopupSpamWindow(false);
-    SetPopupSpamWindow(false);
+  }
+}
+
+void
+nsGlobalWindow::SetIsPopupSpamWindow(bool aIsPopupSpam)
+{
+  MOZ_ASSERT(IsOuterWindow());
+
+  mIsPopupSpam = aIsPopupSpam;
+  if (aIsPopupSpam) {
+    ++gOpenPopupSpamCount;
+  } else {
     --gOpenPopupSpamCount;
     NS_ASSERTION(gOpenPopupSpamCount >= 0,
                  "Unbalanced decrement of gOpenPopupSpamCount");
@@ -11884,6 +11895,13 @@ nsGlobalWindow::OpenInternal(const nsAString& aUrl, const nsAString& aName,
   nsCOMPtr<nsPIWindowWatcher> pwwatch(do_QueryInterface(wwatch));
   NS_ENSURE_STATE(pwwatch);
 
+  MOZ_ASSERT_IF(checkForPopup, abuseLevel < openAbused);
+  // At this point we should know for a fact that if checkForPopup then
+  // abuseLevel < openAbused, so we could just check for abuseLevel ==
+  // openControlled.  But let's be defensive just in case and treat anything
+  // that fails the above assert as a spam popup too, if it ever happens.
+  bool isPopupSpamWindow = checkForPopup && (abuseLevel >= openControlled);
+
   {
     // Reset popup state while opening a window to prevent the
     // current state from being active the whole time a modal
@@ -11896,6 +11914,7 @@ nsGlobalWindow::OpenInternal(const nsAString& aUrl, const nsAString& aName,
       rv = pwwatch->OpenWindow2(AsOuter(), url.get(), name_ptr,
                                 options_ptr, /* aCalledFromScript = */ true,
                                 aDialog, aNavigate, argv,
+                                isPopupSpamWindow,
                                 getter_AddRefs(domReturn));
     } else {
       // Force a system caller here so that the window watcher won't screw us
@@ -11912,10 +11931,10 @@ nsGlobalWindow::OpenInternal(const nsAString& aUrl, const nsAString& aName,
         nojsapi.emplace();
       }
 
-
       rv = pwwatch->OpenWindow2(AsOuter(), url.get(), name_ptr,
                                 options_ptr, /* aCalledFromScript = */ false,
                                 aDialog, aNavigate, aExtraArgument,
+                                isPopupSpamWindow,
                                 getter_AddRefs(domReturn));
 
     }
@@ -11946,18 +11965,6 @@ nsGlobalWindow::OpenInternal(const nsAString& aUrl, const nsAString& aName,
     }
   }
 
-  if (checkForPopup) {
-    MOZ_ASSERT(abuseLevel < openAbused, "Why didn't we take the early return?");
-
-    if (abuseLevel >= openControlled) {
-      nsGlobalWindow *opened = nsGlobalWindow::Cast(*aReturn);
-      if (!opened->IsPopupSpamWindow()) {
-        opened->SetPopupSpamWindow(true);
-        ++gOpenPopupSpamCount;
-      }
-    }
-  }
-
   return rv;
 }
 

dom/base/nsGlobalWindow.h

@@ -1407,6 +1407,7 @@ protected:
   // Get the parent, returns null if this is a toplevel window
   nsPIDOMWindowOuter* GetParentInternal();
 
+public:
   // popup tracking
   bool IsPopupSpamWindow()
   {
@@ -1417,17 +1418,10 @@ protected:
     return GetOuterWindowInternal()->mIsPopupSpam;
   }
 
-  void SetPopupSpamWindow(bool aPopup)
+  // Outer windows only.
-  {
+  void SetIsPopupSpamWindow(bool aIsPopupSpam);
-    if (IsInnerWindow() && !mOuterWindow) {
-      NS_ERROR("SetPopupSpamWindow() called on inner window w/o an outer!");
-
-      return;
-    }
-
-    GetOuterWindowInternal()->mIsPopupSpam = aPopup;
-  }
 
+protected:
   // Window Control Functions
 
   // Outer windows only.

dom/workers/ServiceWorkerClients.cpp

@@ -607,6 +607,8 @@ private:
                            nullptr,
                            nullptr,
                            false, false, true, nullptr,
+                           // Not a spammy popup; we got permission, we swear!
+                           /* aIsPopupSpam = */ false,
                            getter_AddRefs(newWindow));
       nsCOMPtr<nsPIDOMWindowOuter> pwindow = nsPIDOMWindowOuter::From(newWindow);
       pwindow.forget(aWindow);

embedding/components/windowwatcher/nsPIWindowWatcher.idl

@@ -55,6 +55,8 @@ interface nsPIWindowWatcher : nsISupports
       @param aNavigate true if we should navigate the new window to the
              specified URL.
       @param aArgs Window argument
+      @param aIsPopupSpam true if the window is a popup spam window; used for
+                          popup blocker internals.
       @return the new window
 
       @note This method may examine the JS context stack for purposes of
@@ -70,7 +72,8 @@ interface nsPIWindowWatcher : nsISupports
                                  in boolean aCalledFromScript,
                                  in boolean aDialog,
                                  in boolean aNavigate,
-                                 in nsISupports aArgs);
+                                 in nsISupports aArgs,
+                                 in boolean aIsPopupSpam);
 
   /**
    * Opens a new window using the most recent non-private browser

embedding/components/windowwatcher/nsWindowWatcher.cpp

@@ -19,6 +19,7 @@
 #include "plstr.h"
 
 #include "nsDocShell.h"
+#include "nsGlobalWindow.h"
 #include "nsIBaseWindow.h"
 #include "nsIBrowserDOMWindow.h"
 #include "nsIDocShell.h"
@@ -371,7 +372,9 @@ nsWindowWatcher::OpenWindow(mozIDOMWindowProxy* aParent,
 
   return OpenWindowInternal(aParent, aUrl, aName, aFeatures,
                             /* calledFromJS = */ false, dialog,
-                            /* navigate = */ true, argv, aResult);
+                            /* navigate = */ true, argv,
+                            /* aIsPopupSpam = */ false,
+                            aResult);
 }
 
 struct SizeSpec
@@ -434,6 +437,7 @@ nsWindowWatcher::OpenWindow2(mozIDOMWindowProxy* aParent,
                              bool aDialog,
                              bool aNavigate,
                              nsISupports* aArguments,
+                             bool aIsPopupSpam,
                              mozIDOMWindowProxy** aResult)
 {
   nsCOMPtr<nsIArray> argv = ConvertArgsToArray(aArguments);
@@ -453,7 +457,7 @@ nsWindowWatcher::OpenWindow2(mozIDOMWindowProxy* aParent,
 
   return OpenWindowInternal(aParent, aUrl, aName, aFeatures,
                             aCalledFromScript, dialog,
-                            aNavigate, argv,
+                            aNavigate, argv, aIsPopupSpam,
                             aResult);
 }
 
@@ -690,6 +694,7 @@ nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy* aParent,
                                     bool aDialog,
                                     bool aNavigate,
                                     nsIArray* aArgv,
+                                    bool aIsPopupSpam,
                                     mozIDOMWindowProxy** aResult)
 {
   nsresult rv = NS_OK;
@@ -1133,6 +1138,16 @@ nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy* aParent,
     // SetInitialPrincipalToSubject is safe to call multiple times.
     if (newWindow) {
       newWindow->SetInitialPrincipalToSubject();
+      if (aIsPopupSpam) {
+        nsGlobalWindow* globalWin = nsGlobalWindow::Cast(newWindow);
+        MOZ_ASSERT(!globalWin->IsPopupSpamWindow(),
+                   "Who marked it as popup spam already???");
+        if (!globalWin->IsPopupSpamWindow()) { // Make sure we don't mess up our
+                                               // counter even if the above
+                                               // assert fails.
+          globalWin->SetIsPopupSpamWindow(true);
+        }
+      }
     }
   }
 

embedding/components/windowwatcher/nsWindowWatcher.h

@@ -84,6 +84,7 @@ protected:
                               bool aDialog,
                               bool aNavigate,
                               nsIArray* aArgv,
+                              bool aIsPopupSpam,
                               mozIDOMWindowProxy** aResult);
 
   static nsresult URIfromURL(const char* aURL,