Action box menu

chrome/browser/extensions/extension_toolbar_model.cc

Index: chrome/browser/extensions/extension_toolbar_model.cc
diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/extensions/extension_toolbar_model.cc
index f8289acce74241d6756862e1d60617d87e109b1b..c4eced6fc60cc01318c343b8d3fcd9156661cb3c 100644
--- a/chrome/browser/extensions/extension_toolbar_model.cc
+++ b/chrome/browser/extensions/extension_toolbar_model.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_switch_utils.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
@@ -60,23 +61,23 @@ void ExtensionToolbarModel::MoveBrowserAction(const Extension* extension,
     NOTREACHED();
     return;
   }
-  toolitems_.erase(pos);
+  toolbar_items_.erase(pos);
 
   int i = 0;
   bool inserted = false;
   for (ExtensionList::iterator iter = begin(); iter != end(); ++iter, ++i) {
     if (i == index) {
-      toolitems_.insert(iter, make_scoped_refptr(extension));
+      toolbar_items_.insert(iter, make_scoped_refptr(extension));
       inserted = true;
       break;
     }
   }
 
   if (!inserted) {
-    DCHECK_EQ(index, static_cast<int>(toolitems_.size()));
-    index = toolitems_.size();
+    DCHECK_EQ(index, static_cast<int>(toolbar_items_.size()));
+    index = toolbar_items_.size();
 
-    toolitems_.push_back(make_scoped_refptr(extension));
+    toolbar_items_.push_back(make_scoped_refptr(extension));
   }
 
   FOR_EACH_OBSERVER(Observer, observers_, BrowserActionMoved(extension, index));
@@ -121,7 +122,7 @@ void ExtensionToolbarModel::Observe(
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   if (type == chrome::NOTIFICATION_EXTENSIONS_READY) {
-    InitializeExtensionList();
+    InitializeExtensionLists();
     return;
   }
 
@@ -139,8 +140,8 @@ void ExtensionToolbarModel::Observe(
     // We don't want to add the same extension twice. It may have already been
     // added by EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED below, if the user
     // hides the browser action and then disables and enables the extension.
-    for (size_t i = 0; i < toolitems_.size(); i++) {
-      if (toolitems_[i].get() == extension)
+    for (size_t i = 0; i < toolbar_items_.size(); i++) {
+      if (toolbar_items_[i].get() == extension)
         return;  // Already exists.
     }
     if (service_->extension_prefs()->GetBrowserActionVisibility(extension))
@@ -165,15 +166,15 @@ void ExtensionToolbarModel::AddExtension(const Extension* extension) {
     return;
 
   if (extension->id() == last_extension_removed_ &&
-      last_extension_removed_index_ < toolitems_.size()) {
-    toolitems_.insert(begin() + last_extension_removed_index_,
+      last_extension_removed_index_ < toolbar_items_.size()) {
+    toolbar_items_.insert(begin() + last_extension_removed_index_,
                       make_scoped_refptr(extension));
     FOR_EACH_OBSERVER(Observer, observers_,
         BrowserActionAdded(extension, last_extension_removed_index_));
   } else {
-    toolitems_.push_back(make_scoped_refptr(extension));
+    toolbar_items_.push_back(make_scoped_refptr(extension));
     FOR_EACH_OBSERVER(Observer, observers_,
-                      BrowserActionAdded(extension, toolitems_.size() - 1));
+                      BrowserActionAdded(extension, toolbar_items_.size() - 1));
   }
 
   last_extension_removed_ = "";
@@ -191,7 +192,7 @@ void ExtensionToolbarModel::RemoveExtension(const Extension* extension) {
   last_extension_removed_ = extension->id();
   last_extension_removed_index_ = pos - begin();
 
-  toolitems_.erase(pos);
+  toolbar_items_.erase(pos);
   FOR_EACH_OBSERVER(Observer, observers_,
                     BrowserActionRemoved(extension));
 
@@ -205,11 +206,95 @@ void ExtensionToolbarModel::RemoveExtension(const Extension* extension) {
 // have holes.
 // 2. Create a vector of extensions that did not have a pref value.
 // 3. Remove holes from the sorted vector and append the unsorted vector.
-void ExtensionToolbarModel::InitializeExtensionList() {
+void ExtensionToolbarModel::InitializeExtensionLists() {
   DCHECK(service_->is_ready());
 
-  std::vector<std::string> pref_order = service_->extension_prefs()->
-      GetToolbarOrder();
+  if (extensions::switch_utils::IsActionBoxEnabled())
+    PopulateForActionBoxMode();
+  else
+    PopulateForNonActionBoxMode();
+
+  UpdatePrefs();
+
+  extensions_initialized_ = true;
+  FOR_EACH_OBSERVER(Observer, observers_, ModelLoaded());
+}
+
+void ExtensionToolbarModel::PopulateForActionBoxMode() {
+  const std::vector<std::string> bar_order =

[Aaron Boodman, 2012/07/02 22:41:34]

Nit: toolbar_order?

[yefimt, 2012/07/11 22:34:34]

Done.

+      service_->extension_prefs()->GetToolbarOrder();
+  const std::vector<std::string> action_box_order =
+      service_->extension_prefs()->GetActionBoxOrder();
+
+  // Items that have a pref for their position.
+  ExtensionList bar_sorted;
+  bar_sorted.resize(bar_order.size(), NULL);
+  ExtensionList action_box_sorted;
+  action_box_sorted.resize(action_box_order.size(), NULL);
+
+  // The items that don't have a pref for their position.
+  ExtensionList bar_unsorted;
+  ExtensionList action_box_unsorted;
+
+  // Create the lists.
+  for (ExtensionSet::const_iterator it = service_->extensions()->begin();
+       it != service_->extensions()->end(); ++it) {
+    const Extension* extension = *it;
+    if (!extension->browser_action())
+      continue;
+
+    bool show_in_toolbar =
+        service_->extension_prefs()->GetBrowserActionVisibility(extension);

[Aaron Boodman, 2012/07/02 22:41:34]

Do we need the pinned preference? Is it enough to just say that a browser action
is pinned if its ID shows up in the toolbar order list?

Same question for old code ... maybe we can remove that preference? Maybe it was
added before these items were re-orderable or something.

Adding finnur in case he remembers the history.

[yefimt, 2012/07/11 22:34:34]

yes, I think pinned pref could be removed.
I've changed code here and will remove pinned in a separate CL

On 2012/07/02 22:41:34, Aaron Boodman wrote:

+
+    if (show_in_toolbar)
+      AddToProperList(extension, bar_order, &bar_sorted, &bar_unsorted);
+    else
+      AddToProperList(extension, action_box_order, &action_box_sorted,
+      &action_box_unsorted);
+  }
+
+  MergeLists(action_box_sorted, action_box_unsorted, &action_box_menu_items_);
+  MergeLists(bar_sorted, bar_unsorted, &toolbar_items_);
+
+  // Inform observers.
+  for (size_t i = 0; i < toolbar_items_.size(); i++) {
+    FOR_EACH_OBSERVER(Observer, observers_,
+                      BrowserActionAdded(action_box_menu_items_[i], i));
+    FOR_EACH_OBSERVER(Observer, observers_,
+                      BrowserActionAdded(toolbar_items_[i], i));
+  }
+}
+
+void ExtensionToolbarModel::AddToProperList(const Extension* extension,
+    const std::vector<std::string>& order,
+    ExtensionList* sorted,
+    ExtensionList* unsorted) {
+  std::vector<std::string>::const_iterator pos =
+      std::find(order.begin(), order.end(), extension->id());
+  if (pos != order.end()) {
+    int index = std::distance(order.begin(), pos);
+    sorted->at(index) = extension;

[Aaron Boodman, 2012/07/02 22:41:34]

Nit: clearer to just say sorted[index] = extension.

[yefimt, 2012/07/11 22:34:34]

sorted is a pointer so it would be (*sorted)[index] = ...
I think sorted->at(index) = ... is more readable

On 2012/07/02 22:41:34, Aaron Boodman wrote:

[tfarina, 2012/07/12 01:21:43]

No, don't use std::vector<T>::at.

See Peter's recommendation at http://codereview.chromium.org/10458045/

[yefimt, 2012/07/13 19:59:20]

Done.

+  } else {
+    unsorted->push_back(make_scoped_refptr(extension));
+  }
+}
+
+void ExtensionToolbarModel::MergeLists(const ExtensionList& sorted,
+                                       const ExtensionList& unsorted,
+                                       ExtensionList* result_list) {
+  result_list->reserve(sorted.size() + unsorted.size());
+  for (ExtensionList::const_iterator iter = sorted.begin();
+       iter != sorted.end();
+       ++iter) {
+    if (*iter != NULL)
+      result_list->push_back(*iter);
+  }
+  result_list->insert(result_list->end(), unsorted.begin(), unsorted.end());
+}
+
+void ExtensionToolbarModel::PopulateForNonActionBoxMode() {
+  const std::vector<std::string> pref_order =
+      service_->extension_prefs()->GetToolbarOrder();
   // Items that have a pref for their position.
   ExtensionList sorted;
   sorted.resize(pref_order.size(), NULL);
@@ -225,35 +310,16 @@ void ExtensionToolbarModel::InitializeExtensionList() {
     if (!service_->extension_prefs()->GetBrowserActionVisibility(extension))
       continue;
 
-    std::vector<std::string>::iterator pos =
-        std::find(pref_order.begin(), pref_order.end(), extension->id());
-    if (pos != pref_order.end()) {
-      int index = std::distance(pref_order.begin(), pos);
-      sorted[index] = extension;
-    } else {
-      unsorted.push_back(make_scoped_refptr(extension));
-    }
+    AddToProperList(extension, pref_order, &sorted, &unsorted);
   }
 
-  // Merge the lists.
-  toolitems_.reserve(sorted.size() + unsorted.size());
-  for (ExtensionList::iterator iter = sorted.begin();
-       iter != sorted.end(); ++iter) {
-    if (*iter != NULL)
-      toolitems_.push_back(*iter);
-  }
-  toolitems_.insert(toolitems_.end(), unsorted.begin(), unsorted.end());
+  MergeLists(sorted, unsorted, &toolbar_items_);
 
   // Inform observers.
-  for (size_t i = 0; i < toolitems_.size(); i++) {
+  for (size_t i = 0; i < toolbar_items_.size(); i++) {
     FOR_EACH_OBSERVER(Observer, observers_,
-                      BrowserActionAdded(toolitems_[i], i));
+                      BrowserActionAdded(toolbar_items_[i], i));
   }
-
-  UpdatePrefs();
-
-  extensions_initialized_ = true;
-  FOR_EACH_OBSERVER(Observer, observers_, ModelLoaded());
 }
 
 void ExtensionToolbarModel::UpdatePrefs() {
@@ -261,14 +327,20 @@ void ExtensionToolbarModel::UpdatePrefs() {
     return;
 
   std::vector<std::string> ids;
-  ids.reserve(toolitems_.size());
+  ids.reserve(toolbar_items_.size());
   for (ExtensionList::iterator iter = begin(); iter != end(); ++iter)
     ids.push_back((*iter)->id());
   service_->extension_prefs()->SetToolbarOrder(ids);
+
+  ids.clear();
+  ids.reserve(action_box_menu_items_.size());
+  for (size_t i = 0; i < action_box_menu_items_.size(); ++i)
+    ids.push_back(action_box_menu_items_[i]->id());
+  service_->extension_prefs()->SetActionBoxOrder(ids);
 }
 
 const Extension* ExtensionToolbarModel::GetExtensionByIndex(int index) const {
-  return toolitems_[index];
+  return toolbar_items_[index];
 }
 
 int ExtensionToolbarModel::IncognitoIndexToOriginal(int incognito_index) {
@@ -295,3 +367,8 @@ int ExtensionToolbarModel::OriginalIndexToIncognito(int original_index) {
   }
   return incognito_index;
 }
+
+const Extension* ExtensionToolbarModel::GetActionBoxExtensionByIndex(
+    int index) const {
+  return action_box_menu_items_[index];
+}