Bug 1298219 - Don't fire oop-browser-crashed event if the browser has already flipped remoteness and moved on. r=billm
authorMike Conley <mconley@mozilla.com>
Wed, 31 Aug 2016 18:23:40 -0400
changeset 354169 804c98e6fef45ffd4cbaef194e94fec3b578068f
parent 354168 a093339f5b6181440201bcf747c9013da11c4bda
child 354170 bd4d11e10ee28439add0cbfbf7c99f25a662a82e
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)
reviewersbillm
bugs1298219
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 1298219 - Don't fire oop-browser-crashed event if the browser has already flipped remoteness and moved on. r=billm We currently make the initial browser in a window remote by default. If early on in the session, that one remote browser goes away (and the content process was still booting), there's about 5 seconds before the shutdown kill timer will take that content process out for not quitting fast enough. There are some cases during startup where the content process is waiting on information from the parent, so it cannot respond to the request to quit in time. The parents shutdown kill timer goes off, and the shutdown kill occurs. In this bug, what's happening is that the initial browser flips remoteness from remote to non-remote when it goes to about:sessionrestore. This starts the shutdown kill timer. The content process runs out of time, and the shutdown kill timer fires, killing the content process. The TabParent::ActorDestroy method (which still exists, even though the browser is no longer remote), interprets this as an abnormal shutdown, and bubbles the oop-browser-crashed event to the associated <xul:browser>, which causes the page to browser to about:tabcrashed, when it had already loaded about:sessionrestore. This patch makes it so that the TabParent::ActorDestroy method first checks to ensure that the associated remote frameloader is still the one that the frameloader owner cares about. If not (because, say, the remoteness has flipped and a new non-remote frameloader has been created), then the event is not fired, since the user has moved on. MozReview-Commit-ID: G4jmR6lMMFl
dom/ipc/TabParent.cpp
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -613,19 +613,29 @@ TabParent::ActorDestroy(ActorDestroyReas
     nsCOMPtr<Element> frameElement(mFrameElement);
     ReceiveMessage(CHILD_PROCESS_SHUTDOWN_MESSAGE, false, nullptr, nullptr,
                    nullptr);
     frameLoader->DestroyComplete();
 
     if (why == AbnormalShutdown && os) {
       os->NotifyObservers(NS_ISUPPORTS_CAST(nsIFrameLoader*, frameLoader),
                           "oop-frameloader-crashed", nullptr);
-      nsContentUtils::DispatchTrustedEvent(frameElement->OwnerDoc(), frameElement,
-                                           NS_LITERAL_STRING("oop-browser-crashed"),
-                                           true, true);
+      nsCOMPtr<nsIFrameLoaderOwner> owner = do_QueryInterface(frameElement);
+      if (owner) {
+        RefPtr<nsFrameLoader> currentFrameLoader = owner->GetFrameLoader();
+        // It's possible that the frameloader owner has already moved on
+        // and created a new frameloader. If so, we don't fire the event,
+        // since the frameloader owner has clearly moved on.
+        if (currentFrameLoader == frameLoader) {
+          nsContentUtils::DispatchTrustedEvent(frameElement->OwnerDoc(), frameElement,
+                                               NS_LITERAL_STRING("oop-browser-crashed"),
+                                               true, true);
+
+        }
+      }
     }
 
     mFrameLoader = nullptr;
   }
 
   if (os) {
     os->NotifyObservers(NS_ISUPPORTS_CAST(nsITabParent*, this), "ipc:browser-destroyed", nullptr);
   }