Move ownership of ExtensionAction into ExtensionSystem.

ExtensionActions will be created by ExtensionActionManager during the
EXTENSION_LOADED notification and destroyed during EXTENSION_UNLOADED.  In this
change, the ExtensionActionManager inserts pointers to the ExtensionActions into
their Extension, but those pointers will go away in the next refactoring step.

This is part of a 4-part refactoring to clean up ExtensionAction icon handling.
The subsequent parts are:
2) Change ExtensionAction accesses from using Extension::*action() to using
   ExtensionSystem.
3) Move ExtensionAction from common/ to browser/
4) Make the icon-loading and -caching classes into ExtensionAction methods.

BUG=153463


Review URL: https://chromiumcodereview.appspot.com/11032013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@160677 0039d316-1c4b-4281-b951-d872f2087c98

14 files changed, 250 insertions, 11 deletions

diff --git a/chrome/browser/extensions/api/extension_action/extension_actions_api.cc b/chrome/browser/extensions/api/extension_action/extension_actions_api.cc
index 6c5f9c2..79f71da 100644
--- a/chrome/browser/extensions/api/extension_action/extension_actions_api.cc
+++ b/chrome/browser/extensions/api/extension_action/extension_actions_api.cc
@@ -241,7 +241,7 @@ void ExtensionActionStorageManager::ReadFromStorage(
     const std::string& extension_id, scoped_ptr<base::Value> value) {
   const Extension* extension =
       ExtensionSystem::Get(profile_)->extension_service()->
-          GetExtensionById(extension_id, true);
+      extensions()->GetByID(extension_id);
   if (!extension)
     return;
 
@@ -352,15 +352,15 @@ bool ExtensionActionFunction::RunImpl() {
 
 void ExtensionActionFunction::NotifyChange() {
   switch (extension_action_->action_type()) {
-    case ExtensionAction::TYPE_BROWSER:
-    case ExtensionAction::TYPE_PAGE:
+    case extensions::Extension::ActionInfo::TYPE_BROWSER:
+    case extensions::Extension::ActionInfo::TYPE_PAGE:
       if (extension_->browser_action()) {
         NotifyBrowserActionChange();
       } else if (extension_->page_action()) {
         NotifyLocationBarChange();
       }
       return;
-    case ExtensionAction::TYPE_SCRIPT_BADGE:
+    case extensions::Extension::ActionInfo::TYPE_SCRIPT_BADGE:
       NotifyLocationBarChange();
       return;
   }
diff --git a/chrome/browser/extensions/browser_event_router.cc b/chrome/browser/extensions/browser_event_router.cc
index 7f69f0c..ce87028 100644
--- a/chrome/browser/extensions/browser_event_router.cc
+++ b/chrome/browser/extensions/browser_event_router.cc
@@ -21,6 +21,7 @@
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/notification_service.h"
@@ -576,13 +577,13 @@ void BrowserEventRouter::ExtensionActionExecuted(
     TabContents* tab_contents) {
   const char* event_name = NULL;
   switch (extension_action.action_type()) {
-    case ExtensionAction::TYPE_BROWSER:
+    case Extension::ActionInfo::TYPE_BROWSER:
       event_name = "browserAction.onClicked";
       break;
-    case ExtensionAction::TYPE_PAGE:
+    case Extension::ActionInfo::TYPE_PAGE:
       event_name = "pageAction.onClicked";
       break;
-    case ExtensionAction::TYPE_SCRIPT_BADGE:
+    case Extension::ActionInfo::TYPE_SCRIPT_BADGE:
       event_name = "scriptBadge.onClicked";
       break;
   }
diff --git a/chrome/browser/extensions/extension_action_icon_factory_unittest.cc b/chrome/browser/extensions/extension_action_icon_factory_unittest.cc
index cea9a78..8c64eee 100644
--- a/chrome/browser/extensions/extension_action_icon_factory_unittest.cc
+++ b/chrome/browser/extensions/extension_action_icon_factory_unittest.cc
@@ -4,13 +4,17 @@
 
 #include "chrome/browser/extensions/extension_action_icon_factory.h"
 
+#include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_action.h"
+#include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread.h"
 #include "grit/theme_resources.h"
 #include "skia/ext/image_operations.h"
@@ -112,14 +116,29 @@ class ExtensionActionIconFactoryTest
     if (!valid_value.get())
       return NULL;
 
-    return Extension::Create(test_file, location, *valid_value,
-                             Extension::NO_FLAGS, &error);
+    scoped_refptr<Extension> extension =
+        Extension::Create(test_file, location, *valid_value,
+                          Extension::NO_FLAGS, &error);
+    EXPECT_TRUE(extension) << error;
+    if (extension)
+      extension_service_->AddExtension(extension);
+    return extension;
   }
 
   // testing::Test overrides:
   virtual void SetUp() OVERRIDE {
     file_thread_.Start();
     io_thread_.Start();
+    profile_.reset(new TestingProfile);
+    CommandLine command_line(CommandLine::NO_PROGRAM);
+    extension_service_ = static_cast<extensions::TestExtensionSystem*>(
+        extensions::ExtensionSystem::Get(profile_.get()))->
+        CreateExtensionService(&command_line, FilePath(), false);
+  }
+
+  virtual void TearDown() OVERRIDE {
+    profile_.reset();  // Get all DeleteSoon calls sent to ui_loop_.
+    ui_loop_.RunAllPending();
   }
 
   // ExtensionActionIconFactory::Observer overrides:
@@ -139,6 +158,8 @@ class ExtensionActionIconFactoryTest
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
   content::TestBrowserThread io_thread_;
+  scoped_ptr<TestingProfile> profile_;
+  ExtensionService* extension_service_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionActionIconFactoryTest);
 };
diff --git a/chrome/browser/extensions/extension_action_manager.cc b/chrome/browser/extensions/extension_action_manager.cc
new file mode 100644
index 0000000..4c06197
--- /dev/null
+++ b/chrome/browser/extensions/extension_action_manager.cc
@@ -0,0 +1,147 @@
+// 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_action_manager.h"
+
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_action.h"
+#include "chrome/common/extensions/extension_switch_utils.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
+
+namespace extensions {
+
+ExtensionActionManager::ExtensionActionManager(Profile* profile)
+    : profile_(profile) {
+  CHECK_EQ(profile, profile->GetOriginalProfile())
+      << "Don't instantiate this with an incognito profile.";
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
+                 content::Source<Profile>(profile));
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
+                 content::Source<Profile>(profile));
+}
+
+ExtensionActionManager::~ExtensionActionManager() {
+  // Don't assert that the ExtensionAction maps are empty because Extensions are
+  // sometimes (only in tests?) not unloaded before the Profile is destroyed.
+}
+
+// This will be unnecessary when the accessors within Extension go away.
+void ExtensionActionManager::Shutdown() {
+  const ExtensionService* extension_service =
+      ExtensionSystem::Get(profile_)->extension_service();
+  if (!extension_service) {
+    // Some tests fire the EXTENSION_LOADED notification without using
+    // an ExtensionService.  For them, don't bother nulling out the
+    // extension fields on shutdown.
+    return;
+  }
+
+  for (ExtIdToActionMap::const_iterator it = page_actions_.begin();
+       it != page_actions_.end(); ++it) {
+    const std::string& extension_id = it->second->extension_id();
+    const Extension* extension =
+        extension_service->GetExtensionById(extension_id,
+                                            /*include_disabled=*/true);
+    if (extension) {
+      // If the Extension is still alive, make sure it doesn't have a
+      // dangling pointer to the destroyed ExtensionAction.
+      extension->page_action_ = NULL;
+    }
+  }
+  for (ExtIdToActionMap::const_iterator it = browser_actions_.begin();
+       it != browser_actions_.end(); ++it) {
+    const std::string& extension_id = it->second->extension_id();
+    const Extension* extension =
+        extension_service->GetExtensionById(extension_id,
+                                            /*include_disabled=*/true);
+    if (extension) {
+      // If the Extension is still alive, make sure it doesn't have a
+      // dangling pointer to the destroyed ExtensionAction.
+      extension->browser_action_ = NULL;
+    }
+  }
+  for (ExtIdToActionMap::const_iterator it = script_badges_.begin();
+       it != script_badges_.end(); ++it) {
+    const std::string& extension_id = it->second->extension_id();
+    const Extension* extension =
+        extension_service->GetExtensionById(extension_id,
+                                            /*include_disabled=*/true);
+    if (extension) {
+      // If the Extension is still alive, make sure it doesn't have a
+      // dangling pointer to the destroyed ExtensionAction.
+      extension->script_badge_ = NULL;
+    }
+  }
+}
+
+void ExtensionActionManager::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  switch (type) {
+    case chrome::NOTIFICATION_EXTENSION_LOADED: {
+      const Extension* extension =
+          content::Details<const Extension>(details).ptr();
+      if (extension->page_action_info() &&
+          !ContainsKey(page_actions_, extension->id())) {
+        linked_ptr<ExtensionAction> page_action(
+            new ExtensionAction(extension->id(),
+                                Extension::ActionInfo::TYPE_PAGE,
+                                *extension->page_action_info()));
+        // The action box changes the meaning of the page action area, so we
+        // need to convert page actions into browser actions.
+        if (switch_utils::AreScriptBadgesEnabled()) {
+          browser_actions_[extension->id()] = page_action;
+          extension->browser_action_ = page_action.get();
+        } else {
+          page_actions_[extension->id()] = page_action;
+          extension->page_action_ = page_action.get();
+        }
+      }
+      if (extension->browser_action_info() &&
+          !ContainsKey(browser_actions_, extension->id())) {
+        linked_ptr<ExtensionAction> browser_action(
+            new ExtensionAction(extension->id(),
+                                Extension::ActionInfo::TYPE_BROWSER,
+                                *extension->browser_action_info()));
+        browser_actions_[extension->id()] = browser_action;
+        extension->browser_action_ = browser_action.get();
+      }
+      DCHECK(extension->script_badge_info());
+      if (!ContainsKey(script_badges_, extension->id())) {
+        linked_ptr<ExtensionAction> script_badge(
+            new ExtensionAction(extension->id(),
+                                Extension::ActionInfo::TYPE_SCRIPT_BADGE,
+                                *extension->script_badge_info()));
+        script_badges_[extension->id()] = script_badge;
+        extension->script_badge_ = script_badge.get();
+      }
+      break;
+    }
+    case chrome::NOTIFICATION_EXTENSION_UNLOADED: {
+      const Extension* extension =
+          content::Details<UnloadedExtensionInfo>(details)->extension;
+      if (ContainsKey(page_actions_, extension->id())) {
+        extension->page_action_ = NULL;
+        page_actions_.erase(extension->id());
+      }
+      if (ContainsKey(browser_actions_, extension->id())) {
+        extension->browser_action_ = NULL;
+        browser_actions_.erase(extension->id());
+      }
+      if (ContainsKey(script_badges_, extension->id())) {
+        extension->script_badge_ = NULL;
+        script_badges_.erase(extension->id());
+      }
+      break;
+    }
+  }
+}
+
+}
diff --git a/chrome/browser/extensions/extension_action_manager.h b/chrome/browser/extensions/extension_action_manager.h
new file mode 100644
index 0000000..4219dec
--- /dev/null
+++ b/chrome/browser/extensions/extension_action_manager.h
@@ -0,0 +1,54 @@
+// 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.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_ACTION_MANAGER_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_ACTION_MANAGER_H_
+
+#include <map>
+#include <string>
+
+#include "base/memory/linked_ptr.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+
+class ExtensionAction;
+class Profile;
+
+namespace extensions {
+
+class Extension;
+
+// Owns the ExtensionActions associated with each extension.  These actions live
+// while an extension is loaded and are destroyed on unload.
+class ExtensionActionManager : public content::NotificationObserver {
+ public:
+  explicit ExtensionActionManager(Profile* profile);
+  virtual ~ExtensionActionManager();
+
+  // Called during ExtensionSystem::Shutdown(), when the associated
+  // Profile is going to be destroyed.
+  void Shutdown();
+
+ private:
+  // Implement content::NotificationObserver.
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  Profile* const profile_;
+  content::NotificationRegistrar registrar_;
+
+  // Keyed by Extension ID.  These maps are populated when the extension is
+  // loaded, and the entries are removed when the extension is unloaded.  Not
+  // every extension has a page action or browser action, but all have a script
+  // badge.
+  typedef std::map<std::string, linked_ptr<ExtensionAction> > ExtIdToActionMap;
+  ExtIdToActionMap page_actions_;
+  ExtIdToActionMap browser_actions_;
+  ExtIdToActionMap script_badges_;
+};
+
+}
+
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_ACTION_MANAGER_H_
diff --git a/chrome/browser/extensions/extension_system.cc b/chrome/browser/extensions/extension_system.cc
index ecd6067..e41189c 100644
--- a/chrome/browser/extensions/extension_system.cc
+++ b/chrome/browser/extensions/extension_system.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/extensions/api/declarative/rules_registry_service.h"
 #include "chrome/browser/extensions/component_loader.h"
 #include "chrome/browser/extensions/event_router.h"
+#include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_devtools_manager.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_info_map.h"
@@ -108,6 +109,7 @@ void ExtensionSystemImpl::Shared::Init(bool extensions_enabled) {
   extension_event_router_.reset(new EventRouter(profile_,
                                                 extension_prefs_.get()));
   navigation_observer_.reset(new NavigationObserver(profile_));
+  extension_action_manager_.reset(new ExtensionActionManager(profile_));
 
   ExtensionErrorReporter::Init(true);  // allow noisy errors.
 
@@ -197,6 +199,8 @@ void ExtensionSystemImpl::Shared::Init(bool extensions_enabled) {
 }
 
 void ExtensionSystemImpl::Shared::Shutdown() {
+  if (extension_action_manager_.get())
+    extension_action_manager_->Shutdown();
   if (extension_service_.get())
     extension_service_->Shutdown();
 }
diff --git a/chrome/browser/extensions/extension_system.h b/chrome/browser/extensions/extension_system.h
index 93cec34..b9c19f2 100644
--- a/chrome/browser/extensions/extension_system.h
+++ b/chrome/browser/extensions/extension_system.h
@@ -28,6 +28,7 @@ namespace extensions {
 class AlarmManager;
 class EventRouter;
 class Extension;
+class ExtensionActionManager;
 class ExtensionPrefs;
 class ExtensionSystemSharedFactory;
 class LazyBackgroundTaskQueue;
@@ -204,6 +205,7 @@ class ExtensionSystemImpl : public ExtensionSystem {
     LazyBackgroundTaskQueue* lazy_background_task_queue();
     MessageService* message_service();
     EventRouter* event_router();
+    ExtensionActionManager* extension_action_manager();
 
    private:
     Profile* profile_;
@@ -224,6 +226,7 @@ class ExtensionSystemImpl : public ExtensionSystem {
     scoped_ptr<MessageService> message_service_;
     scoped_ptr<EventRouter> extension_event_router_;
     scoped_ptr<NavigationObserver> navigation_observer_;
+    scoped_ptr<ExtensionActionManager> extension_action_manager_;
   };
 
   Profile* profile_;
diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/extensions/extension_toolbar_model.cc
index f6c7909..67f665a 100644
--- a/chrome/browser/extensions/extension_toolbar_model.cc
+++ b/chrome/browser/extensions/extension_toolbar_model.cc
@@ -7,8 +7,8 @@
 #include "chrome/browser/extensions/browser_event_router.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
+#include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
@@ -16,6 +16,7 @@
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/extensions/extension_switch_utils.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_details.h"
diff --git a/chrome/browser/extensions/page_action_browsertest.cc b/chrome/browser/extensions/page_action_browsertest.cc
index e7fba44..db2fe2a 100644
--- a/chrome/browser/extensions/page_action_browsertest.cc
+++ b/chrome/browser/extensions/page_action_browsertest.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_action.h"
 #include "chrome/test/base/ui_test_utils.h"
 
 using extensions::Extension;
diff --git a/chrome/browser/extensions/page_action_controller.cc b/chrome/browser/extensions/page_action_controller.cc
index 631a3f4..e4aec65 100644
--- a/chrome/browser/extensions/page_action_controller.cc
+++ b/chrome/browser/extensions/page_action_controller.cc
@@ -12,8 +12,9 @@
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
-#include "chrome/common/extensions/extension_set.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/extensions/extension_action.h"
+#include "chrome/common/extensions/extension_set.h"
 #include "content/public/browser/invalidate_type.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/notification_service.h"
diff --git a/chrome/browser/extensions/page_action_controller_unittest.cc b/chrome/browser/extensions/page_action_controller_unittest.cc
index 8211a88..e08ab27 100644
--- a/chrome/browser/extensions/page_action_controller_unittest.cc
+++ b/chrome/browser/extensions/page_action_controller_unittest.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/tab_contents/test_tab_contents.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/extensions/extension_builder.h"
 #include "chrome/common/extensions/value_builder.h"
 #include "content/public/browser/browser_thread.h"
diff --git a/chrome/browser/extensions/script_bubble_controller_unittest.cc b/chrome/browser/extensions/script_bubble_controller_unittest.cc
index 2251f5e..4b69464 100644
--- a/chrome/browser/extensions/script_bubble_controller_unittest.cc
+++ b/chrome/browser/extensions/script_bubble_controller_unittest.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/tab_contents/test_tab_contents.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/extensions/extension_builder.h"
 #include "chrome/common/extensions/feature_switch.h"
 #include "chrome/common/extensions/value_builder.h"
diff --git a/chrome/browser/extensions/test_extension_system.cc b/chrome/browser/extensions/test_extension_system.cc
index 915e137..5393495 100644
--- a/chrome/browser/extensions/test_extension_system.cc
+++ b/chrome/browser/extensions/test_extension_system.cc
@@ -29,6 +29,7 @@ namespace extensions {
 
 TestExtensionSystem::TestExtensionSystem(Profile* profile)
     : profile_(profile),
+      extension_action_manager_(profile_),
       info_map_(new ExtensionInfoMap()) {
 }
 
@@ -36,6 +37,7 @@ TestExtensionSystem::~TestExtensionSystem() {
 }
 
 void TestExtensionSystem::Shutdown() {
+  extension_action_manager_.Shutdown();
   extension_process_manager_.reset();
 }
 
diff --git a/chrome/browser/extensions/test_extension_system.h b/chrome/browser/extensions/test_extension_system.h
index 92822e0..2331a8c 100644
--- a/chrome/browser/extensions/test_extension_system.h
+++ b/chrome/browser/extensions/test_extension_system.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_SYSTEM_H_
 #define CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_SYSTEM_H_
 
+#include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_system.h"
 
 class CommandLine;
@@ -78,6 +79,7 @@ class TestExtensionSystem : public ExtensionSystem {
   scoped_ptr<ShellWindowGeometryCache> shell_window_geometry_cache_;
   scoped_ptr<ExtensionService> extension_service_;
   scoped_ptr<ManagementPolicy> management_policy_;
+  ExtensionActionManager extension_action_manager_;
   scoped_ptr<ExtensionProcessManager> extension_process_manager_;
   scoped_ptr<AlarmManager> alarm_manager_;
   scoped_refptr<ExtensionInfoMap> info_map_;