Bug 1269036 - Never call PBrowserChild::Send__delete__ in content process. r=smaug
authorKan-Ru Chen <kanru@kanru.info>
Wed, 24 Aug 2016 18:49:42 +0800
changeset 352349 7f1978918cb165cebd15024ba0b365f10f7cb2e5
parent 352348 9c50e31bc8b6393cdcf328a92b1ce6fbfd5459dd
child 352350 11c7856941a184465862501db844a194b01b6519
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1269036
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1269036 - Never call PBrowserChild::Send__delete__ in content process. r=smaug 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
@@ -805,23 +805,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
@@ -98,16 +98,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 "nsCDefaultURIFixup.h"
@@ -4999,16 +5000,24 @@ ContentParent::RecvCreateWindow(PBrowser
 
   if (NS_WARN_IF(thisTabParent && thisTabParent->IsMozBrowserOrApp())) {
     return false;
   }
 
   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());
   }