From 9c68071fa264bbe5bb941a1a156c8f56c23b72d5 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 20 Oct 2016 16:52:38 -0400 Subject: [PATCH] Bug 1267339 part 2. Push maintenance of the popup spam count down into the window watcher. r=mconley --- dom/base/nsGlobalWindow.cpp | 39 +++++++++++-------- dom/base/nsGlobalWindow.h | 14 ++----- dom/workers/ServiceWorkerClients.cpp | 2 + .../windowwatcher/nsPIWindowWatcher.idl | 5 ++- .../windowwatcher/nsWindowWatcher.cpp | 19 ++++++++- .../windowwatcher/nsWindowWatcher.h | 1 + 6 files changed, 51 insertions(+), 29 deletions(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 2d900ab8d5f1..a228352487b6 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -1502,9 +1502,20 @@ void nsGlobalWindow::MaybeForgiveSpamCount() { if (IsOuterWindow() && - IsPopupSpamWindow()) - { - SetPopupSpamWindow(false); + IsPopupSpamWindow()) { + SetIsPopupSpamWindow(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 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; } diff --git a/dom/base/nsGlobalWindow.h b/dom/base/nsGlobalWindow.h index 131a13e94a7b..15e87d321121 100644 --- a/dom/base/nsGlobalWindow.h +++ b/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) - { - if (IsInnerWindow() && !mOuterWindow) { - NS_ERROR("SetPopupSpamWindow() called on inner window w/o an outer!"); - - return; - } - - GetOuterWindowInternal()->mIsPopupSpam = aPopup; - } + // Outer windows only. + void SetIsPopupSpamWindow(bool aIsPopupSpam); +protected: // Window Control Functions // Outer windows only. diff --git a/dom/workers/ServiceWorkerClients.cpp b/dom/workers/ServiceWorkerClients.cpp index 565287943bf5..e98c472f6290 100644 --- a/dom/workers/ServiceWorkerClients.cpp +++ b/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 pwindow = nsPIDOMWindowOuter::From(newWindow); pwindow.forget(aWindow); diff --git a/embedding/components/windowwatcher/nsPIWindowWatcher.idl b/embedding/components/windowwatcher/nsPIWindowWatcher.idl index 825c80428f6a..d878bad6c8bb 100644 --- a/embedding/components/windowwatcher/nsPIWindowWatcher.idl +++ b/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 diff --git a/embedding/components/windowwatcher/nsWindowWatcher.cpp b/embedding/components/windowwatcher/nsWindowWatcher.cpp index 09bce49823be..647943d91bd2 100644 --- a/embedding/components/windowwatcher/nsWindowWatcher.cpp +++ b/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 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); + } + } } } diff --git a/embedding/components/windowwatcher/nsWindowWatcher.h b/embedding/components/windowwatcher/nsWindowWatcher.h index d5947b79d588..f70a6dfaff83 100644 --- a/embedding/components/windowwatcher/nsWindowWatcher.h +++ b/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,