Fixed crash when chrome.runtime.reload() is called with app windows open

chrome/browser/extensions/extension_process_manager.cc

 // Copyright (c) 2012 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 "chrome/browser/extensions/extension_process_manager.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
@@ skipping 48 matching lines @@
 
 std::string GetExtensionID(RenderViewHost* render_view_host) {
   // This works for both apps and extensions because the site has been
   // normalized to the extension URL for apps.
   if (!render_view_host->GetSiteInstance())
     return std::string();
 
   return render_view_host->GetSiteInstance()->GetSiteURL().host();
 }
 
+void OnRenderViewHostUnregistered(Profile* profile,
+                                  RenderViewHost* render_view_host) {
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_EXTENSION_VIEW_UNREGISTERED,
+      content::Source<Profile>(profile),
+      content::Details<RenderViewHost>(render_view_host));
+}
+
 // Incognito profiles use this process manager. It is mostly a shim that decides
 // whether to fall back on the original profile's ExtensionProcessManager based
 // on whether a given extension uses "split" or "spanning" incognito behavior.
 class IncognitoExtensionProcessManager : public ExtensionProcessManager {
  public:
   explicit IncognitoExtensionProcessManager(Profile* profile);
   virtual ~IncognitoExtensionProcessManager();
   virtual ExtensionHost* CreateViewHost(
       const Extension* extension,
       const GURL& url,
@@ skipping 274 matching lines @@
   return service->extensions()->GetByID(GetExtensionID(render_view_host));
 }
 
 void ExtensionProcessManager::UnregisterRenderViewHost(
     RenderViewHost* render_view_host) {
   ExtensionRenderViews::iterator view =
       all_extension_views_.find(render_view_host);
   if (view == all_extension_views_.end())
     return;
 
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_EXTENSION_VIEW_UNREGISTERED,
-      content::Source<Profile>(GetProfile()),
-      content::Details<RenderViewHost>(render_view_host));
-
+  OnRenderViewHostUnregistered(GetProfile(), render_view_host);
   extensions::ViewType view_type = view->second;
   all_extension_views_.erase(view);
 
   // Keepalive count, balanced in RegisterRenderViewHost.
   if (view_type != extensions::VIEW_TYPE_INVALID &&
       view_type != extensions::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
     const Extension* extension = GetExtensionForRenderViewHost(
         render_view_host);
     if (extension)
       DecrementLazyKeepaliveCount(extension);
@@ skipping 222 matching lines @@
           content::Details<extensions::UnloadedExtensionInfo>(
               details)->extension;
       for (ExtensionHostSet::iterator iter = background_hosts_.begin();
            iter != background_hosts_.end(); ++iter) {
         ExtensionHost* host = *iter;
         if (host->extension_id() == extension->id()) {
           CloseBackgroundHost(host);
           break;
         }
       }
-      background_page_data_.erase(extension->id());
+      UnregisterExtension(extension->id());
       break;
     }
 
     case chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED: {
       ExtensionHost* host = content::Details<ExtensionHost>(details).ptr();
       if (background_hosts_.erase(host)) {
         ClearBackgroundPageData(host->extension()->id());
         background_page_data_[host->extension()->id()].since_suspended.reset(
             new PerfTimer());
       }
@@ skipping 145 matching lines @@
 }
 
 void ExtensionProcessManager::CloseBackgroundHosts() {
   for (ExtensionHostSet::iterator iter = background_hosts_.begin();
        iter != background_hosts_.end(); ) {
     ExtensionHostSet::iterator current = iter++;
     delete *current;
   }
 }
 
+void ExtensionProcessManager::UnregisterExtension(
+    const std::string& extension_id) {
+  // The lazy_keepalive_count may be greater than zero at this point because
+  // RenderViewHosts are still alive. During extension reloading, they will
+  // decrement the lazy_keepalive_count to negative for the new extension
+  // instance when they are destroyed. Since we are erasing the background page
+  // data for the unloaded extension, unregister the RenderViewHosts too.
+  Profile* profile = GetProfile();
+  for (ExtensionRenderViews::iterator it = all_extension_views_.begin();
+       it != all_extension_views_.end(); ) {
+    if (GetExtensionID(it->first) == extension_id) {
+      OnRenderViewHostUnregistered(profile, it->first);

[benwells, 2013/09/23 05:38:23]

Would this have been sent out before when unloading?

[tmdiep, 2013/09/23 05:49:00]

This gets sent when the render view process is destroyed - so it would have been
sent before when that old render view process is unregistered. If we don't
broadcast the notification when removing the RenderViewHost from
all_extension_views_, Chrome will crash badly with the Chrome task manager open.

+      it = all_extension_views_.erase(it);
+    } else {
+      ++it;
+    }
+  }
+
+  background_page_data_.erase(extension_id);
+}
+
 void ExtensionProcessManager::ClearBackgroundPageData(
     const std::string& extension_id) {
   background_page_data_.erase(extension_id);
 
   // Re-register all RenderViews for this extension. We do this to restore
   // the lazy_keepalive_count (if any) to properly reflect the number of open
   // views.
   for (ExtensionRenderViews::const_iterator it = all_extension_views_.begin();
        it != all_extension_views_.end(); ++it) {
     if (GetExtensionID(it->first) == extension_id)
@@ skipping 123 matching lines @@
         if (service && service->is_ready())
           CreateBackgroundHostsForProfileStartup();
       }
       break;
     }
     default:
       ExtensionProcessManager::Observe(type, source, details);
       break;
   }
 }