Don't create extension actions for disabled extensions.

chrome/browser/extensions/extension_action_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_action_manager.h"
 
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/system_indicator/system_indicator_manager.h"
 #include "chrome/browser/extensions/api/system_indicator/system_indicator_manager_factory.h"
 #include "chrome/browser/extensions/extension_action.h"
+#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/extension_action/action_info.h"
 #include "chrome/common/extensions/api/extension_action/page_action_handler.h"
 #include "chrome/common/extensions/api/extension_action/script_badge_handler.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/feature_switch.h"
 #include "components/browser_context_keyed_service/browser_context_dependency_manager.h"
 #include "components/browser_context_keyed_service/browser_context_keyed_service_factory.h"
@@ skipping 78 matching lines @@
 
 namespace {
 
 // Returns map[extension_id] if that entry exists. Otherwise, if
 // action_info!=NULL, creates an ExtensionAction from it, fills in the map, and
 // returns that.  Otherwise (action_info==NULL), returns NULL.
 ExtensionAction* GetOrCreateOrNull(
     std::map<std::string, linked_ptr<ExtensionAction> >* map,
     const std::string& extension_id,
     ActionInfo::Type action_type,
-    const ActionInfo* action_info) {
+    const ActionInfo* action_info,
+    Profile* profile) {
   std::map<std::string, linked_ptr<ExtensionAction> >::const_iterator it =
       map->find(extension_id);
   if (it != map->end())
     return it->second.get();
   if (!action_info)
     return NULL;
+
+  // Only create action info for enabled extensions.
+  // This avoids bugs where actions are recreated just after being removed

[Jeffrey Yasskin, 2013/07/30 00:25:22]

Argh, use after free. :( Do you know which code path hit this?

I'm worried that any calls to these functions during loading or unloading might
use an Extension object that's not the one that's going to live until the next
unload.

That said, I don't see a great way to ensure that an extension object is the
right one, given our notification system.

+  // in response to NOTIFICATION_EXTENSION_UNLOADED in
+  // ExtensionActionManager::Observe()
+  ExtensionService* service =
+      ExtensionSystem::Get(profile)->extension_service();
+  if (!service->GetExtensionById(extension_id, false))
+    return NULL;
+
   linked_ptr<ExtensionAction> action(new ExtensionAction(
       extension_id, action_type, *action_info));
   (*map)[extension_id] = action;
   return action.get();
 }
 
 }  // namespace
 
 ExtensionAction* ExtensionActionManager::GetPageAction(
     const extensions::Extension& extension) const {
   // The action box changes the meaning of the page action area, so we
   // need to convert page actions into browser actions.
   if (FeatureSwitch::script_badges()->IsEnabled())
     return NULL;
   return GetOrCreateOrNull(&page_actions_, extension.id(),
                            ActionInfo::TYPE_PAGE,
-                           ActionInfo::GetPageActionInfo(&extension));
+                           ActionInfo::GetPageActionInfo(&extension),
+                           profile_);
 }
 
 ExtensionAction* ExtensionActionManager::GetBrowserAction(
     const extensions::Extension& extension) const {
   const ActionInfo* action_info = ActionInfo::GetBrowserActionInfo(&extension);
   ActionInfo::Type action_type = ActionInfo::TYPE_BROWSER;
   if (FeatureSwitch::script_badges()->IsEnabled() &&
       ActionInfo::GetPageActionInfo(&extension)) {
     // The action box changes the meaning of the page action area, so we
     // need to convert page actions into browser actions.
     action_info = ActionInfo::GetPageActionInfo(&extension);
     action_type = ActionInfo::TYPE_PAGE;
   }
   return GetOrCreateOrNull(&browser_actions_, extension.id(),
-                           action_type, action_info);
+                           action_type, action_info, profile_);
 }
 
 ExtensionAction* ExtensionActionManager::GetSystemIndicator(
     const extensions::Extension& extension) const {
   // If it does not already exist, create the SystemIndicatorManager for the
   // given profile.  This could return NULL if the system indicator area is
   // unavailable on the current system.  If so, return NULL to signal that
   // the system indicator area is unusable.
   if (!extensions::SystemIndicatorManagerFactory::GetForProfile(profile_))
     return NULL;
 
   return GetOrCreateOrNull(&system_indicators_, extension.id(),
                            ActionInfo::TYPE_SYSTEM_INDICATOR,
-                           ActionInfo::GetSystemIndicatorInfo(&extension));
+                           ActionInfo::GetSystemIndicatorInfo(&extension),
+                           profile_);
 }
 
 ExtensionAction* ExtensionActionManager::GetScriptBadge(
     const extensions::Extension& extension) const {
   return GetOrCreateOrNull(&script_badges_, extension.id(),
                            ActionInfo::TYPE_SCRIPT_BADGE,
-                           ActionInfo::GetScriptBadgeInfo(&extension));
+                           ActionInfo::GetScriptBadgeInfo(&extension),
+                           profile_);
 }
 
 }  // namespace extensions