Bug 1051995: Remove the mozdocommand listener when remote browsers are destroyed to avoid leaking. r=fabrice
☠☠ backed out by b7088613e4c7 ☠ ☠
authorKyle Huey <khuey@kylehuey.com>
Wed, 17 Sep 2014 17:06:28 -0700
changeset 206101 b030bb382b31903cbf8d20e2f598b414c6daaeca
parent 206100 2d5de8b63f6963154c52791c2023635b234c8dc8
child 206102 f6b1cf6fb33f3e704fbc37ac37c3a9c42fb46188
push id27512
push usercbook@mozilla.com
push dateFri, 19 Sep 2014 12:07:02 +0000
treeherdermozilla-central@c8dee1c9cc3d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfabrice
bugs1051995
milestone35.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 1051995: Remove the mozdocommand listener when remote browsers are destroyed to avoid leaking. r=fabrice
dom/browser-element/BrowserElementParent.jsm
dom/ipc/TabParent.cpp
--- a/dom/browser-element/BrowserElementParent.jsm
+++ b/dom/browser-element/BrowserElementParent.jsm
@@ -170,17 +170,17 @@ function BrowserElementParent(frameLoade
   }
 
   this._doCommandHandlerBinder = this._doCommandHandler.bind(this);
   this._frameElement.addEventListener('mozdocommand',
                                       this._doCommandHandlerBinder,
                                       /* useCapture = */ false,
                                       /* wantsUntrusted = */ false);
 
-  Services.obs.addObserver(this, 'ipc:browser-destroyed', /* ownsWeak = */ true);
+  Services.obs.addObserver(this, 'ipc:browser-destroyed', /* ownsWeak = */ false);
 
   this._window._browserElementParents.set(this, null);
 
   // Insert ourself into the prompt service.
   BrowserElementPromptService.mapFrameToBrowserElementParent(this._frameElement, this);
   if (!isPendingFrame) {
     this._setupMessageListener();
     this._registerAppManifest();
@@ -914,28 +914,29 @@ BrowserElementParent.prototype = {
     case 'ask-children-to-exit-fullscreen':
       if (this._isAlive() &&
           this._frameElement.ownerDocument == subject &&
           this._hasRemoteFrame) {
         this._sendAsyncMsg('exit-fullscreen');
       }
       break;
     case 'remote-browser-frame-shown':
+      // XXXkhuey why don't we check _isAlive() here?
       if (this._frameLoader == subject) {
         if (!this._mm) {
           this._setupMessageListener();
           this._registerAppManifest();
         }
         Services.obs.removeObserver(this, 'remote-browser-frame-shown');
       }
     case 'ipc:browser-destroyed':
-      if (this._isAlive() && subject == this._frameLoader) {
+      if (this._isAlive() && subject == this._frameLoader.tabParent) {
         Services.obs.removeObserver(this, 'ipc:browser-destroyed');
         this._frameElement.removeEventListener('mozdocommand',
-                                               this._doCommandHandlerBinder)
+                                               this._doCommandHandlerBinder);
       }
       break;
     default:
       debug('Unknown topic: ' + topic);
       break;
     };
   },
 };
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -333,38 +333,39 @@ void
 TabParent::ActorDestroy(ActorDestroyReason why)
 {
   if (sEventCapturer == this) {
     sEventCapturer = nullptr;
   }
   if (mIMETabParent == this) {
     mIMETabParent = nullptr;
   }
+
+  nsCOMPtr<nsIObserverService> os = services::GetObserverService();
+  MOZ_ASSERT(os);
+  os->NotifyObservers(NS_ISUPPORTS_CAST(nsITabParent*, this), "ipc:browser-destroyed", nullptr);
+
   nsRefPtr<nsFrameLoader> frameLoader = GetFrameLoader();
-  nsCOMPtr<nsIObserverService> os = services::GetObserverService();
   nsRefPtr<nsFrameMessageManager> fmm;
   if (frameLoader) {
     fmm = frameLoader->GetFrameMessageManager();
     nsCOMPtr<Element> frameElement(mFrameElement);
     ReceiveMessage(CHILD_PROCESS_SHUTDOWN_MESSAGE, false, nullptr, nullptr,
                    nullptr);
     frameLoader->DestroyChild();
 
-    if (why == AbnormalShutdown && os) {
+    if (why == AbnormalShutdown) {
       os->NotifyObservers(NS_ISUPPORTS_CAST(nsIFrameLoader*, frameLoader),
                           "oop-frameloader-crashed", nullptr);
       nsContentUtils::DispatchTrustedEvent(frameElement->OwnerDoc(), frameElement,
                                            NS_LITERAL_STRING("oop-browser-crashed"),
                                            true, true);
     }
   }
 
-  if (os) {
-    os->NotifyObservers(NS_ISUPPORTS_CAST(nsITabParent*, this), "ipc:browser-destroyed", nullptr);
-  }
   if (fmm) {
     fmm->Disconnect();
   }
 }
 
 bool
 TabParent::RecvMoveFocus(const bool& aForward)
 {