app_shell: Extract extension runtime data from ExtensionService

chrome/browser/extensions/extension_service.cc

Index: chrome/browser/extensions/extension_service.cc
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index b9c734071674fc1a52e7e0df6d7d737803cc8c26..56de6bf6ad669dba50bd0535af9f5ea0db2ea642 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -83,6 +83,7 @@
 #include "extensions/browser/pref_names.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/process_map.h"
+#include "extensions/browser/runtime_data.h"
 #include "extensions/browser/update_observer.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/error_utils.h"
@@ -189,15 +190,6 @@ class SharedModuleProvider : public extensions::ManagementPolicy::Provider {
 
 }  // namespace
 
-ExtensionService::ExtensionRuntimeData::ExtensionRuntimeData()
-    : background_page_ready(false),
-      being_upgraded(false),
-      has_used_webrequest(false) {
-}
-
-ExtensionService::ExtensionRuntimeData::~ExtensionRuntimeData() {
-}
-
 // ExtensionService.
 
 void ExtensionService::CheckExternalUninstall(const std::string& id) {
@@ -740,7 +732,7 @@ void ExtensionService::ReloadExtension(const std::string extension_id) {
 
     path = current_extension->path();
     // BeingUpgraded is set back to false when the extension is added.
-    SetBeingUpgraded(current_extension, true);
+    system_->runtime_data()->SetBeingUpgraded(current_extension, true);
     DisableExtension(extension_id, Extension::DISABLE_RELOAD);
     reloading_extensions_.insert(extension_id);
   } else {
@@ -979,19 +971,20 @@ void ExtensionService::DisableExtension(
   if (!extension)
     return;
 
-  // Reset the background_page_ready flag
-  if (extensions::BackgroundInfo::HasBackgroundPage(extension))
-    extension_runtime_data_[extension->id()].background_page_ready = false;
+  bool was_enabled = registry_->enabled_extensions().Contains(extension->id());
 
-  // Move it over to the disabled list. Don't send a second unload notification
-  // for terminated extensions being disabled.
+  // Move it over to the disabled list.
   registry_->AddDisabled(make_scoped_refptr(extension));
-  if (registry_->enabled_extensions().Contains(extension->id())) {
+  if (was_enabled)
     registry_->RemoveEnabled(extension->id());
-    NotifyExtensionUnloaded(extension, UnloadedExtensionInfo::REASON_DISABLE);
-  } else {
+  else
     registry_->RemoveTerminated(extension->id());
-  }
+  registry_->TriggerOnDisabled(extension);
+
+  // Only send the unload notification for enabled extensions to avoid sending
+  // a second notification for terminated extensions.
+  if (was_enabled)
+    NotifyExtensionUnloaded(extension, UnloadedExtensionInfo::REASON_DISABLE);
 
   if (extension_sync_service_)
     extension_sync_service_->SyncDisableExtension(*extension);
@@ -1590,18 +1583,23 @@ void ExtensionService::UnloadExtension(
   // Clean up if the extension is meant to be enabled after a reload.
   reloading_extensions_.erase(extension->id());
 
-  // Clean up runtime data.
-  extension_runtime_data_.erase(extension_id);
+  bool was_disabled =
+      registry_->disabled_extensions().Contains(extension->id());
 
-  if (registry_->disabled_extensions().Contains(extension->id())) {
+  // Remove the extension from the registry.
+  if (was_disabled)
     registry_->RemoveDisabled(extension->id());
+  else
+    registry_->RemoveEnabled(extension->id());
+  registry_->TriggerOnUnloaded(extension);

[not at google - send to devlin, 2014/01/22 16:17:43]

Should this go in NotifyExtensionUnloaded? That would actually change the
behaviour here, since it would only trigger the unloaded event on !was_disabled,
which given the old code is probably the right thing to do.

Hm, maybe I'm missing something here because it looks like you did explicitly
make that change; reverting it would also mean going basically back to the old
code just without that runtime_data reference.

[James Cook, 2014/01/22 17:41:52]

The code I wrote has identical behavior to the old code with respect to clearing
the runtime data and calling NotifyExtensionUnloaded. Are you saying I should
change when runtime data is cleared?

[not at google - send to devlin, 2014/01/22 17:49:07]

Yeah I'm questioning whether the old code is correct.

+
+  // Notify other subsystems that the extension is gone.
+  if (was_disabled) {
     // Make sure the profile cleans up its RequestContexts when an already
     // disabled extension is unloaded (since they are also tracking the disabled
     // extensions).
     system_->UnregisterExtensionWithRequestContexts(extension_id, reason);
   } else {
-    // Remove the extension from the enabled list.
-    registry_->RemoveEnabled(extension->id());
     NotifyExtensionUnloaded(extension.get(), reason);
   }
 
@@ -1721,7 +1719,7 @@ void ExtensionService::AddExtension(const Extension* extension) {
     if (!Manifest::IsUnpackedLocation(extension->location()))
       CHECK_GE(version_compare_result, 0);
   }
-  SetBeingUpgraded(extension, is_extension_upgrade);
+  system_->runtime_data()->SetBeingUpgraded(extension, is_extension_upgrade);
 
   // The extension is now loaded, remove its data from unloaded extension map.
   unloaded_extension_paths_.erase(extension->id());
@@ -1784,7 +1782,7 @@ void ExtensionService::AddExtension(const Extension* extension) {
       extension_sync_service_->SyncExtensionChangeIfNeeded(*extension);
     NotifyExtensionLoaded(extension);
   }
-  SetBeingUpgraded(extension, false);
+  system_->runtime_data()->SetBeingUpgraded(extension, false);
 }
 
 void ExtensionService::AddComponentExtension(const Extension* extension) {
@@ -2619,32 +2617,6 @@ ExtensionIdSet ExtensionService::GetAppIds() const {
   return result;
 }
 
-bool ExtensionService::IsBackgroundPageReady(const Extension* extension) const {
-  if (!extensions::BackgroundInfo::HasPersistentBackgroundPage(extension))
-    return true;
-  ExtensionRuntimeDataMap::const_iterator it =
-      extension_runtime_data_.find(extension->id());
-  return it == extension_runtime_data_.end() ? false :
-                                               it->second.background_page_ready;
-}
-
-void ExtensionService::SetBackgroundPageReady(const Extension* extension) {
-  DCHECK(extensions::BackgroundInfo::HasBackgroundPage(extension));
-  extension_runtime_data_[extension->id()].background_page_ready = true;
-}
-
-bool ExtensionService::IsBeingUpgraded(const Extension* extension) const {
-  ExtensionRuntimeDataMap::const_iterator it =
-      extension_runtime_data_.find(extension->id());
-  return it == extension_runtime_data_.end() ? false :
-                                               it->second.being_upgraded;
-}
-
-void ExtensionService::SetBeingUpgraded(const Extension* extension,
-                                        bool value) {
-  extension_runtime_data_[extension->id()].being_upgraded = value;
-}
-
 bool ExtensionService::IsBeingReloaded(
     const std::string& extension_id) const {
   return ContainsKey(extensions_being_reloaded_, extension_id);
@@ -2658,18 +2630,6 @@ void ExtensionService::SetBeingReloaded(const std::string& extension_id,
     extensions_being_reloaded_.erase(extension_id);
 }
 
-bool ExtensionService::HasUsedWebRequest(const Extension* extension) const {
-  ExtensionRuntimeDataMap::const_iterator it =
-      extension_runtime_data_.find(extension->id());
-  return it == extension_runtime_data_.end() ? false :
-                                               it->second.has_used_webrequest;
-}
-
-void ExtensionService::SetHasUsedWebRequest(const Extension* extension,
-                                            bool value) {
-  extension_runtime_data_[extension->id()].has_used_webrequest = value;
-}
-
 bool ExtensionService::ShouldEnableOnInstall(const Extension* extension) {
   // Extensions installed by policy can't be disabled. So even if a previous
   // installation disabled the extension, make sure it is now enabled.
@@ -2822,7 +2782,7 @@ void ExtensionService::UnloadAllExtensionsInternal() {
   profile_->GetExtensionSpecialStoragePolicy()->RevokeRightsForAllExtensions();
 
   registry_->ClearAll();
-  extension_runtime_data_.clear();
+  system_->runtime_data()->ClearAll();
 
   // TODO(erikkay) should there be a notification for this?  We can't use
   // EXTENSION_UNLOADED since that implies that the extension has been disabled