Fixing bug 260385. Prevent popups from opening from beforeunload events. r=danm.moz@gmail.com, sr=bzbarsky@mit.edu, a=asa@mozilla.org

content/base/src/nsDocumentViewer.cpp

@@ -963,15 +963,22 @@ DocumentViewerImpl::PermitUnload(PRBool *aPermitUnload)
   // to unload...
   nsEventStatus status = nsEventStatus_eIgnore;
   nsBeforePageUnloadEvent event(NS_BEFORE_PAGE_UNLOAD);
+  nsresult rv = NS_OK;
 
-  // In evil cases we might be destroyed while handling the
-  // onbeforeunload event, don't let that happen.
-  nsRefPtr<DocumentViewerImpl> kungFuDeathGrip(this);
+  {
+    // Never permit popups from the beforeunload handler, no matter
+    // how we get here.
+    nsAutoPopupStatePusher popupStatePusher(openAbused, PR_TRUE);
 
-  mInPermitUnload = PR_TRUE;
-  nsresult rv = global->HandleDOMEvent(mPresContext, &event, nsnull,
-                                       NS_EVENT_FLAG_INIT, &status);
-  mInPermitUnload = PR_FALSE;
+    // In evil cases we might be destroyed while handling the
+    // onbeforeunload event, don't let that happen.
+    nsRefPtr<DocumentViewerImpl> kungFuDeathGrip(this);
+
+    mInPermitUnload = PR_TRUE;
+    rv = global->HandleDOMEvent(mPresContext, &event, nsnull,
+                                NS_EVENT_FLAG_INIT, &status);
+    mInPermitUnload = PR_FALSE;
+  }
 
   if (NS_SUCCEEDED(rv) && event.flags & NS_EVENT_FLAG_NO_DEFAULT) {
     // Ask the user if it's ok to unload the current page
@@ -1061,6 +1068,10 @@ DocumentViewerImpl::Unload()
   nsEventStatus status = nsEventStatus_eIgnore;
   nsEvent event(NS_PAGE_UNLOAD);
 
+  // Never permit popups from the unload handler, no matter how we get
+  // here.
+  nsAutoPopupStatePusher popupStatePusher(openAbused, PR_TRUE);
+
   return global->HandleDOMEvent(mPresContext, &event, nsnull,
                                 NS_EVENT_FLAG_INIT, &status);
 }

dom/public/base/nsPIDOMWindow.h

@@ -136,7 +136,7 @@ protected:
 
 #ifdef _IMPL_NS_LAYOUT
 PopupControlState
-PushPopupControlState(PopupControlState aState);
+PushPopupControlState(PopupControlState aState, PRBool aForce);
 
 void
 PopPopupControlState(PopupControlState aState);
@@ -150,8 +150,8 @@ class nsAutoPopupStatePusher
 {
 public:
 #ifdef _IMPL_NS_LAYOUT
-  nsAutoPopupStatePusher(PopupControlState aState)
-    : mOldState(::PushPopupControlState(aState))
+  nsAutoPopupStatePusher(PopupControlState aState, PRBool aForce = PR_FALSE)
+    : mOldState(::PushPopupControlState(aState, aForce))
   {
   }
 

dom/src/base/nsGlobalWindow.cpp

@@ -389,11 +389,11 @@ GlobalWindowImpl::SetOpenerScriptURL(nsIURI* aURI)
 }
 
 PopupControlState
-PushPopupControlState(PopupControlState aState)
+PushPopupControlState(PopupControlState aState, PRBool aForce)
 {
   PopupControlState oldState = gPopupControlState;
 
-  if (aState < gPopupControlState) {
+  if (aState < gPopupControlState || aForce) {
     gPopupControlState = aState;
   }
 
@@ -409,7 +409,7 @@ PopPopupControlState(PopupControlState aState)
 PopupControlState
 GlobalWindowImpl::PushPopupControlState(PopupControlState aState) const
 {
-  return ::PushPopupControlState(aState);
+  return ::PushPopupControlState(aState, PR_FALSE);
 }
 
 void
@@ -2954,7 +2954,6 @@ PopupControlState
 GlobalWindowImpl::CheckForAbusePoint()
 {
   nsCOMPtr<nsIDocShellTreeItem> item(do_QueryInterface(mDocShell));
-  PRBool isBeingDestroyed = PR_FALSE;
 
   if (item) {
     PRInt32 type = nsIDocShellTreeItem::typeChrome;
@@ -2962,26 +2961,11 @@ GlobalWindowImpl::CheckForAbusePoint()
     item->GetItemType(&type);
     if (type != nsIDocShellTreeItem::typeContent)
       return openAllowed;
-
-    while (item) {
-      nsCOMPtr<nsIDocShell> docShell(do_QueryInterface(item));
-
-      docShell->IsBeingDestroyed(&isBeingDestroyed);
-
-      if (isBeingDestroyed) {
-        break;
-      }
-
-      nsIDocShellTreeItem *tmp = item;
-      tmp->GetParent(getter_AddRefs(item));
-    }
   }
 
   // level of abuse we've detected, initialized to the current popup
-  // state, except in the case where this window is being destroyed
-  // since then we override the current state and prevent popups
-  // alltogether.
-  PopupControlState abuse = isBeingDestroyed ? openAbused : gPopupControlState;
+  // state
+  PopupControlState abuse = gPopupControlState;
 
   // limit the number of simultaneously open popups
   if (abuse == openAbused || abuse == openControlled) {

layout/base/nsDocumentViewer.cpp

@@ -963,15 +963,22 @@ DocumentViewerImpl::PermitUnload(PRBool *aPermitUnload)
   // to unload...
   nsEventStatus status = nsEventStatus_eIgnore;
   nsBeforePageUnloadEvent event(NS_BEFORE_PAGE_UNLOAD);
+  nsresult rv = NS_OK;
 
-  // In evil cases we might be destroyed while handling the
-  // onbeforeunload event, don't let that happen.
-  nsRefPtr<DocumentViewerImpl> kungFuDeathGrip(this);
+  {
+    // Never permit popups from the beforeunload handler, no matter
+    // how we get here.
+    nsAutoPopupStatePusher popupStatePusher(openAbused, PR_TRUE);
 
-  mInPermitUnload = PR_TRUE;
-  nsresult rv = global->HandleDOMEvent(mPresContext, &event, nsnull,
-                                       NS_EVENT_FLAG_INIT, &status);
-  mInPermitUnload = PR_FALSE;
+    // In evil cases we might be destroyed while handling the
+    // onbeforeunload event, don't let that happen.
+    nsRefPtr<DocumentViewerImpl> kungFuDeathGrip(this);
+
+    mInPermitUnload = PR_TRUE;
+    rv = global->HandleDOMEvent(mPresContext, &event, nsnull,
+                                NS_EVENT_FLAG_INIT, &status);
+    mInPermitUnload = PR_FALSE;
+  }
 
   if (NS_SUCCEEDED(rv) && event.flags & NS_EVENT_FLAG_NO_DEFAULT) {
     // Ask the user if it's ok to unload the current page
@@ -1061,6 +1068,10 @@ DocumentViewerImpl::Unload()
   nsEventStatus status = nsEventStatus_eIgnore;
   nsEvent event(NS_PAGE_UNLOAD);
 
+  // Never permit popups from the unload handler, no matter how we get
+  // here.
+  nsAutoPopupStatePusher popupStatePusher(openAbused, PR_TRUE);
+
   return global->HandleDOMEvent(mPresContext, &event, nsnull,
                                 NS_EVENT_FLAG_INIT, &status);
 }