Unobserve the host WebContents of an ExtensionHost on destruction, to prevent

extensions/browser/extension_host.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/browser/extension_host.h"
 
 #include <list>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 103 matching lines @@
 // ExtensionHost
 
 ExtensionHost::ExtensionHost(const Extension* extension,
                              SiteInstance* site_instance,
                              const GURL& url,
                              ViewType host_type)
     : delegate_(ExtensionsBrowserClient::Get()->CreateExtensionHostDelegate()),
       extension_(extension),
       extension_id_(extension->id()),
       browser_context_(site_instance->GetBrowserContext()),
-      render_view_host_(NULL),
+      render_view_host_(nullptr),
       did_stop_loading_(false),
       document_element_available_(false),
       initial_url_(url),
       extension_function_dispatcher_(browser_context_, this),
       extension_host_type_(host_type) {
   // Not used for panels, see PanelHost.
   DCHECK(host_type == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE ||
          host_type == VIEW_TYPE_EXTENSION_DIALOG ||
          host_type == VIEW_TYPE_EXTENSION_POPUP);
   host_contents_.reset(WebContents::Create(
@@ skipping 21 matching lines @@
     UMA_HISTOGRAM_LONG_TIMES("Extensions.EventPageActiveTime2",
                              load_start_->Elapsed());
   }
   content::NotificationService::current()->Notify(
       extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
       content::Source<BrowserContext>(browser_context_),
       content::Details<ExtensionHost>(this));
   FOR_EACH_OBSERVER(ExtensionHostObserver, observer_list_,
                     OnExtensionHostDestroyed(this));
   ProcessCreationQueue::GetInstance()->Remove(this);
+  // Immediately stop observing |host_contents_| because its destruction events
+  // (like DidStopLoading, it turns out) can call back into ExtensionHost
+  // re-entrantly, when anything declared after |host_contents_| has already
+  // been destroyed.
+  content::WebContentsObserver::Observe(nullptr);
 }
 
 content::RenderProcessHost* ExtensionHost::render_process_host() const {
   return render_view_host()->GetProcess();
 }
 
 RenderViewHost* ExtensionHost::render_view_host() const {
-  // TODO(mpcomplete): This can be NULL. How do we handle that?
+  // TODO(mpcomplete): This can be null. How do we handle that?
   return render_view_host_;
 }
 
 bool ExtensionHost::IsRenderViewLive() const {
   return render_view_host()->IsRenderViewLive();
 }
 
 void ExtensionHost::CreateRenderViewSoon() {
   if (render_process_host() && render_process_host()->HasConnection()) {
     // If the process is already started, go ahead and initialize the RenderView
@@ skipping 71 matching lines @@
   DCHECK(extension_host_type_ == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE);
   return true;
 }
 
 void ExtensionHost::Observe(int type,
                             const content::NotificationSource& source,
                             const content::NotificationDetails& details) {
   switch (type) {
     case extensions::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED:
       // The extension object will be deleted after this notification has been
-      // sent. NULL it out so that dirty pointer issues don't arise in cases
+      // sent. Null it out so that dirty pointer issues don't arise in cases
       // when multiple ExtensionHost objects pointing to the same Extension are
       // present.
       if (extension_ == content::Details<UnloadedExtensionInfo>(details)->
           extension) {
-        extension_ = NULL;
+        extension_ = nullptr;
       }
       break;
     default:
       NOTREACHED() << "Unexpected notification sent.";
       break;
   }
 }
 
 void ExtensionHost::RenderProcessGone(base::TerminationStatus status) {
   // During browser shutdown, we may use sudden termination on an extension
   // process, so it is expected to lose our connection to the render view.
   // Do nothing.
   RenderProcessHost* process_host = host_contents_->GetRenderProcessHost();
   if (process_host && process_host->FastShutdownStarted())
     return;
 
   // In certain cases, multiple ExtensionHost objects may have pointed to
   // the same Extension at some point (one with a background page and a
   // popup, for example). When the first ExtensionHost goes away, the extension
   // is unloaded, and any other host that pointed to that extension will have
-  // its pointer to it NULLed out so that any attempt to unload a dirty pointer
+  // its pointer to it null'd out so that any attempt to unload a dirty pointer
   // will be averted.
   if (!extension_)
     return;
 
   // TODO(aa): This is suspicious. There can be multiple views in an extension,
   // and they aren't all going to use ExtensionHost. This should be in someplace
   // more central, like EPM maybe.
   content::NotificationService::current()->Notify(
       extensions::NOTIFICATION_EXTENSION_PROCESS_TERMINATED,
       content::Source<BrowserContext>(browser_context_),
       content::Details<ExtensionHost>(this));
 }
 
 void ExtensionHost::DidStopLoading(content::RenderViewHost* render_view_host) {
   bool notify = !did_stop_loading_;
   did_stop_loading_ = true;
   OnDidStopLoading();
   if (notify) {
-    // Log metrics. It's tempting to CHECK(load_start_) here, but it's possible
-    // to get a DidStopLoading even if we never started loading, in convoluted
-    // notification and observer chains.
-    if (load_start_.get()) {
-      if (extension_host_type_ == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
-        if (extension_ && BackgroundInfo::HasLazyBackgroundPage(extension_)) {
-          UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.EventPageLoadTime2",
-                                     load_start_->Elapsed());
-        } else {
-          UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.BackgroundPageLoadTime2",
-                                     load_start_->Elapsed());
-        }
-      } else if (extension_host_type_ == VIEW_TYPE_EXTENSION_POPUP) {
-        UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.PopupLoadTime2",
+    CHECK(load_start_.get());
+    if (extension_host_type_ == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
+      if (extension_ && BackgroundInfo::HasLazyBackgroundPage(extension_)) {
+        UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.EventPageLoadTime2",
+                                   load_start_->Elapsed());
+      } else {
+        UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.BackgroundPageLoadTime2",
                                    load_start_->Elapsed());
       }
+    } else if (extension_host_type_ == VIEW_TYPE_EXTENSION_POPUP) {
+      UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.PopupLoadTime2",
+                                 load_start_->Elapsed());
     }
-
     // Send the notification last, because it might result in this being
     // deleted.
     content::NotificationService::current()->Notify(
         extensions::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
         content::Source<BrowserContext>(browser_context_),
         content::Details<ExtensionHost>(this));
   }
 }
 
 void ExtensionHost::OnDidStopLoading() {
@@ skipping 146 matching lines @@
   return delegate_->CheckMediaAccessPermission(
       web_contents, security_origin, type, extension());
 }
 
 bool ExtensionHost::IsNeverVisible(content::WebContents* web_contents) {
   ViewType view_type = extensions::GetViewType(web_contents);
   return view_type == extensions::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE;
 }
 
 }  // namespace extensions