Bug 391834, rearrange some popup dialog code to be simpler,r=smaug,patch mostly by gavin

dom/base/nsGlobalWindow.cpp

@@ -684,7 +684,7 @@ nsGlobalWindow::nsGlobalWindow(nsGlobalWindow *aOuterWindow)
     mCleanedUp(false),
     mCallCleanUpAfterModalDialogCloses(false),
     mDialogAbuseCount(0),
-    mDialogDisabled(false)
+    mStopAbuseDialogs(false)
 {
   nsLayoutStatics::AddRef();
 
@@ -2551,67 +2551,64 @@ nsGlobalWindow::PreHandleEvent(nsEventChainPreVisitor& aVisitor)
 }
 
 bool
-nsGlobalWindow::DialogOpenAttempted()
+nsGlobalWindow::DialogsAreBlocked(bool *aBeingAbused)
 {
+  *aBeingAbused = false;
+
   nsGlobalWindow *topWindow = GetScriptableTop();
   if (!topWindow) {
-    NS_ERROR("DialogOpenAttempted() called without a top window?");
+    NS_ERROR("DialogsAreBlocked() called without a top window?");
-
-    return false;
-  }
-
-  topWindow = topWindow->GetCurrentInnerWindowInternal();
-  if (!topWindow ||
-      topWindow->mLastDialogQuitTime.IsNull() ||
-      nsContentUtils::CallerHasUniversalXPConnect()) {
-    return false;
-  }
-
-  TimeDuration dialogDuration(TimeStamp::Now() -
-                              topWindow->mLastDialogQuitTime);
-
-  if (dialogDuration.ToSeconds() <
-        Preferences::GetInt("dom.successive_dialog_time_limit",
-                            SUCCESSIVE_DIALOG_TIME_LIMIT)) {
-    topWindow->mDialogAbuseCount++;
-
-    return (topWindow->GetPopupControlState() > openAllowed ||
-            topWindow->mDialogAbuseCount > MAX_DIALOG_COUNT);
-  }
-
-  topWindow->mDialogAbuseCount = 0;
-
-  return false;
-}
-
-bool
-nsGlobalWindow::AreDialogsBlocked()
-{
-  nsGlobalWindow *topWindow = GetScriptableTop();
-  if (!topWindow) {
-    NS_ASSERTION(!mDocShell, "AreDialogsBlocked() called without a top window?");
-
     return true;
   }
 
   topWindow = topWindow->GetCurrentInnerWindowInternal();
+  if (!topWindow) {
+    return true;
+  }
 
-  return !topWindow ||
+  *aBeingAbused = topWindow->DialogsAreBeingAbused();
-         (topWindow->mDialogDisabled &&
+
-          (topWindow->GetPopupControlState() > openAllowed ||
+  return topWindow->mStopAbuseDialogs && *aBeingAbused;
-           topWindow->mDialogAbuseCount >= MAX_DIALOG_COUNT));
 }
 
 bool
-nsGlobalWindow::ConfirmDialogAllowed()
+nsGlobalWindow::DialogsAreBeingAbused()
 {
-  FORWARD_TO_OUTER(ConfirmDialogAllowed, (), false);
+  NS_ASSERTION(GetScriptableTop() &&
+               GetScriptableTop()->GetCurrentInnerWindowInternal() == this,
+               "DialogsAreBeingAbused called with invalid window");
+            
+  if (mLastDialogQuitTime.IsNull() ||
+      nsContentUtils::CallerHasUniversalXPConnect()) {
+    return false;
+  }
+ 
+  TimeDuration dialogInterval(TimeStamp::Now() - mLastDialogQuitTime);
+  if (dialogInterval.ToSeconds() <
+      Preferences::GetInt("dom.successive_dialog_time_limit",
+                          DEFAULT_SUCCESSIVE_DIALOG_TIME_LIMIT)) {
+    mDialogAbuseCount++;
+
+    return GetPopupControlState() > openAllowed ||
+           mDialogAbuseCount > MAX_SUCCESSIVE_DIALOG_COUNT;
+  }
+
+  // Reset the abuse counter
+  mDialogAbuseCount = 0;
+
+  return false;
+}
+
+bool
+nsGlobalWindow::ConfirmDialogIfNeeded()
+{
+  FORWARD_TO_OUTER(ConfirmDialogIfNeeded, (), false);
 
   NS_ENSURE_TRUE(mDocShell, false);
   nsCOMPtr<nsIPromptService> promptSvc =
     do_GetService("@mozilla.org/embedcomp/prompt-service;1");
 
-  if (!DialogOpenAttempted() || !promptSvc) {
+  if (!promptSvc) {
     return true;
   }
 
@@ -2641,14 +2638,13 @@ nsGlobalWindow::PreventFurtherDialogs()
   nsGlobalWindow *topWindow = GetScriptableTop();
   if (!topWindow) {
     NS_ERROR("PreventFurtherDialogs() called without a top window?");
-
     return;
   }
 
   topWindow = topWindow->GetCurrentInnerWindowInternal();
-
+  if (topWindow) {
-  if (topWindow)
+    topWindow->mStopAbuseDialogs = true;
-    topWindow->mDialogDisabled = true;
+  }
 }
 
 nsresult
@@ -4792,12 +4788,10 @@ nsGlobalWindow::Alert(const nsAString& aString)
 {
   FORWARD_TO_OUTER(Alert, (aString), NS_ERROR_NOT_INITIALIZED);
 
-  if (AreDialogsBlocked())
+  bool needToPromptForAbuse;
+  if (DialogsAreBlocked(&needToPromptForAbuse)) {
     return NS_ERROR_NOT_AVAILABLE;
-
+  }
-  // We have to capture this now so as not to get confused with the
-  // popup state we push next
-  bool shouldEnableDisableDialog = DialogOpenAttempted();
 
   // Reset popup state while opening a modal dialog, and firing events
   // about the dialog, to prevent the current state from being active
@@ -4844,7 +4838,7 @@ nsGlobalWindow::Alert(const nsAString& aString)
   nsAutoSyncOperation sync(GetCurrentInnerWindowInternal() ? 
                              GetCurrentInnerWindowInternal()->mDoc :
                              nullptr);
-  if (shouldEnableDisableDialog) {
+  if (needToPromptForAbuse) {
     bool disallowDialog = false;
     nsXPIDLString label;
     nsContentUtils::GetLocalizedString(nsContentUtils::eCOMMON_DIALOG_PROPERTIES,
@@ -4866,12 +4860,10 @@ nsGlobalWindow::Confirm(const nsAString& aString, bool* aReturn)
 {
   FORWARD_TO_OUTER(Confirm, (aString, aReturn), NS_ERROR_NOT_INITIALIZED);
 
-  if (AreDialogsBlocked())
+  bool needToPromptForAbuse;
+  if (DialogsAreBlocked(&needToPromptForAbuse)) {
     return NS_ERROR_NOT_AVAILABLE;
-
+  }
-  // We have to capture this now so as not to get confused with the popup state
-  // we push next
-  bool shouldEnableDisableDialog = DialogOpenAttempted();
 
   // Reset popup state while opening a modal dialog, and firing events
   // about the dialog, to prevent the current state from being active
@@ -4913,7 +4905,7 @@ nsGlobalWindow::Confirm(const nsAString& aString, bool* aReturn)
   nsAutoSyncOperation sync(GetCurrentInnerWindowInternal() ? 
                              GetCurrentInnerWindowInternal()->mDoc :
                              nullptr);
-  if (shouldEnableDisableDialog) {
+  if (needToPromptForAbuse) {
     bool disallowDialog = false;
     nsXPIDLString label;
     nsContentUtils::GetLocalizedString(nsContentUtils::eCOMMON_DIALOG_PROPERTIES,
@@ -4939,12 +4931,10 @@ nsGlobalWindow::Prompt(const nsAString& aMessage, const nsAString& aInitial,
 
   SetDOMStringToNull(aReturn);
 
-  if (AreDialogsBlocked())
+  bool needToPromptForAbuse;
+  if (DialogsAreBlocked(&needToPromptForAbuse)) {
     return NS_ERROR_NOT_AVAILABLE;
-
+  }
-  // We have to capture this now so as not to get confused with the popup state
-  // we push next
-  bool shouldEnableDisableDialog = DialogOpenAttempted();
 
   // Reset popup state while opening a modal dialog, and firing events
   // about the dialog, to prevent the current state from being active
@@ -4987,7 +4977,7 @@ nsGlobalWindow::Prompt(const nsAString& aMessage, const nsAString& aInitial,
   bool disallowDialog = false;
 
   nsXPIDLString label;
-  if (shouldEnableDisableDialog) {
+  if (needToPromptForAbuse) {
     nsContentUtils::GetLocalizedString(nsContentUtils::eCOMMON_DIALOG_PROPERTIES,
                                        "ScriptDialogLabel", label);
   }
@@ -5258,8 +5248,14 @@ nsGlobalWindow::Print()
   if (Preferences::GetBool("dom.disable_window_print", false))
     return NS_ERROR_NOT_AVAILABLE;
 
-  if (AreDialogsBlocked() || !ConfirmDialogAllowed())
+  bool needToPromptForAbuse;
+  if (DialogsAreBlocked(&needToPromptForAbuse)) {
     return NS_ERROR_NOT_AVAILABLE;
+  }
+
+  if (needToPromptForAbuse && !ConfirmDialogIfNeeded()) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
 
   nsCOMPtr<nsIWebBrowserPrint> webBrowserPrint;
   if (NS_SUCCEEDED(GetInterface(NS_GET_IID(nsIWebBrowserPrint),
@@ -7190,8 +7186,14 @@ nsGlobalWindow::ShowModalDialog(const nsAString& aURI, nsIVariant *aArgs,
   // pending reflows.
   EnsureReflowFlushAndPaint();
 
-  if (AreDialogsBlocked() || !ConfirmDialogAllowed())
+  bool needToPromptForAbuse;
+  if (DialogsAreBlocked(&needToPromptForAbuse)) {
     return NS_ERROR_NOT_AVAILABLE;
+  }
+
+  if (needToPromptForAbuse && !ConfirmDialogIfNeeded()) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
 
   nsCOMPtr<nsIDOMWindow> dlgWin;
   nsAutoString options(NS_LITERAL_STRING("-moz-internal-modal=1,status=1"));

dom/base/nsGlobalWindow.h

@@ -74,11 +74,11 @@
 
 // Amount of time allowed between alert/prompt/confirm before enabling
 // the stop dialog checkbox.
-#define SUCCESSIVE_DIALOG_TIME_LIMIT 3 // 3 sec
+#define DEFAULT_SUCCESSIVE_DIALOG_TIME_LIMIT 3 // 3 sec
 
-// During click or mousedown events (and others, see nsDOMEvent) we allow modal
+// Maximum number of successive dialogs before we prompt users to disable
-// dialogs up to this limit, even if they were disabled.
+// dialogs for this window.
-#define MAX_DIALOG_COUNT 10
+#define MAX_SUCCESSIVE_DIALOG_COUNT 5
 
 // Idle fuzz time upper limit
 #define MAX_IDLE_FUZZ_TIME_MS 90000
@@ -438,19 +438,19 @@ public:
     return nullptr;
   }
 
-  // Call this when a modal dialog is about to be opened.  Returns
+  // Returns true if dialogs need to be prevented from appearings for this
-  // true if we've reached the state in this top level window where we
+  // window. beingAbused returns whether dialogs are being abused.
-  // ask the user if further dialogs should be blocked.
+  bool DialogsAreBlocked(bool *aBeingAbused);
-  bool DialogOpenAttempted();
 
-  // Returns true if dialogs have already been blocked for this
+  // Returns true if we've reached the state in this top level window where we
-  // window.
+  // ask the user if further dialogs should be blocked. This method must only
-  bool AreDialogsBlocked();
+  // be called on the scriptable top inner window.
+  bool DialogsAreBeingAbused();
 
-  // Ask the user if further dialogs should be blocked. This is used
+  // Ask the user if further dialogs should be blocked, if dialogs are currently
-  // in the cases where we have no modifiable UI to show, in that case
+  // being abused. This is used in the cases where we have no modifiable UI to
-  // we show a separate dialog when asking this question.
+  // show, in that case we show a separate dialog to ask this question.
-  bool ConfirmDialogAllowed();
+  bool ConfirmDialogIfNeeded();
 
   // Prevent further dialogs in this (top level) window
   void PreventFurtherDialogs();
@@ -1042,17 +1042,22 @@ protected:
 
   nsCOMPtr<nsIIDBFactory> mIndexedDB;
 
-  // In the case of a "trusted" dialog (@see PopupControlState), we
+  // This counts the number of windows that have been opened in rapid succession
-  // set this counter to ensure a max of MAX_DIALOG_LIMIT
+  // (i.e. within dom.successive_dialog_time_limit of each other). It is reset
+  // to 0 once a dialog is opened after dom.successive_dialog_time_limit seconds
+  // have elapsed without any other dialogs.
   PRUint32                      mDialogAbuseCount;
 
-  // This holds the time when the last modal dialog was shown, if two
+  // This holds the time when the last modal dialog was shown. If more than
-  // dialogs are shown within CONCURRENT_DIALOG_TIME_LIMIT the
+  // MAX_DIALOG_LIMIT dialogs are shown within the time span defined by
-  // checkbox is shown. In the case of ShowModalDialog another Confirm
+  // dom.successive_dialog_time_limit, we show a checkbox or confirmation prompt
-  // dialog will be shown, the result of the checkbox/confirm dialog
+  // to allow disabling of further dialogs from this window.
-  // will be stored in mDialogDisabled variable.
   TimeStamp                     mLastDialogQuitTime;
-  bool                          mDialogDisabled;
+
+  // This is set to true once the user has opted-in to preventing further
+  // dialogs for this window. Subsequent dialogs may still open if
+  // mDialogAbuseCount gets reset.
+  bool                          mStopAbuseDialogs;
 
   nsRefPtr<nsDOMMozURLProperty> mURLProperty;