Adding "send to mobile" and "add bookmark" to the action box menu and wiring to their respective ac…

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/app/chrome_command_ids.h"
 #include "chrome/browser/extensions/extension_toolbar_model.h"
+#include "grit/generated_resources.h"
+#include "grit/theme_resources.h"
+#include "ui/base/resource/resource_bundle.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;
+namespace {
+
+// Extensions get command IDs that are beyond the maximal valid extension ID
+// (0xDFFF) so that they are not confused with actual commands that appear in
+// the menu. For more details see: chrome/app/chrome_command_ids.h
+//
+static const int kFirstExtensionCommandId = 0xE000;

[Scott Hess - ex-Googler, 2012/08/13 18:54:43]

No need for static in anonymous namespace.

[beaudoin, 2012/08/17 20:04:21]

Done.

+
+}  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // ActionBoxMenuModel
 
-ActionBoxMenuModel::ActionBoxMenuModel(ExtensionService* extension_service)
-    : ALLOW_THIS_IN_INITIALIZER_LIST(ui::SimpleMenuModel(NULL)),
+ActionBoxMenuModel::ActionBoxMenuModel(Browser* browser,
+                                       ExtensionService* extension_service)
+    : ALLOW_THIS_IN_INITIALIZER_LIST(ui::SimpleMenuModel(this)),
+      browser_(browser),
       extension_service_(extension_service) {
+

[Scott Hess - ex-Googler, 2012/08/13 18:54:43]

No need for this empty line.

[beaudoin, 2012/08/17 20:04:21]

Done.

+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  InsertItemWithStringIdAt(0, IDC_CHROME_TO_MOBILE_PAGE,
+                           IDS_CHROME_TO_MOBILE_BUBBLE_TOOLTIP);
+  SetIcon(0, *rb.GetImageSkiaNamed(IDR_MOBILE));
+  InsertItemWithStringIdAt(1, IDC_BOOKMARK_PAGE, IDS_BOOKMARK_STAR);
+  SetIcon(1, *rb.GetImageSkiaNamed(IDR_STAR));
+
   // Adds extensions to the model.
   int command_id = kFirstExtensionCommandId;
   const extensions::ExtensionList& action_box_items = action_box_menu_items();
+  if (action_box_items.size() > 0)

[Scott Hess - ex-Googler, 2012/08/13 18:54:43]

Might as well put the for() loop in here at this point.  Also,
!action_box_items.empty() is a more straight-forward phrasing, though it's your
pref as to which you use.

[beaudoin, 2012/08/17 20:04:21]

Done.

+    InsertSeparatorAt(2);

[Scott Hess - ex-Googler, 2012/08/13 18:54:43]

Could this be coded as "Insert separator at the end"?  The earlier index
constants are presumably temporary while you're fleshing things out, but this
one will probably need adjusting.

[beaudoin, 2012/08/17 20:04:21]

Done.

   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() {
 }

[Scott Hess - ex-Googler, 2012/08/13 18:54:43]

Does SimpleMenuModel() support clearing the delegate?  If ~SimpleMenuModel()
makes any delegate calls, the results will go to the abstract base class (ie,
crash).

[beaudoin, 2012/08/17 20:04:21]

Done.

 
+bool ActionBoxMenuModel::IsCommandIdChecked(int command_id) const {
+  return false;
+}
+
+bool ActionBoxMenuModel::IsCommandIdEnabled(int command_id) const {
+  return true;
+}
+
+bool ActionBoxMenuModel::GetAcceleratorForCommandId(
+    int command_id,
+    ui::Accelerator* accelerator) {
+  return false;
+}
+
+void ActionBoxMenuModel::ExecuteCommand(int command_id) {
+  if (command_id < kFirstExtensionCommandId)
+    chrome::ExecuteCommand(browser_, command_id);
+}
+
 void ActionBoxMenuModel::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
 }