Order the script badges in the location bar in the order that

they appeared, and fix the logic on each platform which 
assumes they never change.

BUG=133139

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

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

11 files changed, 136 insertions, 68 deletions

diff --git a/chrome/browser/extensions/location_bar_controller.h b/chrome/browser/extensions/location_bar_controller.h
index 7f468b17..68048b0 100644
--- a/chrome/browser/extensions/location_bar_controller.h
+++ b/chrome/browser/extensions/location_bar_controller.h
@@ -10,8 +10,6 @@
 #include <string>
 #include <vector>
 
-#include "base/memory/scoped_ptr.h"
-
 class ExtensionAction;
 
 namespace extensions {
@@ -36,7 +34,7 @@ class LocationBarController {
       std::vector<ExtensionAction*>* out);
 
   // Gets the action data for all extensions.
-  virtual scoped_ptr<std::vector<ExtensionAction*> > GetCurrentActions() = 0;
+  virtual std::vector<ExtensionAction*> GetCurrentActions() = 0;
 
   // Notifies this that the badge for an extension has been clicked with some
   // mouse button (1 for left, 2 for middle, and 3 for right click), and
diff --git a/chrome/browser/extensions/page_action_controller.cc b/chrome/browser/extensions/page_action_controller.cc
index 9ba6f54..59157a8 100644
--- a/chrome/browser/extensions/page_action_controller.cc
+++ b/chrome/browser/extensions/page_action_controller.cc
@@ -21,22 +21,20 @@ PageActionController::PageActionController(TabContents* tab_contents)
 
 PageActionController::~PageActionController() {}
 
-scoped_ptr<std::vector<ExtensionAction*> >
-PageActionController::GetCurrentActions() {
-  scoped_ptr<std::vector<ExtensionAction*> > current_actions(
-      new std::vector<ExtensionAction*>());
-
+std::vector<ExtensionAction*> PageActionController::GetCurrentActions() {
   ExtensionService* service = GetExtensionService();
   if (!service)
-    return current_actions.Pass();
+    return std::vector<ExtensionAction*>();
+
+  std::vector<ExtensionAction*> current_actions;
 
   for (ExtensionSet::const_iterator i = service->extensions()->begin();
        i != service->extensions()->end(); ++i) {
     ExtensionAction* action = (*i)->page_action();
     if (action)
-      current_actions->push_back(action);
+      current_actions.push_back(action);
   }
-  return current_actions.Pass();
+  return current_actions;
 }
 
 LocationBarController::Action PageActionController::OnClicked(
diff --git a/chrome/browser/extensions/page_action_controller.h b/chrome/browser/extensions/page_action_controller.h
index 6346e73..e0f983b 100644
--- a/chrome/browser/extensions/page_action_controller.h
+++ b/chrome/browser/extensions/page_action_controller.h
@@ -25,8 +25,7 @@ class PageActionController : public LocationBarController {
   virtual ~PageActionController();
 
   // LocationBarController implementation.
-  virtual scoped_ptr<std::vector<ExtensionAction*> > GetCurrentActions()
-      OVERRIDE;
+  virtual std::vector<ExtensionAction*> GetCurrentActions() OVERRIDE;
   virtual Action OnClicked(const std::string& extension_id,
                            int mouse_button) OVERRIDE;
 
diff --git a/chrome/browser/extensions/script_badge_controller.cc b/chrome/browser/extensions/script_badge_controller.cc
index 7efd6a9..3fa85ab 100644
--- a/chrome/browser/extensions/script_badge_controller.cc
+++ b/chrome/browser/extensions/script_badge_controller.cc
@@ -24,26 +24,16 @@ namespace extensions {
 ScriptBadgeController::ScriptBadgeController(TabContents* tab_contents)
     : content::WebContentsObserver(tab_contents->web_contents()),
       script_executor_(tab_contents->web_contents()),
-      tab_contents_(tab_contents) {}
+      tab_contents_(tab_contents) {
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_EXTENSION_UNLOADED,
+                 content::Source<Profile>(tab_contents->profile()));
+}
 
 ScriptBadgeController::~ScriptBadgeController() {}
 
-scoped_ptr<std::vector<ExtensionAction*> >
-ScriptBadgeController::GetCurrentActions() {
-  scoped_ptr<std::vector<ExtensionAction*> > current_actions(
-      new std::vector<ExtensionAction*>());
-
-  ExtensionService* service = GetExtensionService();
-  if (!service)
-    return current_actions.Pass();
-
-  for (ExtensionSet::const_iterator it = service->extensions()->begin();
-       it != service->extensions()->end(); ++it) {
-    const Extension* extension = *it;
-    if (extensions_executing_scripts_.count(extension->id()))
-      current_actions->push_back(extension->GetScriptBadge());
-  }
-  return current_actions.Pass();
+std::vector<ExtensionAction*> ScriptBadgeController::GetCurrentActions() {
+  return current_actions_;
 }
 
 LocationBarController::Action ScriptBadgeController::OnClicked(
@@ -98,8 +88,8 @@ void ScriptBadgeController::OnExecuteScriptFinished(
     int32 page_id,
     const std::string& error) {
   if (success && page_id == GetPageID()) {
-    extensions_executing_scripts_.insert(extension_id);
-    Notify();
+    if (InsertExtension(extension_id))
+      Notify();
   }
 
   callback.Run(success, page_id, error);
@@ -125,6 +115,18 @@ void ScriptBadgeController::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
   extensions_executing_scripts_.clear();
+  current_actions_.clear();
+}
+
+void ScriptBadgeController::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(type, chrome::NOTIFICATION_EXTENSION_UNLOADED);
+  const Extension* extension =
+      content::Details<UnloadedExtensionInfo>(details)->extension;
+  if (EraseExtension(extension))
+    Notify();
 }
 
 bool ScriptBadgeController::OnMessageReceived(const IPC::Message& message) {
@@ -139,13 +141,57 @@ bool ScriptBadgeController::OnMessageReceived(const IPC::Message& message) {
 
 void ScriptBadgeController::OnContentScriptsExecuting(
     const std::set<std::string>& extension_ids, int32 page_id) {
-  if (page_id == GetPageID()) {
-    size_t original_size = extensions_executing_scripts_.size();
-    extensions_executing_scripts_.insert(extension_ids.begin(),
-                                         extension_ids.end());
-    if (extensions_executing_scripts_.size() > original_size)
-      Notify();
+  if (page_id != GetPageID())
+    return;
+
+  bool changed = false;
+  for (std::set<std::string>::const_iterator it = extension_ids.begin();
+       it != extension_ids.end(); ++it) {
+    changed |= InsertExtension(*it);
   }
+  if (changed)
+    Notify();
+}
+
+bool ScriptBadgeController::InsertExtension(const std::string& extension_id) {
+  if (!extensions_executing_scripts_.insert(extension_id).second)
+    return false;
+
+  ExtensionService* service = GetExtensionService();
+  if (!service)
+    return  false;
+
+  const Extension* extension = service->extensions()->GetByID(extension_id);
+  if (!extension)
+    return false;
+
+  current_actions_.push_back(extension->GetScriptBadge());
+  return true;
+}
+
+
+bool ScriptBadgeController::EraseExtension(const Extension* extension) {
+  if (extensions_executing_scripts_.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 e40ae49..420e666b 100644
--- a/chrome/browser/extensions/script_badge_controller.h
+++ b/chrome/browser/extensions/script_badge_controller.h
@@ -16,6 +16,8 @@
 #include "chrome/browser/extensions/location_bar_controller.h"
 #include "chrome/browser/extensions/script_executor.h"
 #include "chrome/browser/extensions/script_executor_impl.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/web_contents_observer.h"
 
 class ExtensionAction;
@@ -48,13 +50,13 @@ class ScriptBadgeController
     : public base::RefCountedThreadSafe<ScriptBadgeController>,
       public LocationBarController,
       public ScriptExecutor,
-      public content::WebContentsObserver {
+      public content::WebContentsObserver,
+      public content::NotificationObserver {
  public:
   explicit ScriptBadgeController(TabContents* tab_contents);
 
   // LocationBarController implementation.
-  virtual scoped_ptr<std::vector<ExtensionAction*> > GetCurrentActions()
-      OVERRIDE;
+  virtual std::vector<ExtensionAction*> GetCurrentActions() OVERRIDE;
   virtual Action OnClicked(const std::string& extension_id,
                            int mouse_button) OVERRIDE;
 
@@ -94,19 +96,38 @@ class ScriptBadgeController
       const content::FrameNavigateParams& params) OVERRIDE;
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
+  // content::NotificationObserver implementation.
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
   // IPC::Message handlers.
   void OnContentScriptsExecuting(const std::set<std::string>& extension_ids,
                                  int32 page_id);
 
+  // Tries to insert an extension into the relevant collections, and returns
+  // whether any change was made.
+  bool InsertExtension(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);
+
   // Delegate ScriptExecutorImpl for running ExecuteScript.
   ScriptExecutorImpl script_executor_;
 
   // Our parent TabContents.
   TabContents* tab_contents_;
 
+  // The current extension actions in the order they appeared.
+  std::vector<ExtensionAction*> current_actions_;
+
   // The extensions that have called ExecuteScript on the current frame.
   std::set<std::string> extensions_executing_scripts_;
 
+  // Listen to extension unloaded notifications.
+  content::NotificationRegistrar registrar_;
+
   DISALLOW_COPY_AND_ASSIGN(ScriptBadgeController);
 };
 
diff --git a/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h b/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
index cf6bd4d..68c74cb 100644
--- a/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
+++ b/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
@@ -218,7 +218,10 @@ class LocationBarViewMac : public AutocompleteEditController,
   // Chrome To Mobile page action icon.
   scoped_ptr<ChromeToMobileDecoration> chrome_to_mobile_decoration_;
 
-  // Any installed Page Actions.
+  // The installed page actions.
+  std::vector<ExtensionAction*> page_actions_;
+
+  // Decorations for the installed Page Actions.
   ScopedVector<PageActionDecoration> page_action_decorations_;
 
   // The content blocked decorations.
diff --git a/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm b/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
index 62ef99d..e9305bd 100644
--- a/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
+++ b/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
@@ -595,26 +595,22 @@ void LocationBarViewMac::RefreshPageActionDecorations() {
     return;
   }
 
-  std::vector<ExtensionAction*> page_actions;
-
   TabContents* tab_contents = GetTabContents();
   if (!tab_contents) {
     DeletePageActionDecorations();  // Necessary?
     return;
   }
 
-  extensions::LocationBarController* controller =
-      tab_contents->extension_tab_helper()->location_bar_controller();
-  page_actions.swap(*controller->GetCurrentActions());
-
-  // On startup we sometimes haven't loaded any extensions. This makes sure
-  // we catch up when the extensions (and any Page Actions) load.
-  if (page_actions.size() != page_action_decorations_.size()) {
-    DeletePageActionDecorations();  // Delete the old views (if any).
+  std::vector<ExtensionAction*> new_page_actions =
+      tab_contents->extension_tab_helper()->location_bar_controller()->
+          GetCurrentActions();
 
-    for (size_t i = 0; i < page_actions.size(); ++i) {
+  if (new_page_actions != page_actions_) {
+    page_actions_.swap(new_page_actions);
+    DeletePageActionDecorations();
+    for (size_t i = 0; i < page_actions_.size(); ++i) {
       page_action_decorations_.push_back(
-          new PageActionDecoration(this, profile_, page_actions[i]));
+          new PageActionDecoration(this, profile_, page_actions_[i]));
     }
   }
 
diff --git a/chrome/browser/ui/gtk/location_bar_view_gtk.cc b/chrome/browser/ui/gtk/location_bar_view_gtk.cc
index d0c7234..6a495e8 100644
--- a/chrome/browser/ui/gtk/location_bar_view_gtk.cc
+++ b/chrome/browser/ui/gtk/location_bar_view_gtk.cc
@@ -461,7 +461,6 @@ void LocationBarViewGtk::SetPreviewEnabledPageAction(
     ExtensionAction *page_action,
     bool preview_enabled) {
   DCHECK(page_action);
-  UpdatePageActions();
   for (ScopedVector<PageActionViewGtk>::iterator iter =
        page_action_views_.begin(); iter != page_action_views_.end();
        ++iter) {
@@ -722,23 +721,24 @@ void LocationBarViewGtk::UpdateContentSettingsIcons() {
 }
 
 void LocationBarViewGtk::UpdatePageActions() {
-  std::vector<ExtensionAction*> page_actions;
+  std::vector<ExtensionAction*> new_page_actions;
 
   TabContents* tab_contents = GetTabContents();
   if (tab_contents) {
     LocationBarController* controller =
         tab_contents->extension_tab_helper()->location_bar_controller();
-    page_actions.swap(*controller->GetCurrentActions());
+    new_page_actions = controller->GetCurrentActions();
   }
 
-  // Initialize on the first call, or re-inialize if more extensions have been
+  // Initialize on the first call, or re-initialize if more extensions have been
   // loaded or added after startup.
-  if (page_actions.size() != page_action_views_.size()) {
-    page_action_views_.reset();  // Delete the old views (if any).
+  if (new_page_actions != page_actions_) {
+    page_actions_.swap(new_page_actions);
+    page_action_views_.reset();
 
-    for (size_t i = 0; i < page_actions.size(); ++i) {
+    for (size_t i = 0; i < page_actions_.size(); ++i) {
       page_action_views_.push_back(
-          new PageActionViewGtk(this, page_actions[i]));
+          new PageActionViewGtk(this, page_actions_[i]));
       gtk_box_pack_end(GTK_BOX(page_action_hbox_.get()),
                        page_action_views_[i]->widget(), FALSE, FALSE, 0);
     }
diff --git a/chrome/browser/ui/gtk/location_bar_view_gtk.h b/chrome/browser/ui/gtk/location_bar_view_gtk.h
index f9aad7b..f8c3e99 100644
--- a/chrome/browser/ui/gtk/location_bar_view_gtk.h
+++ b/chrome/browser/ui/gtk/location_bar_view_gtk.h
@@ -449,6 +449,9 @@ class LocationBarViewGtk : public AutocompleteEditController,
   ui::OwnedWidgetGtk content_setting_hbox_;
   ScopedVector<ContentSettingImageViewGtk> content_setting_views_;
 
+  // Extension page actions.
+  std::vector<ExtensionAction*> page_actions_;
+
   // Extension page action icons.
   ui::OwnedWidgetGtk page_action_hbox_;
   ScopedVector<PageActionViewGtk> page_action_views_;
diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc
index 67f3c21..c7f1a31 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -1028,21 +1028,22 @@ void LocationBarView::RefreshPageActionViews() {
     old_visibility[(*i)->image_view()->page_action()] = (*i)->visible();
   }
 
-  std::vector<ExtensionAction*> page_actions;
+  std::vector<ExtensionAction*> new_page_actions;
 
   TabContents* tab_contents = GetTabContents();
   if (tab_contents) {
     extensions::LocationBarController* controller =
         tab_contents->extension_tab_helper()->location_bar_controller();
-    page_actions.swap(*controller->GetCurrentActions());
+    new_page_actions = controller->GetCurrentActions();
   }
 
   // On startup we sometimes haven't loaded any extensions. This makes sure
   // we catch up when the extensions (and any page actions) load.
-  if (page_actions.size() != page_action_views_.size()) {
+  if (page_actions_ != new_page_actions) {
+    page_actions_.swap(new_page_actions);
     DeletePageActionViews();  // Delete the old views (if any).
 
-    page_action_views_.resize(page_actions.size());
+    page_action_views_.resize(page_actions_.size());
     View* right_anchor = chrome_to_mobile_view_;
     if (!right_anchor)
       right_anchor = star_view_;
@@ -1052,9 +1053,9 @@ void LocationBarView::RefreshPageActionViews() {
 
     // Add the page actions in reverse order, so that the child views are
     // inserted in left-to-right order for accessibility.
-    for (int i = page_actions.size() - 1; i >= 0; --i) {
+    for (int i = page_actions_.size() - 1; i >= 0; --i) {
       page_action_views_[i] = new PageActionWithBadgeView(
-          delegate_->CreatePageActionImageView(this, page_actions[i]));
+          delegate_->CreatePageActionImageView(this, page_actions_[i]));
       page_action_views_[i]->SetVisible(false);
       AddChildViewAt(page_action_views_[i], GetIndexOf(right_anchor));
     }
diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.h b/chrome/browser/ui/views/location_bar/location_bar_view.h
index a1d51cb..9cd3f2e 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.h
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.h
@@ -438,6 +438,9 @@ class LocationBarView : public LocationBar,
   // The content setting views.
   ContentSettingViews content_setting_views_;
 
+  // The current page actions.
+  std::vector<ExtensionAction*> page_actions_;
+
   // The page action icon views.
   PageActionViews page_action_views_;