Only show chrome2mobile in action box if it is enabled

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/chrome_to_mobile_service.h"
+#include "chrome/browser/chrome_to_mobile_service_factory.h"
+#include "chrome/browser/command_updater.h"
 #include "chrome/browser/extensions/extension_toolbar_model.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/bookmarks/bookmark_tab_helper.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/base/resource/resource_bundle.h"
 
 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
 //
 const int kFirstExtensionCommandId = 0xE000;
 
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // ActionBoxMenuModel
 
 ActionBoxMenuModel::ActionBoxMenuModel(Browser* browser,
-                                       ExtensionService* extension_service)
+                                       ExtensionService* extension_service,
+                                       CommandUpdater* command_updater)
     : ALLOW_THIS_IN_INITIALIZER_LIST(ui::SimpleMenuModel(this)),
       browser_(browser),
-      extension_service_(extension_service) {
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  InsertItemWithStringIdAt(0, IDC_CHROME_TO_MOBILE_PAGE,
-                           IDS_CHROME_TO_MOBILE_BUBBLE_TOOLTIP);
-  SetIcon(0, rb.GetNativeImageNamed(IDR_MOBILE));
-
-  TabContents* current_tab_contents = chrome::GetActiveTabContents(browser);
-  bool starred = current_tab_contents->bookmark_tab_helper()->is_starred();
-  InsertItemWithStringIdAt(1, IDC_BOOKMARK_PAGE,
-                           starred ? IDS_TOOLTIP_STARRED : IDS_TOOLTIP_STAR);
-  SetIcon(1, rb.GetNativeImageNamed(starred ? IDR_STAR_LIT : 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.empty()) {
-    AddSeparator(ui::NORMAL_SEPARATOR);
-    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();
-    }
+      extension_service_(extension_service),
+      command_updater_(command_updater) {
+  // Disable Chrome To Mobile for off-the-record and non-synced profiles,
+  // or if the feature is disabled by a command line flag or chrome://flags.
+  if (!browser_->profile()->IsOffTheRecord() &&
+      browser_->profile()->IsSyncAccessible() &&

[msw, 2012/08/29 19:27:04]

nit: I think we should drop the IsSyncAccessible condition.

[Cait (Slow), 2012/08/29 20:26:14]

Done.

+     ChromeToMobileService::IsChromeToMobileEnabled()) {
+    command_updater_->AddCommandObserver(IDC_CHROME_TO_MOBILE_PAGE, this);
   }
+  UpdateMenu();
 }
 
 ActionBoxMenuModel::~ActionBoxMenuModel() {
   // Ensures parent destructor does not use a partially destroyed delegate.
   set_delegate(NULL);
+  // Remove command observer.
+  command_updater_->RemoveCommandObserver(this);

[msw, 2012/08/29 19:27:04]

Will this work if the ctor skips adding the observer?

[Cait (Slow), 2012/08/29 20:26:14]

Yeah, I think it is just a no-op in that case.

 }
 
 bool ActionBoxMenuModel::IsItemExtension(int index) {
   return GetCommandIdAt(index) >= kFirstExtensionCommandId;
 }
 
 const extensions::Extension* ActionBoxMenuModel::GetExtensionAt(int index) {
   if (!IsItemExtension(index))
     return NULL;
 
@@ skipping 23 matching lines @@
 
 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) {
 }
+
+void ActionBoxMenuModel::EnabledStateChangedForCommand(int id, bool enabled) {
+  DCHECK_EQ(id, IDC_CHROME_TO_MOBILE_PAGE);
+  UpdateMenu();
+  command_updater_->UpdateCommandEnabled(IDC_CHROME_TO_MOBILE_PAGE, enabled);

[msw, 2012/08/29 19:27:04]

This seems unnecessary.

[Cait (Slow), 2012/08/29 20:26:14]

Done.

+}
+
+void ActionBoxMenuModel::UpdateMenu() {
+  int next_item_index = 0;
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+
+  bool c2m_enabled = ChromeToMobileService::IsChromeToMobileEnabled() &&

[msw, 2012/08/29 19:27:04]

nit: nix |c2m_enabled|, inline conditionals into the if().
Ideally this is the command enabled state, but this seems ok.

[Cait (Slow), 2012/08/29 20:26:14]

Done.

+      ChromeToMobileServiceFactory::GetForProfile(browser_->profile())->
+          HasMobiles();
+  if (c2m_enabled) {
+    InsertItemWithStringIdAt(next_item_index, IDC_CHROME_TO_MOBILE_PAGE,

[msw, 2012/08/29 19:27:04]

nit: perhaps use AddItem instead and nix next_item_index?

[Cait (Slow), 2012/08/29 20:26:14]

Done.

+                           IDS_CHROME_TO_MOBILE_BUBBLE_TOOLTIP);

[msw, 2012/08/29 19:27:04]

nit: indent

[Cait (Slow), 2012/08/29 20:26:14]

Done.

+    SetIcon(next_item_index, rb.GetNativeImageNamed(IDR_MOBILE));
+    next_item_index++;
+  }
+  TabContents* current_tab_contents = chrome::GetActiveTabContents(browser_);
+  bool starred = current_tab_contents->bookmark_tab_helper()->is_starred();
+  InsertItemWithStringIdAt(next_item_index, IDC_BOOKMARK_PAGE,
+                           starred ? IDS_TOOLTIP_STARRED : IDS_TOOLTIP_STAR);
+  SetIcon(next_item_index,
+          rb.GetNativeImageNamed(starred ? IDR_STAR_LIT : 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.empty()) {
+    AddSeparator(ui::NORMAL_SEPARATOR);
+    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();
+    }
+  }
+}