From d533aee7439c6cf2e01de5b540ad10fb9e79ab60 Mon Sep 17 00:00:00 2001 From: "mark a. foltz" Date: Mon, 8 Feb 2016 16:14:55 -0800 Subject: [PATCH] [Media Router] Adds a contextual menu item to remove the MR component action. This adds a menu item to remove the MR component action from the toolbar. It sets a pref so this preference is respected across sessions. The user can restore the action by reinstalling the Cast extension from the Web Store. (They may have to reload the Web Store page immediately after installation to do so.) BUG=576362 TESTED=Unit test, manually tested Review URL: https://codereview.chromium.org/1612203002 Cr-Commit-Position: refs/heads/master@{#371629} (cherry picked from commit 95cfa837a726c4400e025e826fcaddf1bfc751eb) Review URL: https://codereview.chromium.org/1679993002 . Cr-Commit-Position: refs/branch-heads/2623@{#311} Cr-Branched-From: 92d77538a86529ca35f9220bd3cd512cbea1f086-refs/heads/master@{#369907} --- chrome/app/chrome_command_ids.h | 1 + .../extensions/component_migration_helper.cc | 10 +++++ .../extensions/component_migration_helper.h | 3 ++ .../component_migration_helper_unittest.cc | 18 +++++++++ .../toolbar/media_router_contextual_menu.cc | 38 +++++++------------ .../ui/toolbar/media_router_contextual_menu.h | 2 +- .../ui/toolbar/toolbar_actions_model.h | 4 ++ 7 files changed, 51 insertions(+), 25 deletions(-) diff --git a/chrome/app/chrome_command_ids.h b/chrome/app/chrome_command_ids.h index 746d9e138c2b0..bfcb7e89127be 100644 --- a/chrome/app/chrome_command_ids.h +++ b/chrome/app/chrome_command_ids.h @@ -348,6 +348,7 @@ #define IDC_MEDIA_ROUTER_HELP 51201 #define IDC_MEDIA_ROUTER_LEARN_MORE 51202 #define IDC_MEDIA_ROUTER_REPORT_ISSUE 51203 +#define IDC_MEDIA_ROUTER_REMOVE_TOOLBAR_ACTION 51204 // Context menu items for media stream status tray #define IDC_MEDIA_STREAM_DEVICE_STATUS_TRAY 51300 diff --git a/chrome/browser/extensions/component_migration_helper.cc b/chrome/browser/extensions/component_migration_helper.cc index e2baca13ab7ad..389959a3f5b2b 100644 --- a/chrome/browser/extensions/component_migration_helper.cc +++ b/chrome/browser/extensions/component_migration_helper.cc @@ -113,6 +113,16 @@ void ComponentMigrationHelper::OnFeatureDisabled( delegate_->RemoveComponentAction(component_action_id); } +void ComponentMigrationHelper::OnActionRemoved( + const std::string& component_action_id) { + // Record preference for the future. + SetComponentActionPref(component_action_id, false); + + // Remove the action. + if (delegate_->HasComponentAction(component_action_id)) + delegate_->RemoveComponentAction(component_action_id); +} + void ComponentMigrationHelper::OnExtensionReady( content::BrowserContext* browser_context, const Extension* extension) { diff --git a/chrome/browser/extensions/component_migration_helper.h b/chrome/browser/extensions/component_migration_helper.h index fd9a8e3232dae..4f4ce9eb9cd61 100644 --- a/chrome/browser/extensions/component_migration_helper.h +++ b/chrome/browser/extensions/component_migration_helper.h @@ -100,6 +100,9 @@ class ComponentMigrationHelper : public ExtensionRegistryObserver { // extension loading. void OnFeatureDisabled(const std::string& component_action_id); + // Call when the user manually removes the component action from the toolbar. + void OnActionRemoved(const std::string& component_action_id); + // extensions::ExtensionRegistryObserver: void OnExtensionReady(content::BrowserContext* browser_context, const Extension* extension) override; diff --git a/chrome/browser/extensions/component_migration_helper_unittest.cc b/chrome/browser/extensions/component_migration_helper_unittest.cc index 02489928eddbc..4b0ebb85a52a5 100644 --- a/chrome/browser/extensions/component_migration_helper_unittest.cc +++ b/chrome/browser/extensions/component_migration_helper_unittest.cc @@ -191,4 +191,22 @@ TEST_F(ComponentMigrationHelperTest, InstallUnregisteredExtension) { registry()->enabled_extensions().Contains(unregistered_extension_->id())); } +TEST_F(ComponentMigrationHelperTest, RemoveComponentAction) { + mock_helper_->SetTestComponentActionPref(true); + + EXPECT_CALL(mock_delegate_, HasComponentAction(kTestActionId)) + .WillOnce(Return(false)); + EXPECT_CALL(mock_delegate_, AddComponentAction(kTestActionId)); + + mock_helper_->OnFeatureEnabled(kTestActionId); + EXPECT_TRUE(IsTestComponentActionEnabled()); + + EXPECT_CALL(mock_delegate_, HasComponentAction(kTestActionId)) + .WillOnce(Return(true)); + EXPECT_CALL(mock_delegate_, RemoveComponentAction(kTestActionId)); + + mock_helper_->OnActionRemoved(kTestActionId); + EXPECT_FALSE(IsTestComponentActionEnabled()); +} + } // namespace extensions diff --git a/chrome/browser/ui/toolbar/media_router_contextual_menu.cc b/chrome/browser/ui/toolbar/media_router_contextual_menu.cc index 4d649f04bdd2e..a67ddf3842796 100644 --- a/chrome/browser/ui/toolbar/media_router_contextual_menu.cc +++ b/chrome/browser/ui/toolbar/media_router_contextual_menu.cc @@ -5,12 +5,16 @@ #include "base/logging.h" #include "base/metrics/user_metrics.h" #include "chrome/app/chrome_command_ids.h" +#include "chrome/browser/extensions/component_migration_helper.h" #include "chrome/browser/media/router/media_router_factory.h" #include "chrome/browser/media/router/media_router_mojo_impl.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/singleton_tabs.h" +#include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h" #include "chrome/browser/ui/toolbar/media_router_contextual_menu.h" +#include "chrome/browser/ui/toolbar/toolbar_actions_model.h" +#include "chrome/grit/chromium_strings.h" #include "chrome/grit/generated_resources.h" #include "extensions/common/constants.h" #include "ui/base/l10n/l10n_util.h" @@ -26,6 +30,8 @@ MediaRouterContextualMenu::MediaRouterContextualMenu(Browser* browser) IDS_MEDIA_ROUTER_LEARN_MORE); menu_model_.AddItemWithStringId(IDC_MEDIA_ROUTER_HELP, IDS_MEDIA_ROUTER_HELP); + menu_model_.AddItemWithStringId(IDC_MEDIA_ROUTER_REMOVE_TOOLBAR_ACTION, + IDS_EXTENSIONS_UNINSTALL); menu_model_.AddSeparator(ui::NORMAL_SEPARATOR); menu_model_.AddItemWithStringId(IDC_MEDIA_ROUTER_REPORT_ISSUE, IDS_MEDIA_ROUTER_REPORT_ISSUE); @@ -48,30 +54,6 @@ bool MediaRouterContextualMenu::GetAcceleratorForCommandId( return false; } -base::string16 MediaRouterContextualMenu::GetLabelForCommandId( - int command_id) const { - int string_id; - switch (command_id) { - case IDC_MEDIA_ROUTER_ABOUT: - string_id = IDS_MEDIA_ROUTER_ABOUT; - break; - case IDC_MEDIA_ROUTER_HELP: - string_id = IDS_MEDIA_ROUTER_HELP; - break; - case IDC_MEDIA_ROUTER_LEARN_MORE: - string_id = IDS_MEDIA_ROUTER_LEARN_MORE; - break; - case IDC_MEDIA_ROUTER_REPORT_ISSUE: - string_id = IDS_MEDIA_ROUTER_REPORT_ISSUE; - break; - default: - NOTREACHED(); - return base::string16(); - } - - return l10n_util::GetStringUTF16(string_id); -} - void MediaRouterContextualMenu::ExecuteCommand(int command_id, int event_flags) { const char kAboutPageUrl[] = @@ -93,6 +75,9 @@ void MediaRouterContextualMenu::ExecuteCommand(int command_id, case IDC_MEDIA_ROUTER_LEARN_MORE: chrome::ShowSingletonTab(browser_, GURL(kCastLearnMorePageUrl)); break; + case IDC_MEDIA_ROUTER_REMOVE_TOOLBAR_ACTION: + RemoveMediaRouterComponentAction(); + break; case IDC_MEDIA_ROUTER_REPORT_ISSUE: ReportIssue(); break; @@ -116,3 +101,8 @@ void MediaRouterContextualMenu::ReportIssue() { "/feedback.html"); chrome::ShowSingletonTab(browser_, GURL(feedback_url)); } + +void MediaRouterContextualMenu::RemoveMediaRouterComponentAction() { + ToolbarActionsModel::Get(browser_->profile())->component_migration_helper() + ->OnActionRemoved(ComponentToolbarActionsFactory::kMediaRouterActionId); +} diff --git a/chrome/browser/ui/toolbar/media_router_contextual_menu.h b/chrome/browser/ui/toolbar/media_router_contextual_menu.h index 87ec9181bd3e4..510d7f6e764d3 100644 --- a/chrome/browser/ui/toolbar/media_router_contextual_menu.h +++ b/chrome/browser/ui/toolbar/media_router_contextual_menu.h @@ -25,10 +25,10 @@ class MediaRouterContextualMenu : public ui::SimpleMenuModel::Delegate { bool IsCommandIdEnabled(int command_id) const override; bool GetAcceleratorForCommandId(int command_id, ui::Accelerator* accelerator) override; - base::string16 GetLabelForCommandId(int command_id) const override; void ExecuteCommand(int command_id, int event_flags) override; void ReportIssue(); + void RemoveMediaRouterComponentAction(); Browser* browser_; ui::SimpleMenuModel menu_model_; diff --git a/chrome/browser/ui/toolbar/toolbar_actions_model.h b/chrome/browser/ui/toolbar/toolbar_actions_model.h index c2ba29896b431..01637e80c5ee5 100644 --- a/chrome/browser/ui/toolbar/toolbar_actions_model.h +++ b/chrome/browser/ui/toolbar/toolbar_actions_model.h @@ -165,6 +165,10 @@ class ToolbarActionsModel return is_highlighting() ? highlighted_items_ : toolbar_items_; } + extensions::ComponentMigrationHelper* component_migration_helper() { + return component_migration_helper_.get(); + } + bool is_highlighting() const { return highlight_type_ != HIGHLIGHT_NONE; } HighlightType highlight_type() const { return highlight_type_; } bool highlighting_for_toolbar_redesign() const {