Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
[Media Router] Adds a contextual menu item to remove the MR component…
Browse files Browse the repository at this point in the history
… 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 95cfa83)

Review URL: https://codereview.chromium.org/1679993002 .

Cr-Commit-Position: refs/branch-heads/2623@{#311}
Cr-Branched-From: 92d7753-refs/heads/master@{#369907}
  • Loading branch information
markafoltz committed Feb 9, 2016
1 parent d9e3ee1 commit d533aee
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 25 deletions.
1 change: 1 addition & 0 deletions chrome/app/chrome_command_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions chrome/browser/extensions/component_migration_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/extensions/component_migration_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 18 additions & 0 deletions chrome/browser/extensions/component_migration_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
38 changes: 14 additions & 24 deletions chrome/browser/ui/toolbar/media_router_contextual_menu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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);
Expand All @@ -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[] =
Expand All @@ -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;
Expand All @@ -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);
}
2 changes: 1 addition & 1 deletion chrome/browser/ui/toolbar/media_router_contextual_menu.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ui/toolbar/toolbar_actions_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit d533aee

Please sign in to comment.