Bug 1298219 - Don't fire oop-browser-crashed event if the browser has already flipped remoteness and moved on. r?billm draft
authorMike Conley <mconley@mozilla.com>
Wed, 31 Aug 2016 18:23:40 -0400
changeset 408834 ea86ab0f34f59da04532ec897772cbbb5a002d6c
parent 408802 4dd14de3fa420bf5d750ca7c969ace2a9a9931ac
child 408835 3fb52b2c0759a79dd4df14a18ff9c8946174e89f
push id28285
push usermconley@mozilla.com
push dateThu, 01 Sep 2016 17:14:49 +0000
reviewersbillm
bugs1298219
milestone51.0a1
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);
   }