Bug 1298219 - Don't fire oop-browser-crashed event if the browser has already flipped remoteness and moved on. r=billm
☠☠ backed out by 2e6b9e57d5a0 ☠ ☠
authorMike Conley <mconley@mozilla.com>
Wed, 31 Aug 2016 18:23:40 -0400
changeset 312985 5948ae1f421848c2b998494dad665e140b768fc1
parent 312984 fafc28ee1d5d1537c790c8d03f1ee34fce0f0204
child 312986 98b0e9b8821221843b17a1e191fae13a29e24018
push id81509
push usercbook@mozilla.com
push dateWed, 07 Sep 2016 15:23:10 +0000
treeherdermozilla-inbound@80dccdd8c94a [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);
   }