Initial implementation of action box menu. It is still work in progress, just CL is getting too big

and need to share existing code with other developers.

BUG=125307


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

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

chrome/browser/chromeos/login/simple_web_view_dialog.cc

@@ -183,7 +183,8 @@ void SimpleWebViewDialog::Init() {
   toolbar_model_.reset(new ToolbarModel(this));
 
   // Location bar.
-  location_bar_ = new LocationBarView(profile_,
+  location_bar_ = new LocationBarView(NULL,
+                                      profile_,
                                       command_updater_.get(),
                                       toolbar_model_.get(),
                                       this,

chrome/browser/extensions/extension_prefs.cc

@@ -71,6 +71,15 @@ const char kPrefDisableReason[] = "disable_reason";
 // object stored in the Preferences file. The extensions are stored by ID.
 const char kExtensionToolbar[] = "extensions.toolbar";
 
+// A preference that tracks the order of extensions in an action box
+// (list of extension ids).
+const char kExtensionActionBox[] = "extensions.action_box_order";
+
+// A preference that tracks the order of extensions in a toolbar when
+// action box is enabled (list of extension ids).
+const char kExtensionActionBoxBar[] =
+    "extensions.toolbar_order_with_action_box";
+
 // The key for a serialized Time value indicating the start of the day (from the
 // server's perspective) an extension last included a "ping" parameter during
 // its update check.
@@ -710,7 +719,7 @@ void ExtensionPrefs::RemoveDisableReason(const std::string& extension_id) {
 
 void ExtensionPrefs::UpdateBlacklist(
     const std::set<std::string>& blacklist_set) {
-  std::vector<std::string> remove_pref_ids;
+  ExtensionIdSet remove_pref_ids;
   std::set<std::string> used_id_set;
   const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
 
@@ -757,7 +766,7 @@ void ExtensionPrefs::UpdateBlacklist(
                           Value::CreateBooleanValue(true));
     }
   }
-  for (unsigned int i = 0; i < remove_pref_ids.size(); ++i) {
+  for (size_t i = 0; i < remove_pref_ids.size(); ++i) {
     DeleteExtensionPrefs(remove_pref_ids[i]);
   }
 }
@@ -1308,28 +1317,23 @@ bool ExtensionPrefs::IsExtensionDisabled(
   return DoesExtensionHaveState(id, Extension::DISABLED);
 }
 
-std::vector<std::string> ExtensionPrefs::GetToolbarOrder() {
-  ExtensionIdSet extension_ids;
-  const ListValue* toolbar_order = prefs_->GetList(kExtensionToolbar);
-  if (toolbar_order) {
-    for (size_t i = 0; i < toolbar_order->GetSize(); ++i) {
-      std::string extension_id;
-      if (toolbar_order->GetString(i, &extension_id))
-        extension_ids.push_back(extension_id);
-    }
-  }
-  return extension_ids;
+ExtensionPrefs::ExtensionIdSet ExtensionPrefs::GetToolbarOrder() {
+  bool action_box_enabled = extensions::switch_utils::IsActionBoxEnabled();
+  return GetExtensionPrefAsVector(
+      action_box_enabled ? kExtensionActionBoxBar : kExtensionToolbar);
 }
 
-void ExtensionPrefs::SetToolbarOrder(
-    const std::vector<std::string>& extension_ids) {
-  ListPrefUpdate update(prefs_, kExtensionToolbar);
-  ListValue* toolbar_order = update.Get();
-  toolbar_order->Clear();
-  for (std::vector<std::string>::const_iterator iter = extension_ids.begin();
-       iter != extension_ids.end(); ++iter) {
-    toolbar_order->Append(new StringValue(*iter));
-  }
+void ExtensionPrefs::SetToolbarOrder(const ExtensionIdSet& extension_ids) {
+  SetExtensionPrefFromVector(extensions::switch_utils::IsActionBoxEnabled() ?
+      kExtensionActionBoxBar : kExtensionToolbar, extension_ids);
+}
+
+ExtensionPrefs::ExtensionIdSet ExtensionPrefs::GetActionBoxOrder() {
+  return GetExtensionPrefAsVector(kExtensionActionBox);
+}
+
+void ExtensionPrefs::SetActionBoxOrder(const ExtensionIdSet& extension_ids) {
+  SetExtensionPrefFromVector(kExtensionActionBox, extension_ids);
 }
 
 void ExtensionPrefs::OnExtensionInstalled(
@@ -2046,6 +2050,8 @@ URLPatternSet ExtensionPrefs::GetAllowedInstallSites() {
 void ExtensionPrefs::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterDictionaryPref(kExtensionsPref, PrefService::UNSYNCABLE_PREF);
   prefs->RegisterListPref(kExtensionToolbar, PrefService::UNSYNCABLE_PREF);
+  prefs->RegisterListPref(kExtensionActionBox, PrefService::UNSYNCABLE_PREF);
+  prefs->RegisterListPref(kExtensionActionBoxBar, PrefService::UNSYNCABLE_PREF);
   prefs->RegisterIntegerPref(prefs::kExtensionToolbarSize,
                              -1,  // default value
                              PrefService::UNSYNCABLE_PREF);
@@ -2073,4 +2079,29 @@ void ExtensionPrefs::RegisterUserPrefs(PrefService* prefs) {
                           PrefService::UNSYNCABLE_PREF);
 }
 
+ExtensionPrefs::ExtensionIdSet ExtensionPrefs::GetExtensionPrefAsVector(
+    const char* pref) {
+  ExtensionIdSet extension_ids;
+  const ListValue* list_of_values = prefs_->GetList(pref);
+  if (!list_of_values)
+    return extension_ids;
+
+  std::string extension_id;
+  for (size_t i = 0; i < list_of_values->GetSize(); ++i) {
+    if (list_of_values->GetString(i, &extension_id))
+      extension_ids.push_back(extension_id);
+  }
+  return extension_ids;
+}
+
+void ExtensionPrefs::SetExtensionPrefFromVector(const char* pref,
+                                                const ExtensionIdSet& strings) {
+  ListPrefUpdate update(prefs_, pref);
+  ListValue* list_of_values = update.Get();
+  list_of_values->Clear();
+  for (ExtensionIdSet::const_iterator iter = strings.begin();
+       iter != strings.end(); ++iter)
+    list_of_values->Append(new StringValue(*iter));
+}
+
 }  // namespace extensions

chrome/browser/extensions/extension_prefs.h

@@ -60,6 +60,7 @@ class ExtensionPrefs : public ContentSettingsStore::Observer,
   typedef std::set<std::string> PrefKeySet;
 
   // Vector containing identifiers for extensions.
+  // TODO(yefim): rename to ExtensionIds.
   typedef std::vector<std::string> ExtensionIdSet;
 
   // This enum is used for the launch type the user wants to use for an
@@ -103,11 +104,13 @@ class ExtensionPrefs : public ContentSettingsStore::Observer,
   // ExtensionService::IsExtensionEnabled instead.
   bool IsExtensionDisabled(const std::string& id) const;
 
-  // Get the order that the browser actions appear in the toolbar.
-  std::vector<std::string> GetToolbarOrder();
+  // Get/Set the order that the browser actions appear in the toolbar.
+  ExtensionIdSet GetToolbarOrder();
+  void SetToolbarOrder(const ExtensionIdSet& extension_ids);
 
-  // Set the order that the browser actions appear in the toolbar.
-  void SetToolbarOrder(const std::vector<std::string>& extension_ids);
+  // Get/Set the order that the browser actions appear in the action box.
+  ExtensionIdSet GetActionBoxOrder();
+  void SetActionBoxOrder(const ExtensionIdSet& extension_ids);
 
   // Called when an extension is installed, so that prefs get created.
   // If |page_ordinal| is an invalid ordinal, then a page will be found
@@ -556,6 +559,11 @@ class ExtensionPrefs : public ContentSettingsStore::Observer,
   bool DoesExtensionHaveState(const std::string& id,
                               Extension::State check_state) const;
 
+  // Helper function to Get/Set array of strings from/to prefs.
+  ExtensionIdSet GetExtensionPrefAsVector(const char* pref);
+  void SetExtensionPrefFromVector(const char* pref,
+                                  const ExtensionIdSet& extension_ids);
+
   // The pref service specific to this set of extension prefs. Owned by profile.
   PrefService* prefs_;
 

chrome/browser/extensions/extension_toolbar_model.h

@@ -8,6 +8,7 @@
 #include "base/compiler_specific.h"
 #include "base/observer_list.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/browser/extensions/extension_prefs.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
@@ -72,25 +73,19 @@ class ExtensionToolbarModel : public content::NotificationObserver {
 
   bool extensions_initialized() const { return extensions_initialized_; }
 
-  size_t size() const {
-    return toolitems_.size();
+  const extensions::ExtensionList& toolbar_items() const {
+    return toolbar_items_;
   }
 
-  extensions::ExtensionList::iterator begin() {
-    return toolitems_.begin();
-  }
-
-  extensions::ExtensionList::iterator end() {
-    return toolitems_.end();
-  }
-
-  const extensions::Extension* GetExtensionByIndex(int index) const;
-
   // Utility functions for converting between an index into the list of
   // incognito-enabled browser actions, and the list of all browser actions.
   int IncognitoIndexToOriginal(int incognito_index);
   int OriginalIndexToIncognito(int original_index);
 
+  const extensions::ExtensionList& action_box_menu_items() const {
+    return action_box_menu_items_;
+  }
+
  private:
   // content::NotificationObserver implementation.
   virtual void Observe(int type,
@@ -99,8 +94,15 @@ class ExtensionToolbarModel : public content::NotificationObserver {
 
   // To be called after the extension service is ready; gets loaded extensions
   // from the extension service and their saved order from the pref service
-  // and constructs |toolitems_| from these data.
-  void InitializeExtensionList();
+  // and constructs |toolbar_items_| and |action_box_items_| from these data.
+  void InitializeExtensionLists();
+  void PopulateForActionBoxMode();
+  void PopulateForNonActionBoxMode();
+
+  // Fills |list| with extensions based on provided |order|.
+  void FillExtensionList(
+      const extensions::ExtensionPrefs::ExtensionIdSet& order,
+      extensions::ExtensionList* list);
 
   // Save the model to prefs.
   void UpdatePrefs();
@@ -108,8 +110,16 @@ class ExtensionToolbarModel : public content::NotificationObserver {
   // Our observers.
   ObserverList<Observer> observers_;
 
-  void AddExtension(const extensions::Extension* extension);
-  void RemoveExtension(const extensions::Extension* extension);
+  void AddExtension(const extensions::Extension* extension,
+                    extensions::ExtensionList* list);
+  void RemoveExtension(const extensions::Extension* extension,
+                       extensions::ExtensionList* list);
+
+  // Searches for the given |extension| in |toolbar_items_| and
+  // |action_box_menu_items_| and returns pointer to the list with it
+  // or NULL if not found.
+  extensions::ExtensionList* FindListWithExtension(
+      const extensions::Extension* extension);
 
   // Our ExtensionService, guaranteed to outlive us.
   ExtensionService* service_;
@@ -120,7 +130,10 @@ class ExtensionToolbarModel : public content::NotificationObserver {
   bool extensions_initialized_;
 
   // Ordered list of browser action buttons.
-  extensions::ExtensionList toolitems_;
+  extensions::ExtensionList toolbar_items_;
+
+  // List of browser action buttons visible in an action box menu.
+  extensions::ExtensionList action_box_menu_items_;
 
   // Keeps track of what the last extension to get disabled was.
   std::string last_extension_removed_;

chrome/browser/extensions/extension_toolbar_model_browsertest.cc

@@ -50,8 +50,9 @@ class ExtensionToolbarModelTest : public ExtensionBrowserTest,
   }
 
   const Extension* ExtensionAt(int index) {
-    for (extensions::ExtensionList::iterator i = model_->begin();
-         i < model_->end(); ++i) {
+    const extensions::ExtensionList& toolbar_items = model_->toolbar_items();
+    for (extensions::ExtensionList::const_iterator i = toolbar_items.begin();
+         i < toolbar_items.end(); ++i) {
       if (index-- == 0)
         return *i;
     }
@@ -77,7 +78,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, Basic) {
 
   // This extension should not be in the model (has no browser action).
   EXPECT_EQ(0, inserted_count_);
-  EXPECT_EQ(0u, model_->size());
+  EXPECT_EQ(0u, model_->toolbar_items().size());
   ASSERT_EQ(NULL, ExtensionAt(0));
 
   // Load an extension with a browser action.
@@ -87,7 +88,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, Basic) {
 
   // We should now find our extension in the model.
   EXPECT_EQ(1, inserted_count_);
-  EXPECT_EQ(1u, model_->size());
+  EXPECT_EQ(1u, model_->toolbar_items().size());
   const Extension* extension = ExtensionAt(0);
   ASSERT_TRUE(NULL != extension);
   EXPECT_STREQ("A browser action with no icon that makes the page red",
@@ -96,13 +97,13 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, Basic) {
   // Should be a no-op, but still fires the events.
   model_->MoveBrowserAction(extension, 0);
   EXPECT_EQ(1, moved_count_);
-  EXPECT_EQ(1u, model_->size());
+  EXPECT_EQ(1u, model_->toolbar_items().size());
   const Extension* extension2 = ExtensionAt(0);
   EXPECT_EQ(extension, extension2);
 
   UnloadExtension(extension->id());
   EXPECT_EQ(1, removed_count_);
-  EXPECT_EQ(0u, model_->size());
+  EXPECT_EQ(0u, model_->toolbar_items().size());
   EXPECT_EQ(NULL, ExtensionAt(0));
 }
 
@@ -118,7 +119,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
 
   // First extension loaded.
   EXPECT_EQ(1, inserted_count_);
-  EXPECT_EQ(1u, model_->size());
+  EXPECT_EQ(1u, model_->toolbar_items().size());
   const Extension* extensionA = ExtensionAt(0);
   ASSERT_TRUE(NULL != extensionA);
   EXPECT_STREQ("A browser action with no icon that makes the page red",
@@ -132,7 +133,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
 
   // Second extension loaded.
   EXPECT_EQ(2, inserted_count_);
-  EXPECT_EQ(2u, model_->size());
+  EXPECT_EQ(2u, model_->toolbar_items().size());
   const Extension* extensionB = ExtensionAt(1);
   ASSERT_TRUE(NULL != extensionB);
   EXPECT_STREQ("Popup tester", extensionB->name().c_str());
@@ -145,7 +146,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
 
   // Third extension loaded.
   EXPECT_EQ(3, inserted_count_);
-  EXPECT_EQ(3u, model_->size());
+  EXPECT_EQ(3u, model_->toolbar_items().size());
   const Extension* extensionC = ExtensionAt(2);
   ASSERT_TRUE(NULL != extensionC);
   EXPECT_STREQ("A page action which removes a popup.",
@@ -154,7 +155,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
   // Order is now A, B, C. Let's put C first.
   model_->MoveBrowserAction(extensionC, 0);
   EXPECT_EQ(1, moved_count_);
-  EXPECT_EQ(3u, model_->size());
+  EXPECT_EQ(3u, model_->toolbar_items().size());
   EXPECT_EQ(extensionC, ExtensionAt(0));
   EXPECT_EQ(extensionA, ExtensionAt(1));
   EXPECT_EQ(extensionB, ExtensionAt(2));
@@ -163,7 +164,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
   // Order is now C, A, B. Let's put A last.
   model_->MoveBrowserAction(extensionA, 2);
   EXPECT_EQ(2, moved_count_);
-  EXPECT_EQ(3u, model_->size());
+  EXPECT_EQ(3u, model_->toolbar_items().size());
   EXPECT_EQ(extensionC, ExtensionAt(0));
   EXPECT_EQ(extensionB, ExtensionAt(1));
   EXPECT_EQ(extensionA, ExtensionAt(2));
@@ -173,7 +174,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
   std::string idB = extensionB->id();
   UnloadExtension(idB);
   EXPECT_EQ(1, removed_count_);
-  EXPECT_EQ(2u, model_->size());
+  EXPECT_EQ(2u, model_->toolbar_items().size());
   EXPECT_EQ(extensionC, ExtensionAt(0));
   EXPECT_EQ(extensionA, ExtensionAt(1));
   EXPECT_EQ(NULL, ExtensionAt(2));
@@ -183,7 +184,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
 
   // Extension B loaded again.
   EXPECT_EQ(4, inserted_count_);
-  EXPECT_EQ(3u, model_->size());
+  EXPECT_EQ(3u, model_->toolbar_items().size());
   // Make sure it gets its old spot in the list. We should get the same
   // extension again, otherwise the order has changed.
   ASSERT_STREQ(idB.c_str(), ExtensionAt(1)->id().c_str());
@@ -191,7 +192,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
   // Unload B again.
   UnloadExtension(idB);
   EXPECT_EQ(2, removed_count_);
-  EXPECT_EQ(2u, model_->size());
+  EXPECT_EQ(2u, model_->toolbar_items().size());
   EXPECT_EQ(extensionC, ExtensionAt(0));
   EXPECT_EQ(extensionA, ExtensionAt(1));
   EXPECT_EQ(NULL, ExtensionAt(2));
@@ -199,7 +200,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
   // Order is now C, A. Flip it.
   model_->MoveBrowserAction(extensionA, 0);
   EXPECT_EQ(3, moved_count_);
-  EXPECT_EQ(2u, model_->size());
+  EXPECT_EQ(2u, model_->toolbar_items().size());
   EXPECT_EQ(extensionA, ExtensionAt(0));
   EXPECT_EQ(extensionC, ExtensionAt(1));
   EXPECT_EQ(NULL, ExtensionAt(2));
@@ -207,7 +208,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
   // Move A to the location it already occupies.
   model_->MoveBrowserAction(extensionA, 0);
   EXPECT_EQ(4, moved_count_);
-  EXPECT_EQ(2u, model_->size());
+  EXPECT_EQ(2u, model_->toolbar_items().size());
   EXPECT_EQ(extensionA, ExtensionAt(0));
   EXPECT_EQ(extensionC, ExtensionAt(1));
   EXPECT_EQ(NULL, ExtensionAt(2));
@@ -216,7 +217,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
   std::string idC = extensionC->id();
   UnloadExtension(idC);
   EXPECT_EQ(3, removed_count_);
-  EXPECT_EQ(1u, model_->size());
+  EXPECT_EQ(1u, model_->toolbar_items().size());
   EXPECT_EQ(extensionA, ExtensionAt(0));
   EXPECT_EQ(NULL, ExtensionAt(1));
 
@@ -225,7 +226,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionToolbarModelTest, ReorderAndReinsert) {
 
   // Extension C loaded again.
   EXPECT_EQ(5, inserted_count_);
-  EXPECT_EQ(2u, model_->size());
+  EXPECT_EQ(2u, model_->toolbar_items().size());
   // Make sure it gets its old spot in the list (at the very end).
   ASSERT_STREQ(idC.c_str(), ExtensionAt(1)->id().c_str());
 }

chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm

@@ -443,8 +443,9 @@ class ExtensionServiceObserverBridge : public content::NotificationObserver,
     return;
 
   NSUInteger i = 0;
-  for (ExtensionList::iterator iter = toolbarModel_->begin();
-       iter != toolbarModel_->end(); ++iter) {
+  for (ExtensionList::const_iterator iter =
+           toolbarModel_->toolbar_items().begin();
+       iter != toolbarModel_->toolbar_items().end(); ++iter) {
     if (![self shouldDisplayBrowserAction:*iter])
       continue;
 
@@ -533,8 +534,9 @@ class ExtensionServiceObserverBridge : public content::NotificationObserver,
 
 - (void)positionActionButtonsAndAnimate:(BOOL)animate {
   NSUInteger i = 0;
-  for (ExtensionList::iterator iter = toolbarModel_->begin();
-       iter != toolbarModel_->end(); ++iter) {
+  for (ExtensionList::const_iterator iter =
+           toolbarModel_->toolbar_items().begin();
+       iter != toolbarModel_->toolbar_items().end(); ++iter) {
     if (![self shouldDisplayBrowserAction:*iter])
       continue;
     BrowserActionButton* button = [self buttonForExtension:(*iter)];
@@ -624,8 +626,9 @@ class ExtensionServiceObserverBridge : public content::NotificationObserver,
 }
 
 - (void)containerDragFinished:(NSNotification*)notification {
-  for (ExtensionList::iterator iter = toolbarModel_->begin();
-       iter != toolbarModel_->end(); ++iter) {
+  for (ExtensionList::const_iterator iter =
+           toolbarModel_->toolbar_items().begin();
+       iter != toolbarModel_->toolbar_items().end(); ++iter) {
     BrowserActionButton* button = [self buttonForExtension:(*iter)];
     NSRect buttonFrame = [button frame];
     if (NSContainsRect([containerView_ bounds], buttonFrame))
@@ -663,8 +666,9 @@ class ExtensionServiceObserverBridge : public content::NotificationObserver,
   NSRect draggedButtonFrame = [draggedButton frame];
 
   NSUInteger index = 0;
-  for (ExtensionList::iterator iter = toolbarModel_->begin();
-       iter != toolbarModel_->end(); ++iter) {
+  for (ExtensionList::const_iterator iter =
+           toolbarModel_->toolbar_items().begin();
+       iter != toolbarModel_->toolbar_items().end(); ++iter) {
     BrowserActionButton* button = [self buttonForExtension:(*iter)];
     CGFloat intersectionWidth =
         NSWidth(NSIntersectionRect(draggedButtonFrame, [button frame]));
@@ -826,8 +830,10 @@ class ExtensionServiceObserverBridge : public content::NotificationObserver,
 - (NSButton*)buttonWithIndex:(NSUInteger)index {
   if (profile_->IsOffTheRecord())
     index = toolbarModel_->IncognitoIndexToOriginal(index);
-  if (index < toolbarModel_->size()) {
-    const Extension* extension = toolbarModel_->GetExtensionByIndex(index);
+  const extensions::ExtensionList& toolbar_items =
+      toolbarModel_->toolbar_items();
+  if (index < toolbar_items.size()) {
+    const Extension* extension = toolbar_items[index];
     return [buttons_ objectForKey:base::SysUTF8ToNSString(extension->id())];
   }
   return nil;

chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc

@@ -626,8 +626,9 @@ void BrowserActionsToolbarGtk::CreateAllButtons() {
   extension_button_map_.clear();
 
   int i = 0;
-  for (extensions::ExtensionList::iterator iter = model_->begin();
-       iter != model_->end(); ++iter) {
+  const extensions::ExtensionList& toolbar_items = model_->toolbar_items();
+  for (extensions::ExtensionList::const_iterator iter = toolbar_items.begin();
+       iter != toolbar_items.end(); ++iter) {
     CreateButtonForExtension(*iter, i++);
   }
 }
@@ -797,7 +798,7 @@ bool BrowserActionsToolbarGtk::IsCommandIdChecked(int command_id) const {
 }
 
 bool BrowserActionsToolbarGtk::IsCommandIdEnabled(int command_id) const {
-  const Extension* extension = model_->GetExtensionByIndex(command_id);
+  const Extension* extension = model_->toolbar_items()[command_id];
   return extension->browser_action()->GetIsVisible(GetCurrentTabId());
 }
 
@@ -808,7 +809,7 @@ bool BrowserActionsToolbarGtk::GetAcceleratorForCommandId(
 }
 
 void BrowserActionsToolbarGtk::ExecuteCommand(int command_id) {
-  const Extension* extension = model_->GetExtensionByIndex(command_id);
+  const Extension* extension = model_->toolbar_items()[command_id];
   GURL popup_url;
 
   switch (model_->ExecuteBrowserAction(extension, browser(), &popup_url)) {
@@ -1039,7 +1040,7 @@ gboolean BrowserActionsToolbarGtk::OnOverflowButtonPress(
     if (profile_->IsOffTheRecord())
       model_index = model_->IncognitoIndexToOriginal(i);
 
-    const Extension* extension = model_->GetExtensionByIndex(model_index);
+    const Extension* extension = model_->toolbar_items()[model_index];
     BrowserActionButton* button = extension_button_map_[extension->id()].get();
 
     overflow_menu_model_->AddItem(model_index, UTF8ToUTF16(extension->name()));
@@ -1079,7 +1080,7 @@ gboolean BrowserActionsToolbarGtk::OnOverflowMenuButtonPress(
   if (profile_->IsOffTheRecord())
     item_index = model_->IncognitoIndexToOriginal(item_index);
 
-  const Extension* extension = model_->GetExtensionByIndex(item_index);
+  const Extension* extension = model_->toolbar_items()[item_index];
   ExtensionButtonMap::iterator it = extension_button_map_.find(
       extension->id());
   if (it == extension_button_map_.end()) {

chrome/browser/ui/toolbar/action_box_menu_model.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/ui/toolbar/action_box_menu_model.h"
+
+#include "base/utf_string_conversions.h"
+#include "chrome/browser/extensions/extension_toolbar_model.h"
+
+// Arbitrary number just to leave enough space for menu IDs
+// that show before extensions. Like "Bookmark this page", "Send tab to device"
+// and so on. They could have any IDs < kFirstExtensionCommandId.
+static const int kFirstExtensionCommandId = 1000;
+
+////////////////////////////////////////////////////////////////////////////////
+// ActionBoxMenuModel
+
+ActionBoxMenuModel::ActionBoxMenuModel(ExtensionService* extension_service)
+    : ALLOW_THIS_IN_INITIALIZER_LIST(ui::SimpleMenuModel(NULL)),
+      extension_service_(extension_service) {
+  // Adds extensions to the model.
+  int command_id = kFirstExtensionCommandId;
+  const extensions::ExtensionList& action_box_items = action_box_menu_items();
+  for (size_t i = 0; i < action_box_items.size(); ++i) {
+    const extensions::Extension* extension = action_box_items[i];
+    AddItem(command_id, UTF8ToUTF16(extension->name()));
+    id_to_extension_id_map_[command_id++] = extension->id();
+  }
+}
+
+ActionBoxMenuModel::~ActionBoxMenuModel() {
+}
+
+void ActionBoxMenuModel::Observe(int type,
+                                 const content::NotificationSource& source,
+                                 const content::NotificationDetails& details) {
+}

chrome/browser/ui/toolbar/action_box_menu_model.h

+// 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.
+
+#ifndef CHROME_BROWSER_UI_TOOLBAR_ACTION_BOX_MENU_MODEL_H_
+#define CHROME_BROWSER_UI_TOOLBAR_ACTION_BOX_MENU_MODEL_H_
+
+#include <map>
+
+#include "content/public/browser/notification_observer.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "ui/base/models/simple_menu_model.h"
+
+namespace extensions {
+class Extension;
+}
+
+// A menu model that builds the contents of the action box menu.
+class ActionBoxMenuModel : public ui::SimpleMenuModel,
+                           public content::NotificationObserver {
+ public:
+  explicit ActionBoxMenuModel(ExtensionService* extension_service);
+  virtual ~ActionBoxMenuModel();
+
+  const extensions::ExtensionList& action_box_menu_items() {
+    return extension_service_->toolbar_model()->action_box_menu_items();
+  }
+
+ private:
+  typedef std::map<int, std::string> IdToEntensionIdMap;
+
+  // Overridden from content::NotificationObserver:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  ExtensionService* extension_service_;
+
+  IdToEntensionIdMap id_to_extension_id_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(ActionBoxMenuModel);
+};
+
+#endif  // CHROME_BROWSER_UI_TOOLBAR_ACTION_BOX_MENU_MODEL_H_

chrome/browser/ui/views/action_box_menu.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/ui/views/action_box_menu.h"
+
+#include "chrome/browser/ui/toolbar/action_box_menu_model.h"
+#include "chrome/browser/ui/views/browser_action_view.h"
+#include "grit/generated_resources.h"
+#include "grit/theme_resources.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/views/bubble/bubble_border.h"
+#include "ui/views/controls/button/menu_button.h"
+#include "ui/views/controls/menu/menu_runner.h"
+#include "ui/views/view.h"
+
+#if defined(OS_WIN)
+// Included for MENU_POPUPITEM and a few other Windows specific constants.
+#include <vssym32.h>
+#include "ui/base/native_theme/native_theme_win.h"
+#endif
+
+////////////////////////////////////////////////////////////////////////////////
+// ActionBoxMenu
+
+ActionBoxMenu::ActionBoxMenu(Browser* browser,
+                             ActionBoxMenuModel* model,
+                             bool starred)
+    : browser_(browser),
+      root_(NULL),
+      model_(model),
+      starred_(starred) {
+}
+
+ActionBoxMenu::~ActionBoxMenu() {
+}
+
+void ActionBoxMenu::Init() {
+  DCHECK(!root_);
+  root_ = new views::MenuItemView(this);
+  root_->set_has_icons(true);
+  PopulateMenu();
+}
+
+void ActionBoxMenu::RunMenu(views::MenuButton* menu_button) {
+  gfx::Point screen_location;
+  views::View::ConvertPointToScreen(menu_button, &screen_location);
+  menu_runner_.reset(new views::MenuRunner(root_));
+  // Ignore the result since we don't need to handle a deleted menu specially.
+  ignore_result(
+      menu_runner_->RunMenuAt(menu_button->GetWidget(),
+                              menu_button,
+                              gfx::Rect(screen_location, menu_button->size()),
+                              views::MenuItemView::TOPRIGHT,
+                              views::MenuRunner::HAS_MNEMONICS));
+}
+
+void ActionBoxMenu::ExecuteCommand(int id) {
+};
+
+views::Border* ActionBoxMenu::CreateMenuBorder() {
+  // TODO(yefim): Use correct theme color on non-Windows.
+  SkColor border_color = SK_ColorBLACK;
+#if defined(OS_WIN)
+  // TODO(yefim): Move to Windows only files if possible.
+  border_color = ui::NativeThemeWin::instance()->GetThemeColorWithDefault(
+      ui::NativeThemeWin::MENU, MENU_POPUPITEM, MPI_NORMAL, TMT_TEXTCOLOR,
+      COLOR_MENUTEXT);
+#endif
+  return views::Border::CreateSolidBorder(1, border_color);
+}
+
+views::Background* ActionBoxMenu::CreateMenuBackground() {
+  // TODO(yefim): Use correct theme color on non-Windows.
+  SkColor background_color = SK_ColorWHITE;
+#if defined(OS_WIN)
+  // TODO(yefim): Move to Windows only files if possible.
+  background_color = ui::NativeThemeWin::instance()->GetThemeColorWithDefault(
+      ui::NativeThemeWin::TEXTFIELD, EP_BACKGROUND, EBS_NORMAL,
+      TMT_BACKGROUND, COLOR_WINDOW);
+#endif
+  return views::Background::CreateSolidBackground(background_color);
+}
+
+int ActionBoxMenu::GetCurrentTabId() const {
+  return 0;
+}
+
+void ActionBoxMenu::OnBrowserActionExecuted(BrowserActionButton* button) {
+}
+
+void ActionBoxMenu::OnBrowserActionVisibilityChanged() {
+}
+
+gfx::Point ActionBoxMenu::GetViewContentOffset() const {
+  return gfx::Point(0, 0);
+}
+
+bool ActionBoxMenu::NeedToShowMultipleIconStates() const {
+  return false;
+}
+
+bool ActionBoxMenu::NeedToShowTooltip() const {
+  return false;
+}
+
+void ActionBoxMenu::WriteDragDataForView(views::View* sender,
+                                         const gfx::Point& press_pt,
+                                         ui::OSExchangeData* data) {
+}
+
+int ActionBoxMenu::GetDragOperationsForView(views::View* sender,
+                                            const gfx::Point& p) {
+  return 0;
+}
+
+bool ActionBoxMenu::CanStartDragForView(views::View* sender,
+                                        const gfx::Point& press_pt,
+                                        const gfx::Point& p) {
+  return false;
+}
+
+void ActionBoxMenu::Observe(int type,
+                            const content::NotificationSource& source,
+                            const content::NotificationDetails& details) {
+}
+
+void ActionBoxMenu::PopulateMenu() {
+  int item_id = 1;
+  AddBookmarkMenuItem(root_, &item_id);
+  if (model_->GetItemCount() > 0)
+    root_->AppendSeparator();
+
+  const extensions::ExtensionList& action_box_items =
+      model_->action_box_menu_items();
+  for (size_t model_index = 0; model_index < action_box_items.size();
+       ++model_index) {
+    DCHECK(model_->GetTypeAt(model_index) == ui::MenuModel::TYPE_COMMAND);
+    views::MenuItemView* menu_item = root_->AppendMenuItemFromModel(
+        model_, model_index, item_id + model_index);
+    menu_item->SetMargins(0, 0);
+    const extensions::Extension* extension = action_box_items[model_index];
+    BrowserActionView* view = new BrowserActionView(extension,
+        browser_, this);
+    browser_action_views_.push_back(view);
+    menu_item->SetIconView(view);
+  }
+}
+
+views::MenuItemView* ActionBoxMenu::AddBookmarkMenuItem(
+    views::MenuItemView* parent,
+    int* item_id) {
+  string16 label = l10n_util::GetStringUTF16(starred_ ?
+      IDS_TOOLTIP_STARRED : IDS_TOOLTIP_STAR);
+  gfx::ImageSkia* icon =
+      ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+          starred_ ? IDR_STAR_LIT : IDR_STAR);
+  views::MenuItemView* item =
+      parent->AppendMenuItemWithIcon(*item_id, label, *icon);
+  (*item_id)++;
+  return item;
+}

chrome/browser/ui/views/action_box_menu.h

+// 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_ACTION_BOX_MENU_H_
+#define CHROME_BROWSER_UI_VIEWS_ACTION_BOX_MENU_H_
+
+#include <map>
+
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "chrome/browser/ui/views/browser_action_view.h"
+#include "content/public/browser/notification_observer.h"
+#include "ui/views/controls/menu/menu_delegate.h"
+
+class ActionBoxMenuModel;
+
+namespace views {
+class Background;
+class Border;
+class MenuItemView;
+class MenuRunner;
+class View;
+}
+
+// ActionBoxMenu adapts the ActionBoxMenuModel to view's menu related classes.
+class ActionBoxMenu : public views::MenuDelegate,
+                      public BrowserActionView::Delegate,
+                      public content::NotificationObserver {
+ public:
+  ActionBoxMenu(Browser* browser,
+                ActionBoxMenuModel* model,
+                bool starred);
+  virtual ~ActionBoxMenu();
+
+  void Init();
+
+  // Shows the menu relative to the specified button.
+  void RunMenu(views::MenuButton* menu_button);
+
+ private:
+  // Overridden from views::MenuDelegate:
+  virtual void ExecuteCommand(int id) OVERRIDE;
+  virtual views::Border* CreateMenuBorder() OVERRIDE;
+  virtual views::Background* CreateMenuBackground() OVERRIDE;
+
+  // Overridden from BrowserActionView::Delegate and DragController overrides:
+  virtual int GetCurrentTabId() const OVERRIDE;
+  virtual void OnBrowserActionExecuted(BrowserActionButton* button) OVERRIDE;
+  virtual void OnBrowserActionVisibilityChanged() OVERRIDE;
+  virtual gfx::Point GetViewContentOffset() const OVERRIDE;
+  virtual bool NeedToShowMultipleIconStates() const OVERRIDE;
+  virtual bool NeedToShowTooltip() const OVERRIDE;
+  virtual void WriteDragDataForView(views::View* sender,
+                                    const gfx::Point& press_pt,
+                                    ui::OSExchangeData* data) OVERRIDE;
+  virtual int GetDragOperationsForView(views::View* sender,
+                                       const gfx::Point& p) OVERRIDE;
+  virtual bool CanStartDragForView(views::View* sender,
+                                   const gfx::Point& press_pt,
+                                   const gfx::Point& p) OVERRIDE;
+
+  // NotificationObserver overrides:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  // Adds a new bookmark menu item to the menu, |next_id| is incremented.
+  views::MenuItemView* AddBookmarkMenuItem(views::MenuItemView* parent,
+                                           int* next_id);
+
+  // Populates |root_| with all the child menu items from the |model_|.
+  void PopulateMenu();
+
+  Browser* browser_;
+
+  // The views menu. Owned by |menu_runner_|.
+  views::MenuItemView* root_;
+
+  scoped_ptr<views::MenuRunner> menu_runner_;
+
+  // The model that tracks the order of the toolbar icons.
+  ActionBoxMenuModel* model_;
+
+  // Set when the current page is bookmarked and thus the star icon should be
+  // drawn in the "starred" rather than "unstarred" state.
+  bool starred_;
+
+  ScopedVector<BrowserActionView> browser_action_views_;
+
+  DISALLOW_COPY_AND_ASSIGN(ActionBoxMenu);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_ACTION_BOX_MENU_H_

chrome/browser/ui/views/browser_action_view.h

@@ -12,15 +12,91 @@
 #include "ui/views/context_menu_controller.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/button/menu_button_listener.h"
+#include "ui/views/drag_controller.h"
 #include "ui/views/view.h"
 
-class BrowserActionsContainer;
+class Browser;
+class BrowserActionButton;
 class ExtensionAction;
 
+namespace extensions {
+class Extension;
+}
+
 namespace views {
 class MenuItemView;
+class MenuRunner;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// BrowserActionView
+// A single entry in the browser action container. This contains the actual
+// BrowserActionButton, as well as the logic to paint the badge.
+class BrowserActionView : public views::View {
+ public:
+  // Need DragController here because BrowserActionView could be
+  // dragged/dropped.
+  class Delegate : public views::DragController {
+   public:
+    // Returns the current tab's ID, or -1 if there is no current tab.
+    virtual int GetCurrentTabId() const = 0;
+
+    // Called when the user clicks on the browser action icon.
+    virtual void OnBrowserActionExecuted(BrowserActionButton* button) = 0;
+
+    // Called when a browser action becomes visible/hidden.
+    virtual void OnBrowserActionVisibilityChanged() = 0;
+
+    // Returns relative position of a button inside BrowserActionView.
+    virtual gfx::Point GetViewContentOffset() const = 0;
+
+    virtual bool NeedToShowMultipleIconStates() const;
+    virtual bool NeedToShowTooltip() const;
+
+   protected:
+    virtual ~Delegate() {}
+  };
+
+  BrowserActionView(const extensions::Extension* extension,
+                    Browser* browser,
+                    Delegate* delegate);
+  virtual ~BrowserActionView();
+
+  BrowserActionButton* button() { return button_; }
+
+  // Allocates a canvas object on the heap and draws into it the icon for the
+  // view as well as the badge (if any). Caller is responsible for deleting the
+  // returned object.
+  gfx::Canvas* GetIconWithBadge();
+
+  // Overridden from views::View:
+  virtual void Layout() OVERRIDE;
+  virtual void GetAccessibleState(ui::AccessibleViewState* state) OVERRIDE;
+  virtual gfx::Size GetPreferredSize() OVERRIDE;
+
+ protected:
+  // Overridden from views::View to paint the badge on top of children.
+  virtual void PaintChildren(gfx::Canvas* canvas) OVERRIDE;
+  virtual void ViewHierarchyChanged(bool is_add,
+                                    View* parent,
+                                    View* child) OVERRIDE;
+
+ private:
+  // The Browser object this view is associated with.
+  Browser* browser_;
+
+  // Usually a container for this view.
+  Delegate* delegate_;
+
+  // The button this view contains.
+  BrowserActionButton* button_;
+
+  // Extension this view associated with.
+  const extensions::Extension* extension_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserActionView);
+};
+
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserActionButton
 
@@ -34,7 +110,8 @@ class BrowserActionButton : public views::MenuButton,
                             public content::NotificationObserver {
  public:
   BrowserActionButton(const extensions::Extension* extension,
-                      BrowserActionsContainer* panel);
+                      Browser* browser_,
+                      BrowserActionView::Delegate* delegate);
 
   // Call this instead of delete.
   void Destroy();
@@ -111,6 +188,9 @@ class BrowserActionButton : public views::MenuButton,
   // it is active.
   void MaybeUnregisterExtensionCommand(bool only_if_active);
 
+  // The Browser object this button is associated with.
+  Browser* browser_;
+
   // The browser action this view represents. The ExtensionAction is not owned
   // by this class.
   ExtensionAction* browser_action_;
@@ -122,58 +202,28 @@ class BrowserActionButton : public views::MenuButton,
   // asynchronously.
   ImageLoadingTracker tracker_;
 
-  // The browser action shelf.
-  BrowserActionsContainer* panel_;
+  // The default icon for our browser action. This might be non-empty if the
+  // browser action had a value for default_icon in the manifest.
+  SkBitmap default_icon_;
+
+  // Delegate that usually represents a container for BrowserActionView.
+  BrowserActionView::Delegate* delegate_;
 
   // The context menu.  This member is non-NULL only when the menu is shown.
   views::MenuItemView* context_menu_;
 
   content::NotificationRegistrar registrar_;
 
-  // The extension keybinding accelerator this browser action is listening for
+  // The extension key binding accelerator this browser action is listening for
   // (to show the popup).
   scoped_ptr<ui::Accelerator> keybinding_;
 
+  // Responsible for running the menu.
+  scoped_ptr<views::MenuRunner> menu_runner_;
+
   friend class base::DeleteHelper<BrowserActionButton>;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserActionButton);
 };
 
-
-////////////////////////////////////////////////////////////////////////////////
-// BrowserActionView
-// A single section in the browser action container. This contains the actual
-// BrowserActionButton, as well as the logic to paint the badge.
-
-class BrowserActionView : public views::View {
- public:
-  BrowserActionView(const extensions::Extension* extension,
-                    BrowserActionsContainer* panel);
-  virtual ~BrowserActionView();
-
-  BrowserActionButton* button() { return button_; }
-
-  // Allocates a canvas object on the heap and draws into it the icon for the
-  // view as well as the badge (if any). Caller is responsible for deleting the
-  // returned object.
-  gfx::Canvas* GetIconWithBadge();
-
-  // Overridden from views::View:
-  virtual void Layout() OVERRIDE;
-  virtual void GetAccessibleState(ui::AccessibleViewState* state) OVERRIDE;
-
- protected:
-  // Overridden from views::View to paint the badge on top of children.
-  virtual void PaintChildren(gfx::Canvas* canvas) OVERRIDE;
-
- private:
-  // The container for this view.
-  BrowserActionsContainer* panel_;
-
-  // The button this view contains.
-  BrowserActionButton* button_;
-
-  DISALLOW_COPY_AND_ASSIGN(BrowserActionView);
-};
-
 #endif  // CHROME_BROWSER_UI_VIEWS_BROWSER_ACTION_VIEW_H_

chrome/browser/ui/views/browser_actions_container.cc

@@ -128,11 +128,6 @@ void BrowserActionsContainer::Init() {
     SetContainerWidth();
 }
 
-int BrowserActionsContainer::GetCurrentTabId() const {
-  TabContents* tab = chrome::GetActiveTabContents(browser_);
-  return tab ? tab->restore_tab_helper()->session_id().id() : -1;
-}
-
 BrowserActionView* BrowserActionsContainer::GetBrowserActionView(
     ExtensionAction* action) {
   for (BrowserActionViews::iterator i(browser_action_views_.begin());
@@ -153,12 +148,13 @@ void BrowserActionsContainer::CreateBrowserActionViews() {
   if (!model_)
     return;
 
-  for (extensions::ExtensionList::iterator i(model_->begin());
-       i != model_->end(); ++i) {
+  const extensions::ExtensionList& toolbar_items = model_->toolbar_items();
+  for (extensions::ExtensionList::const_iterator i(toolbar_items.begin());
+       i != toolbar_items.end(); ++i) {
     if (!ShouldDisplayBrowserAction(*i))
       continue;
 
-    BrowserActionView* view = new BrowserActionView(*i, this);
+    BrowserActionView* view = new BrowserActionView(*i, browser_, this);
     browser_action_views_.push_back(view);
     AddChildView(view);
   }
@@ -169,12 +165,6 @@ void BrowserActionsContainer::DeleteBrowserActionViews() {
   STLDeleteElements(&browser_action_views_);
 }
 
-void BrowserActionsContainer::OnBrowserActionVisibilityChanged() {
-  SetVisible(!browser_action_views_.empty());
-  owner_view_->Layout();
-  owner_view_->SchedulePaint();
-}
-
 size_t BrowserActionsContainer::VisibleBrowserActions() const {
   size_t visible_actions = 0;
   for (size_t i = 0; i < browser_action_views_.size(); ++i) {
@@ -184,19 +174,6 @@ size_t BrowserActionsContainer::VisibleBrowserActions() const {
   return visible_actions;
 }
 
-void BrowserActionsContainer::OnBrowserActionExecuted(
-    BrowserActionButton* button) {
-  const Extension* extension = button->extension();
-  GURL popup_url;
-  switch (model_->ExecuteBrowserAction(extension, browser_, &popup_url)) {
-    case ExtensionToolbarModel::ACTION_NONE:
-      break;
-    case ExtensionToolbarModel::ACTION_SHOW_POPUP:
-      ShowPopup(button, popup_url);
-      break;
-  }
-}
-
 gfx::Size BrowserActionsContainer::GetPreferredSize() {
   if (browser_action_views_.empty())
     return gfx::Size(ToolbarView::kStandardSpacing, 0);
@@ -388,7 +365,8 @@ void BrowserActionsContainer::OnMenuButtonClicked(views::View* source,
                                                   const gfx::Point& point) {
   if (source == chevron_) {
     overflow_menu_ = new BrowserActionOverflowMenuController(
-        this, chevron_, browser_action_views_, VisibleBrowserActions());
+        this, browser_, chevron_, browser_action_views_,
+        VisibleBrowserActions());
     overflow_menu_->set_observer(this);
     overflow_menu_->RunMenu(GetWidget(), false);
   }
@@ -478,6 +456,30 @@ void BrowserActionsContainer::OnWidgetClosing(views::Widget* widget) {
   }
 }
 
+int BrowserActionsContainer::GetCurrentTabId() const {
+  TabContents* tab = chrome::GetActiveTabContents(browser_);
+  return tab ? tab->restore_tab_helper()->session_id().id() : -1;
+}
+
+void BrowserActionsContainer::OnBrowserActionExecuted(
+    BrowserActionButton* button) {
+  const Extension* extension = button->extension();
+  GURL popup_url;
+  if (model_->ExecuteBrowserAction(extension, browser_, &popup_url) ==
+      ExtensionToolbarModel::ACTION_SHOW_POPUP)
+    ShowPopup(button, popup_url);
+}
+
+void BrowserActionsContainer::OnBrowserActionVisibilityChanged() {
+  SetVisible(!browser_action_views_.empty());
+  owner_view_->Layout();
+  owner_view_->SchedulePaint();
+}
+
+gfx::Point BrowserActionsContainer::GetViewContentOffset() const {
+  return gfx::Point(0, ToolbarView::kVertSpacing);
+}
+
 void BrowserActionsContainer::MoveBrowserAction(const std::string& extension_id,
                                                 size_t new_index) {
   ExtensionService* service = profile_->GetExtensionService();
@@ -594,7 +596,7 @@ void BrowserActionsContainer::BrowserActionAdded(const Extension* extension,
   // Add the new browser action to the vector and the view hierarchy.
   if (profile_->IsOffTheRecord())
     index = model_->OriginalIndexToIncognito(index);
-  BrowserActionView* view = new BrowserActionView(extension, this);
+  BrowserActionView* view = new BrowserActionView(extension, browser_, this);
   browser_action_views_.insert(browser_action_views_.begin() + index, view);
   AddChildViewAt(view, index);
 
@@ -680,8 +682,9 @@ void BrowserActionsContainer::LoadImages() {
 void BrowserActionsContainer::SetContainerWidth() {
   int visible_actions = model_->GetVisibleIconCount();
   if (visible_actions < 0)  // All icons should be visible.
-    visible_actions = model_->size();
-  chevron_->SetVisible(static_cast<size_t>(visible_actions) < model_->size());
+    visible_actions = model_->toolbar_items().size();
+  chevron_->SetVisible(
+    static_cast<size_t>(visible_actions) < model_->toolbar_items().size());
   container_width_ = IconCountToWidth(visible_actions, chevron_->visible());
 }
 
@@ -706,7 +709,7 @@ void BrowserActionsContainer::ShowDropFolder() {
   DCHECK(!overflow_menu_);
   SetDropIndicator(-1);
   overflow_menu_ = new BrowserActionOverflowMenuController(
-      this, chevron_, browser_action_views_, VisibleBrowserActions());
+      this, browser_, chevron_, browser_action_views_, VisibleBrowserActions());
   overflow_menu_->set_observer(this);
   overflow_menu_->RunMenu(GetWidget(), true);
 }

chrome/browser/ui/views/browser_actions_container.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_VIEWS_BROWSER_ACTIONS_CONTAINER_H_
 
 #include "chrome/browser/extensions/extension_toolbar_model.h"
+#include "chrome/browser/ui/views/browser_action_view.h"
 #include "chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.h"
 #include "chrome/browser/ui/views/extensions/extension_keybinding_registry_views.h"
 #include "content/public/browser/notification_observer.h"
@@ -104,12 +105,12 @@ class ResizeArea;
 class BrowserActionsContainer
     : public views::View,
       public views::MenuButtonListener,
-      public views::DragController,
       public views::ResizeAreaDelegate,
       public ui::AnimationDelegate,
       public ExtensionToolbarModel::Observer,
       public BrowserActionOverflowMenuController::Observer,
-      public views::WidgetObserver {
+      public views::WidgetObserver,
+      public BrowserActionView::Delegate {
  public:
   BrowserActionsContainer(Browser* browser, views::View* owner_view);
   virtual ~BrowserActionsContainer();
@@ -129,12 +130,6 @@ class BrowserActionsContainer
   // Returns the profile this container is associated with.
   Profile* profile() const { return profile_; }
 
-  // Returns the browser this container is associated with.
-  Browser* browser() const { return browser_; }
-
-  // Returns the current tab's ID, or -1 if there is no current tab.
-  int GetCurrentTabId() const;
-
   // Get a particular browser action view.
   BrowserActionView* GetBrowserActionViewAt(int index) {
     return browser_action_views_[index];
@@ -152,15 +147,9 @@ class BrowserActionsContainer
   // Delete all browser action views.
   void DeleteBrowserActionViews();
 
-  // Called when a browser action becomes visible/hidden.
-  void OnBrowserActionVisibilityChanged();
-
   // Returns how many browser actions are visible.
   size_t VisibleBrowserActions() const;
 
-  // Called when the user clicks on the browser action icon.
-  void OnBrowserActionExecuted(BrowserActionButton* button);
-
   // Overridden from views::View:
   virtual gfx::Size GetPreferredSize() OVERRIDE;
   virtual void Layout() OVERRIDE;
@@ -202,6 +191,12 @@ class BrowserActionsContainer
   // Overridden from views::WidgetObserver:
   virtual void OnWidgetClosing(views::Widget* widget) OVERRIDE;
 
+  // Overridden from BrowserActionView::Delegate:
+  virtual int GetCurrentTabId() const OVERRIDE;
+  virtual void OnBrowserActionExecuted(BrowserActionButton* button) OVERRIDE;
+  virtual void OnBrowserActionVisibilityChanged() OVERRIDE;
+  virtual gfx::Point GetViewContentOffset() const OVERRIDE;
+
   // Moves a browser action with |id| to |new_index|.
   void MoveBrowserAction(const std::string& extension_id, size_t new_index);
 

chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc

@@ -23,10 +23,12 @@
 
 BrowserActionOverflowMenuController::BrowserActionOverflowMenuController(
     BrowserActionsContainer* owner,
+    Browser* browser,
     views::MenuButton* menu_button,
     const std::vector<BrowserActionView*>& views,
     int start_index)
     : owner_(owner),
+      browser_(browser),
       observer_(NULL),
       menu_button_(menu_button),
       menu_(NULL),
@@ -108,7 +110,7 @@ bool BrowserActionOverflowMenuController::ShowContextMenu(
     return false;
 
   scoped_refptr<ExtensionContextMenuModel> context_menu_contents =
-      new ExtensionContextMenuModel(extension, owner_->browser());
+      new ExtensionContextMenuModel(extension, browser_);
   views::MenuModelAdapter context_menu_model_adapter(
       context_menu_contents.get());
   views::MenuRunner context_menu_runner(

chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.h

@@ -13,6 +13,7 @@
 #include "base/sequenced_task_runner_helpers.h"
 #include "ui/views/controls/menu/menu_delegate.h"
 
+class Browser;
 class BrowserActionsContainer;
 class BrowserActionView;
 
@@ -34,6 +35,7 @@ class BrowserActionOverflowMenuController : public views::MenuDelegate {
 
   BrowserActionOverflowMenuController(
       BrowserActionsContainer* owner,
+      Browser* browser,
       views::MenuButton* menu_button,
       const std::vector<BrowserActionView*>& views,
       int start_index);
@@ -88,6 +90,8 @@ class BrowserActionOverflowMenuController : public views::MenuDelegate {
   // A pointer to the browser action container that owns the overflow menu.
   BrowserActionsContainer* owner_;
 
+  Browser* browser_;
+
   // The observer, may be null.
   Observer* observer_;
 

chrome/browser/ui/views/location_bar/action_box_button_view.cc

@@ -8,7 +8,10 @@
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/ui/toolbar/action_box_menu_model.h"
 #include "chrome/browser/ui/view_ids.h"
+#include "chrome/browser/ui/views/action_box_menu.h"
 #include "chrome/browser/ui/views/browser_dialogs.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
@@ -30,9 +33,11 @@ const SkColor kPushedBorderColor = SkColorSetRGB(191, 191, 191);
 }  // namespace
 
 
-ActionBoxButtonView::ActionBoxButtonView(ExtensionService* extension_service)
+ActionBoxButtonView::ActionBoxButtonView(Browser* browser, Profile* profile)
     : views::MenuButton(NULL, string16(), this, false),
-      extension_service_(extension_service) {
+      browser_(browser),
+      profile_(profile),
+      starred_(false) {
   set_id(VIEW_ID_ACTION_BOX_BUTTON);
   SetTooltipText(l10n_util::GetStringUTF16(IDS_TOOLTIP_ACTION_BOX_BUTTON));
   SetIcon(*ui::ResourceBundle::GetSharedInstance().GetBitmapNamed(
@@ -73,5 +78,11 @@ void ActionBoxButtonView::GetAccessibleState(ui::AccessibleViewState* state) {
 
 void ActionBoxButtonView::OnMenuButtonClicked(View* source,
                                               const gfx::Point& point) {
-  // TODO(yefim): Implement menu here.
+  ExtensionService* extension_service =
+      extensions::ExtensionSystem::Get(profile_)->extension_service();
+
+  ActionBoxMenuModel model(extension_service);
+  ActionBoxMenu action_box_menu(browser_, &model, starred_);
+  action_box_menu.Init();
+  action_box_menu.RunMenu(this);
 }

chrome/browser/ui/views/location_bar/action_box_button_view.h

@@ -8,18 +8,21 @@
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/button/menu_button_listener.h"
 
-class ExtensionService;
+class Browser;
+class Profile;
 
 // ActionBoxButtonView displays a plus button with associated menu.
 class ActionBoxButtonView : public views::MenuButton,
                             public views::MenuButtonListener {
  public:
-  explicit ActionBoxButtonView(ExtensionService* extension_service);
+  explicit ActionBoxButtonView(Browser* browser, Profile* profile);
   virtual ~ActionBoxButtonView();
 
   SkColor GetBackgroundColor();
   SkColor GetBorderColor();
 
+  void set_starred(bool starred) { starred_ = starred; }
+
  private:
   // CustomButton
   virtual void GetAccessibleState(ui::AccessibleViewState* state) OVERRIDE;
@@ -28,7 +31,12 @@ class ActionBoxButtonView : public views::MenuButton,
   virtual void OnMenuButtonClicked(View* source,
                                    const gfx::Point& point) OVERRIDE;
 
-  ExtensionService* extension_service_;
+  Browser* browser_;
+  Profile* profile_;
+
+  // Set to true when the current page is bookmarked.
+  // To be passed to action box menu when this button is pressed.
+  bool starred_;
 
   DISALLOW_COPY_AND_ASSIGN(ActionBoxButtonView);
 };

chrome/browser/ui/views/location_bar/location_bar_view.cc

@@ -157,13 +157,15 @@ void LocationBarView::FadeAnimationObserver::OnImplicitAnimationsCompleted() {
 
 // LocationBarView -----------------------------------------------------------
 
-LocationBarView::LocationBarView(Profile* profile,
+LocationBarView::LocationBarView(Browser* browser,
+                                 Profile* profile,
                                  CommandUpdater* command_updater,
                                  ToolbarModel* model,
                                  Delegate* delegate,
                                  chrome::search::SearchModel* search_model,
                                  Mode mode)
-    : profile_(profile),
+    : browser_(browser),
+      profile_(profile),
       command_updater_(command_updater),
       model_(model),
       delegate_(delegate),
@@ -272,9 +274,8 @@ void LocationBarView::Init(views::View* popup_parent_view) {
   zoom_view_ = new ZoomView(model_, delegate_);
   AddChildView(zoom_view_);
 
-  if (extensions::switch_utils::IsActionBoxEnabled()) {
-    action_box_button_view_ = new ActionBoxButtonView(
-        extensions::ExtensionSystem::Get(profile_)->extension_service());
+  if (extensions::switch_utils::IsActionBoxEnabled() && browser_) {
+    action_box_button_view_ = new ActionBoxButtonView(browser_, profile_);
     AddChildView(action_box_button_view_);
   } else if (browser_defaults::bookmarks_enabled && (mode_ == NORMAL)) {
     // Note: condition above means that the star and ChromeToMobile icons are
@@ -495,6 +496,8 @@ views::View* LocationBarView::GetPageActionView(ExtensionAction *page_action) {
 void LocationBarView::SetStarToggled(bool on) {
   if (star_view_)
     star_view_->SetToggled(on);
+  if (action_box_button_view_)
+    action_box_button_view_->set_starred(on);
 }
 
 void LocationBarView::ShowStarBubble(const GURL& url, bool newly_bookmarked) {

chrome/browser/ui/views/location_bar/location_bar_view.h

@@ -150,7 +150,8 @@ class LocationBarView : public LocationBar,
     APP_LAUNCHER
   };
 
-  LocationBarView(Profile* profile,
+  LocationBarView(Browser* browser,
+                  Profile* profile,
                   CommandUpdater* command_updater,
                   ToolbarModel* model,
                   Delegate* delegate,
@@ -445,6 +446,11 @@ class LocationBarView : public LocationBar,
   void CleanupFadeAnimation();
 #endif
 
+  // The Browser this LocationBarView is in.  Note that at least
+  // chromeos::SimpleWebViewDialog uses a LocationBarView outside any browser
+  // window, so this may be NULL.
+  Browser* browser_;
+
   // The Autocomplete Edit field.
   scoped_ptr<OmniboxView> location_entry_;
 

chrome/browser/ui/views/toolbar_view.cc

@@ -245,6 +245,7 @@ void ToolbarView::Init(views::View* location_bar_parent,
       location_bar_parent,
       chrome::search::IsInstantExtendedAPIEnabled(browser_->profile()));
   location_bar_ = new LocationBarView(
+      browser_,
       browser_->profile(),
       browser_->command_controller()->command_updater(),
       model_,

chrome/chrome_browser.gypi

@@ -3519,6 +3519,8 @@
         'browser/ui/tabs/tab_strip_model_order_controller.h',
         'browser/ui/tabs/tab_strip_selection_model.cc',
         'browser/ui/tabs/tab_strip_selection_model.h',
+        'browser/ui/toolbar/action_box_menu_model.cc',
+        'browser/ui/toolbar/action_box_menu_model.h',
         'browser/ui/toolbar/back_forward_menu_model.cc',
         'browser/ui/toolbar/back_forward_menu_model.h',
         'browser/ui/toolbar/bookmark_sub_menu_model.cc',
@@ -3545,6 +3547,8 @@
         'browser/ui/views/accessibility/accessibility_event_router_views.h',
         'browser/ui/views/accessibility/invert_bubble_view.cc',
         'browser/ui/views/accessibility/invert_bubble_view.h',
+        'browser/ui/views/action_box_menu.cc',
+        'browser/ui/views/action_box_menu.h',
         'browser/ui/views/app_menu_button_win.cc',
         'browser/ui/views/app_menu_button_win.h',
         'browser/ui/views/ash/balloon_collection_impl_ash.cc',