Bug 1269036 - Never call PBrowserChild::Send__delete__ in content process. r=smaug a=lizzard
authorKan-Ru Chen <kanru@kanru.info>
Wed, 24 Aug 2016 18:49:42 +0800
changeset 342585 b1450549780a12c3220f64c172f2a80a62a7f9b1
parent 342584 642545b6e7b918733c9b7c1f4b2492f7993e831c
child 342586 d880cbd65bcd0b4345f18f9f3ad86cc3622843ec
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, lizzard
bugs1269036
milestone49.0
Bug 1269036 - Never call PBrowserChild::Send__delete__ in content process. r=smaug a=lizzard Automatically destroy TabParent if *aResult is not NS_OK or *aWindowIsNew is false. We should never call PBrowserChild::Send__delete__ directly in content process because the parent side needs to do some cleanup first. In this case if OpenWindowWithTabParent failed but the TabParent has been associated with a nsFrameLoader we could crash on trying to destroy a already destroyed TabParent. MozReview-Commit-ID: E2KFn6yA1Fm
dom/ipc/ContentChild.cpp
dom/ipc/ContentParent.cpp
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -928,23 +928,21 @@ ContentChild::ProvideWindowCommon(TabChi
                           &textureFactoryIdentifier,
                           &layersId)) {
       PRenderFrameChild::Send__delete__(renderFrame);
       return NS_ERROR_NOT_AVAILABLE;
     }
 
     if (NS_FAILED(rv)) {
       PRenderFrameChild::Send__delete__(renderFrame);
-      PBrowserChild::Send__delete__(newChild);
       return rv;
     }
   }
   if (!*aWindowIsNew) {
     PRenderFrameChild::Send__delete__(renderFrame);
-    PBrowserChild::Send__delete__(newChild);
     return NS_ERROR_ABORT;
   }
 
   if (layersId == 0) { // if renderFrame is invalid.
     PRenderFrameChild::Send__delete__(renderFrame);
     renderFrame = nullptr;
   }
 
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -97,16 +97,17 @@
 #include "mozilla/net/NeckoParent.h"
 #include "mozilla/plugins/PluginBridge.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/ProcessHangMonitor.h"
 #include "mozilla/ProcessHangMonitorIPC.h"
 #ifdef MOZ_ENABLE_PROFILER_SPS
 #include "mozilla/ProfileGatherer.h"
 #endif
+#include "mozilla/ScopeExit.h"
 #include "mozilla/Services.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/WebBrowserPersistDocumentParent.h"
 #include "mozilla/unused.h"
 #include "nsAnonymousTemporaryFile.h"
 #include "nsAppRunner.h"
 #include "nsAutoPtr.h"
@@ -5468,16 +5469,24 @@ ContentParent::RecvCreateWindow(PBrowser
 
   if (NS_WARN_IF(NS_FAILED(*aResult))) {
     return true;
   }
 
   TabParent* newTab = TabParent::GetFrom(aNewTab);
   MOZ_ASSERT(newTab);
 
+  auto destroyNewTabOnError = MakeScopeExit([&] {
+    if (!*aWindowIsNew || NS_FAILED(*aResult)) {
+      if (newTab) {
+        newTab->Destroy();
+      }
+    }
+  });
+
   // Content has requested that we open this new content window, so
   // we must have an opener.
   newTab->SetHasContentOpener(true);
 
   nsCOMPtr<nsIContent> frame;
   if (thisTabParent) {
     frame = do_QueryInterface(thisTabParent->GetOwnerElement());
   }