Order the script badges in the location bar consistently

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

 // 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.
 
 #import "chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h"
 
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
@@ skipping 577 matching lines @@
 
   page_action_decorations_.reset();
 }
 
 void LocationBarViewMac::RefreshPageActionDecorations() {
   if (!IsEditable()) {
     DeletePageActionDecorations();
     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());
+  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();
 
-  // 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).
-
-    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]));
     }
   }
 
   GURL url = GURL(toolbar_model_->GetText());
   for (size_t i = 0; i < page_action_decorations_.size(); ++i) {
     page_action_decorations_[i]->UpdateVisibility(
         toolbar_model_->input_in_progress() ?
             NULL : tab_contents->web_contents(),
         url);
   }
@@ skipping 82 matching lines @@
 void LocationBarViewMac::UpdateChromeToMobileEnabled() {
   if (!chrome_to_mobile_decoration_.get())
     return;
 
   DCHECK(ChromeToMobileService::IsChromeToMobileEnabled());
   bool enabled = [field_ isEditable] && !toolbar_model_->input_in_progress() &&
       ChromeToMobileServiceFactory::GetForProfile(profile_)->HasDevices();
   chrome_to_mobile_decoration_->SetVisible(enabled);
   command_updater_->UpdateCommandEnabled(IDC_CHROME_TO_MOBILE_PAGE, enabled);
 }