Bug 1269036 - Never call PBrowserChild::Send__delete__ in content process. r=smaug a=ritu
authorKan-Ru Chen <kanru@kanru.info>
Wed, 24 Aug 2016 18:49:42 +0800
changeset 347915 1b935d2bbf0b7dc4d58dd176e28f06bb177b558e
parent 347914 1bc4cc8271058198af1652b2fdf2445a0fdfabe5
child 347916 7968233f352e3bac51909671b1480269a72a5c47
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, ritu
bugs1269036
milestone50.0a2
Bug 1269036 - Never call PBrowserChild::Send__delete__ in content process. r=smaug a=ritu 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
@@ -878,23 +878,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"
@@ -5320,16 +5321,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());
   }