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

chrome/browser/extensions/extension_process_manager.cc

Index: chrome/browser/extensions/extension_process_manager.cc
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
index 1f8c9fb8e916a5a91aa6f6623354f9c3fbc8b8dc..87cc6077b443e8e22dfebb1c7bc60cd4a93b7dae 100644
--- a/chrome/browser/extensions/extension_process_manager.cc
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -395,6 +395,18 @@ void ExtensionProcessManager::RegisterRenderViewHost(
   IncrementLazyKeepaliveCountForView(render_view_host);
 }
 
+void ExtensionProcessManager::ReregisterRenderViewHosts(
+    const std::string& 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)
+      IncrementLazyKeepaliveCountForView(it->first);
+  }
+}
+
 SiteInstance* ExtensionProcessManager::GetSiteInstanceForURL(const GURL& url) {
   return site_instance_->GetRelatedSiteInstance(url);
 }
@@ -591,6 +603,10 @@ void ExtensionProcessManager::Observe(
         const Extension* extension =
             content::Details<const Extension>(details).ptr();
         CreateBackgroundHostForExtensionLoad(this, extension);
+
+        if (extension->is_platform_app() &&
+            service->IsBeingReloaded(extension->id()))
+          ReregisterRenderViewHosts(extension->id());

[benwells, 2013/09/20 07:35:31]

This makes me uncomfortable. The render view hosts will be destroyed very soon
which could make the lazy count go to zero and get unloaded. But worse than
that, they are really for a different extension which is now dead.

Can we instead clear all_extension_views_ when the extension is unloaded, or
will that have other side effects?

Unloading / reloading has a few flaky problems which need to be fixed. E.g.
https://code.google.com/p/chromium/issues/detail?id=174250

[tmdiep, 2013/09/20 11:07:36]

Incrementing the render view hosts actually prevents the lazy count going to
zero.  When the extension loading begins, the count is incremented (and later
decremented when the loading is complete). Without incrementing to account for
the "old" render views, they will decrement the count to zero when they are
deregistered and trigger the timeout for suspending the extension.  It doesn't
unload because an increment comes in soon due to some other network-related
activity and therefore cancels the suspension.
This sounds ok, but I'm not confident enough to say what side effects this will
have. We do need to fix this count somehow though.
I noticed.  I got the impression that the ExtensionService::ReloadExtension()
function is heavy-handed - it unloads, destroys everything and initiates
reloading synchronously.  Meanwhile there is a lot of code still lurking around
either with a dangling extension pointer or null pointer (and see a lot of usage
of extension objects which don't check for null).  

Just feels like the whole process needs to be analysed and sequenced properly,
rather than applying sporadic bandaids whenever we encounter a crash. My 2c :)

[benwells, 2013/09/20 11:22:47]

That was the conclusion I came to today as well. Let's talk about it on Monday.

       }
       break;
     }
@@ -776,15 +792,7 @@ void ExtensionProcessManager::CloseBackgroundHosts() {
 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)
-      IncrementLazyKeepaliveCountForView(it->first);
-  }
+  ReregisterRenderViewHosts(extension_id);
 }
 
 bool ExtensionProcessManager::DeferLoadingBackgroundHosts() const {