Bug 1298219 - Don't fire oop-browser-crashed event if the browser has already flipped remoteness and moved on. r=billm, a=ritu
authorMike Conley <mconley@mozilla.com>
Wed, 31 Aug 2016 18:23:40 -0400
changeset 348134 a484581afc4b7a204dc018ed1a435fcf56ade75f
parent 348133 961603912962c62f9715751518358edc82f7afdf
child 348135 5755a15244c83e1bb3bff68835e941e73fed0110
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)
reviewersbillm, ritu
bugs1298219
milestone50.0a2
Bug 1298219 - Don't fire oop-browser-crashed event if the browser has already flipped remoteness and moved on. r=billm, a=ritu 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
@@ -612,19 +612,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);
   }