Order the script badges in the location bar consistently

chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm

Index: chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
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 62ef99db92524857419e8b612f291a40740e4ae6..2779eb2e36210e05ec27e4f50315fb490b64d0b2 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).
+  scoped_ptr<std::vector<ExtensionAction*> > new_page_actions =

[Nico, 2012/06/16 04:58:03]

This looks like a function called GetCurrentActions() returns something that's
supposed to be owned by the caller. Functions called GetBlah() shouldn't do
that, rename the function to NewBlah() or CreateBlah()

[not at google - send to devlin, 2012/06/18 17:50:27]

This is equivalent to a function like

void GetPageActions(std::vector<ExtensionAction*>* out) {
  while (something)
    out->push_back(action);
}

except instead of implicitly creating the return value (via appending to out) it
explicitly creates it. It's still a "getter" in that it's accessing internal
state (the ExtensionActions) not creating anything new besides the container for
the ExtensionActions. Calling this Create gives the impression to me that the
ExtensionActions are also being created.

I can change it to return just a plain std::vector<> but I was trying to avoid
extra unnecessarily copying originally (when it built up the vector on-demand;
PageActionsController still does this).

Could also change it to return a const* or const& but this is an implementation
of an interface, and that would be enforcing a stricter implementation detail on
this class and PageActionsController.

[Nico, 2012/06/18 17:54:23]

Right, it's just a vector of pointers, so just return it by value.

[not at google - send to devlin, 2012/06/18 18:05:59]

Done.

+      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]));
     }
   }