Action box menu

chrome/browser/extensions/extension_toolbar_model.h

Index: chrome/browser/extensions/extension_toolbar_model.h
diff --git a/chrome/browser/extensions/extension_toolbar_model.h b/chrome/browser/extensions/extension_toolbar_model.h
index 2c4f27d6224e47e2e1f8badec951284deb527466..983600f9773ce590547a8a32283196c8c4778994 100644
--- a/chrome/browser/extensions/extension_toolbar_model.h
+++ b/chrome/browser/extensions/extension_toolbar_model.h
@@ -18,6 +18,8 @@ class PrefService;
 // Model for the browser actions toolbar.
 class ExtensionToolbarModel : public content::NotificationObserver {
  public:
+  typedef std::vector<std::string> VectorOfStrings;

[Peter Kasting, 2012/07/21 01:55:13]

Nit: As noted in extension_prefs.h, if you change the functions there to
consistently use the pre-existing ExtensionIdSet type, you can eliminate this
typedef and use that type instead in this header/.cc file.  (This will require
#including extension_prefs.h here, which seems OK to me.)

[yefimt, 2012/07/23 23:47:52]

Done.

+
   explicit ExtensionToolbarModel(ExtensionService* service);
   virtual ~ExtensionToolbarModel();
 
@@ -73,24 +75,34 @@ class ExtensionToolbarModel : public content::NotificationObserver {
   bool extensions_initialized() const { return extensions_initialized_; }
 
   size_t size() const {
-    return toolitems_.size();
+    return toolbar_items_.size();
   }
 
   extensions::ExtensionList::iterator begin() {
-    return toolitems_.begin();
+    return toolbar_items_.begin();
   }
 
   extensions::ExtensionList::iterator end() {
-    return toolitems_.end();
+    return toolbar_items_.end();
   }
 
-  const extensions::Extension* GetExtensionByIndex(int index) const;
+  const extensions::Extension* GetExtensionByIndex(int index) const {

[Peter Kasting, 2012/07/21 01:55:13]

Nit: Inlined functions should be named unix_hacker()-style.

Normally we'd also name them without "get" and as close to the actual variable
names in question as possible, a la:

toolbar_item_for_index()
action_box_menu_item_for_index()

(2 places)

[yefimt, 2012/07/23 23:47:52]

Yes, but they not returning a member variable but and expression with it, and
renaming this function will cause a chain reaction:)
I'll need to touch 2 other files, then you will ask me to fix them and because
they are mac and linux specific, I cannot even verify they compile.

[Peter Kasting, 2012/07/24 04:27:29]

That's what we have trybots for.  Please do at least make the name use lowercase
and underscores, the actual names are up to your best judgment.

+    return toolbar_items_[index];
+  }
 
   // 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);
 
+  size_t action_box_extensions_size() {
+    return action_box_menu_items_.size();
+  }
+
+  const extensions::Extension* GetActionBoxExtensionByIndex(int index) const {
+    return action_box_menu_items_[index];
+  }
+
  private:
   // content::NotificationObserver implementation.
   virtual void Observe(int type,
@@ -99,8 +111,21 @@ 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();
+
+  // A couple helper functions to populate lists.
+  void AddToProperList(const extensions::Extension* extension,
+                       const VectorOfStrings& order,
+                       extensions::ExtensionList* sorted,
+                       extensions::ExtensionList* unsorted);
+  void MergeLists(const extensions::ExtensionList& sorted,
+                  const extensions::ExtensionList& unsorted,
+                  extensions::ExtensionList* result_list);
+  void FillExtensionList(const VectorOfStrings& order,
+                         extensions::ExtensionList* result_list);
 
   // Save the model to prefs.
   void UpdatePrefs();
@@ -120,7 +145,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_;