Avoid reading ExtensionAction*s in the ScriptBadgeController when they may be deleted,

after the extension has started being unloaded.
    
A side-effect is that script badges now appear in alphabetical order by
extension id, rather than when the script runs. I think this is actually an
improvement since it gives a consistent order for scripts that run on tab load,
where they appeared somewhat randomly before.
    
BUG=155050, 154535


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

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

2 files changed, 23 insertions, 41 deletions

diff --git a/chrome/browser/extensions/script_badge_controller.cc b/chrome/browser/extensions/script_badge_controller.cc
index 427e119..17b6596 100644
--- a/chrome/browser/extensions/script_badge_controller.cc
+++ b/chrome/browser/extensions/script_badge_controller.cc
@@ -45,7 +45,23 @@ ScriptBadgeController::ScriptBadgeController(content::WebContents* web_contents,
 ScriptBadgeController::~ScriptBadgeController() {}
 
 std::vector<ExtensionAction*> ScriptBadgeController::GetCurrentActions() const {
-  return current_actions_;
+  std::vector<ExtensionAction*> result;
+  ExtensionService* service = GetExtensionService();
+  if (!service)
+    return result;
+
+  const ExtensionSet* extensions = service->extensions();
+  for (std::set<std::string>::const_iterator
+           it = extensions_in_current_actions_.begin();
+       it != extensions_in_current_actions_.end(); ++it) {
+    const Extension* extension = extensions->GetByID(*it);
+    if (!extension)
+      continue;
+    ExtensionAction* script_badge = extension->script_badge();
+    if (script_badge)
+      result.push_back(script_badge);
+  }
+  return result;
 }
 
 void ScriptBadgeController::GetAttentionFor(
@@ -180,7 +196,7 @@ void ScriptBadgeController::OnContentScriptsExecuting(
     NotifyChange();
 }
 
-ExtensionService* ScriptBadgeController::GetExtensionService() {
+ExtensionService* ScriptBadgeController::GetExtensionService() const {
   TabContents* tab_contents = TabContents::FromWebContents(web_contents());
   return extensions::ExtensionSystem::Get(
       tab_contents->profile())->extension_service();
@@ -206,7 +222,6 @@ void ScriptBadgeController::DidNavigateMainFrame(
   if (details.is_in_page)
     return;
   extensions_in_current_actions_.clear();
-  current_actions_.clear();
 }
 
 void ScriptBadgeController::Observe(
@@ -216,7 +231,7 @@ void ScriptBadgeController::Observe(
   DCHECK_EQ(type, chrome::NOTIFICATION_EXTENSION_UNLOADED);
   const Extension* extension =
       content::Details<UnloadedExtensionInfo>(details)->extension;
-  if (EraseExtension(extension))
+  if (extensions_in_current_actions_.erase(extension->id()))
     NotifyChange();
 }
 
@@ -233,9 +248,7 @@ ExtensionAction* ScriptBadgeController::AddExtensionToCurrentActions(
   if (!extension)
     return NULL;
 
-  ExtensionAction* script_badge = extension->script_badge();
-  current_actions_.push_back(script_badge);
-  return script_badge;
+  return extension->script_badge();
 }
 
 bool ScriptBadgeController::MarkExtensionExecuting(
@@ -249,28 +262,4 @@ bool ScriptBadgeController::MarkExtensionExecuting(
   return true;
 }
 
-bool ScriptBadgeController::EraseExtension(const Extension* extension) {
-  if (extensions_in_current_actions_.erase(extension->id()) == 0)
-    return false;
-
-  size_t size_before = current_actions_.size();
-
-  for (std::vector<ExtensionAction*>::iterator it = current_actions_.begin();
-       it != current_actions_.end(); ++it) {
-    // Safe to -> the extension action because we still have a handle to the
-    // owner Extension.
-    //
-    // Also note that this means that when extensions are uninstalled their
-    // script badges will disappear, even though they're still acting on the
-    // page (since they would have already acted).
-    if ((*it)->extension_id() == extension->id()) {
-      current_actions_.erase(it);
-      break;
-    }
-  }
-
-  CHECK_EQ(size_before, current_actions_.size() + 1);
-  return true;
-}
-
 }  // namespace extensions
diff --git a/chrome/browser/extensions/script_badge_controller.h b/chrome/browser/extensions/script_badge_controller.h
index ec31f22..97b70a8 100644
--- a/chrome/browser/extensions/script_badge_controller.h
+++ b/chrome/browser/extensions/script_badge_controller.h
@@ -83,7 +83,7 @@ class ScriptBadgeController
 
  private:
   // Gets the ExtensionService for |tab_contents_|.
-  ExtensionService* GetExtensionService();
+  ExtensionService* GetExtensionService() const;
 
   // Gets the current page ID, or -1 if no navigation entry has been committed.
   int32 GetPageID();
@@ -109,15 +109,8 @@ class ScriptBadgeController
   // Returns true if any change was made.
   bool MarkExtensionExecuting(const std::string& extension_id);
 
-  // Tries to erase an extension from the relevant collections, and returns
-  // whether any change was made.
-  bool EraseExtension(const Extension* extension);
-
-  // The current extension actions in the order they appeared.  These come from
-  // calls to ExecuteScript or getAttention on the current frame.
-  std::vector<ExtensionAction*> current_actions_;
-
-  // The extensions that have actions in current_actions_.
+  // The current extension actions.  These come from calls to ExecuteScript or
+  // getAttention on the current frame.
   std::set<std::string> extensions_in_current_actions_;
 
   // Listen to extension unloaded notifications.