Commit 15324d56 authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions] ShowExtensionActionPopup -> ShowExtensionActionPopupForAPICall

Rename ShowToolbarActionPopup() and ShowExtensionActionPopup() to
ShowToolbarActionPopupForAPICall() and
ShowExtensionActionPopupForAPICall(). These methods were only used when
displaying a popup in response to an API call (outside of tests), and
this name makes it more obvious.

Additionally, this lets us get rid of the additional "grant_active_tab"
bool, which was always (outside of tests) set to false.

Bug: None
Change-Id: I02b4c489586f6cbec3ac99b02ce15413f2ceff40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2145858Reviewed-by: default avatarCaroline Rising <corising@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758549}
parent 4329568d
...@@ -112,10 +112,9 @@ void ExtensionActionAPI::RemoveObserver(Observer* observer) { ...@@ -112,10 +112,9 @@ void ExtensionActionAPI::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer); observers_.RemoveObserver(observer);
} }
bool ExtensionActionAPI::ShowExtensionActionPopup( bool ExtensionActionAPI::ShowExtensionActionPopupForAPICall(
const Extension* extension, const Extension* extension,
Browser* browser, Browser* browser) {
bool grant_active_tab_permissions) {
ExtensionAction* extension_action = ExtensionAction* extension_action =
ExtensionActionManager::Get(browser_context_)->GetExtensionAction( ExtensionActionManager::Get(browser_context_)->GetExtensionAction(
*extension); *extension);
...@@ -131,8 +130,8 @@ bool ExtensionActionAPI::ShowExtensionActionPopup( ...@@ -131,8 +130,8 @@ bool ExtensionActionAPI::ShowExtensionActionPopup(
// The ExtensionsContainer could be null if, e.g., this is a popup window with // The ExtensionsContainer could be null if, e.g., this is a popup window with
// no toolbar. // no toolbar.
return extensions_container && return extensions_container &&
extensions_container->ShowToolbarActionPopup( extensions_container->ShowToolbarActionPopupForAPICall(
extension->id(), grant_active_tab_permissions); extension->id());
} }
void ExtensionActionAPI::NotifyChange(ExtensionAction* extension_action, void ExtensionActionAPI::NotifyChange(ExtensionAction* extension_action,
...@@ -530,8 +529,8 @@ ExtensionFunction::ResponseAction BrowserActionOpenPopupFunction::Run() { ...@@ -530,8 +529,8 @@ ExtensionFunction::ResponseAction BrowserActionOpenPopupFunction::Run() {
// fixed. // fixed.
if (!browser || !browser->window()->IsActive() || if (!browser || !browser->window()->IsActive() ||
!browser->window()->IsToolbarVisible() || !browser->window()->IsToolbarVisible() ||
!ExtensionActionAPI::Get(profile)->ShowExtensionActionPopup( !ExtensionActionAPI::Get(profile)->ShowExtensionActionPopupForAPICall(
extension_.get(), browser, false)) { extension_.get(), browser)) {
return RespondNow(Error(kOpenPopupError)); return RespondNow(Error(kOpenPopupError));
} }
......
...@@ -68,12 +68,8 @@ class ExtensionActionAPI : public BrowserContextKeyedAPI { ...@@ -68,12 +68,8 @@ class ExtensionActionAPI : public BrowserContextKeyedAPI {
void RemoveObserver(Observer* observer); void RemoveObserver(Observer* observer);
// Opens the popup for the given |extension| in the given |browser|'s window. // Opens the popup for the given |extension| in the given |browser|'s window.
// If |grant_active_tab_permissions| is true, this grants the extension bool ShowExtensionActionPopupForAPICall(const Extension* extension,
// activeTab (so this should only be done if this is through a direct user Browser* browser);
// action).
bool ShowExtensionActionPopup(const Extension* extension,
Browser* browser,
bool grant_active_tab_permissions);
// Notifies that there has been a change in the given |extension_action|. // Notifies that there has been a change in the given |extension_action|.
void NotifyChange(ExtensionAction* extension_action, void NotifyChange(ExtensionAction* extension_action,
......
...@@ -181,8 +181,8 @@ IN_PROC_BROWSER_TEST_F(PageActionApiTest, DISABLED_ShowPageActionPopup) { ...@@ -181,8 +181,8 @@ IN_PROC_BROWSER_TEST_F(PageActionApiTest, DISABLED_ShowPageActionPopup) {
{ {
ResultCatcher catcher; ResultCatcher catcher;
ExtensionActionAPI::Get(browser()->profile())->ShowExtensionActionPopup( ExtensionActionAPI::Get(browser()->profile())
extension, browser(), true); ->ShowExtensionActionPopupForAPICall(extension, browser());
ASSERT_TRUE(catcher.GetNextResult()); ASSERT_TRUE(catcher.GetNextResult());
} }
} }
......
...@@ -63,11 +63,10 @@ class ExtensionsContainer { ...@@ -63,11 +63,10 @@ class ExtensionsContainer {
bool is_sticky, bool is_sticky,
const base::Closure& closure) = 0; const base::Closure& closure) = 0;
// Shows the popup for the action with |id|, returning true if a popup is // Shows the popup for the action with |id| as the result of an API call,
// shown. If |grant_active_tab| is true, then active tab permissions should // returning true if a popup is shown.
// be given to the action (only do this if this is through a user action). virtual bool ShowToolbarActionPopupForAPICall(
virtual bool ShowToolbarActionPopup(const std::string& action_id, const std::string& action_id) = 0;
bool grant_active_tab) = 0;
// Displays the given |bubble| once the toolbar is no longer animating. // Displays the given |bubble| once the toolbar is no longer animating.
virtual void ShowToolbarActionBubble( virtual void ShowToolbarActionBubble(
......
...@@ -383,14 +383,17 @@ void ToolbarActionsBar::Update() { ...@@ -383,14 +383,17 @@ void ToolbarActionsBar::Update() {
ReorderActions(); // Also triggers a draw. ReorderActions(); // Also triggers a draw.
} }
bool ToolbarActionsBar::ShowToolbarActionPopup(const std::string& action_id, bool ToolbarActionsBar::ShowToolbarActionPopupForAPICall(
bool grant_active_tab) { const std::string& action_id) {
// Don't override another popup, and only show in the active window. // Don't override another popup, and only show in the active window.
if (popup_owner() || !browser_->window()->IsActive()) if (popup_owner() || !browser_->window()->IsActive())
return false; return false;
ToolbarActionViewController* action = GetActionForId(action_id); ToolbarActionViewController* action = GetActionForId(action_id);
return action && action->ExecuteAction(grant_active_tab); // Since this was triggered by an API call, we never want to grant activeTab
// to the extension.
constexpr bool kGrantActiveTab = false;
return action && action->ExecuteAction(kGrantActiveTab);
} }
void ToolbarActionsBar::SetOverflowRowWidth(int width) { void ToolbarActionsBar::SetOverflowRowWidth(int width) {
......
...@@ -243,8 +243,7 @@ class ToolbarActionsBar : public ExtensionsContainer, ...@@ -243,8 +243,7 @@ class ToolbarActionsBar : public ExtensionsContainer,
void PopOutAction(ToolbarActionViewController* action, void PopOutAction(ToolbarActionViewController* action,
bool is_sticky, bool is_sticky,
const base::Closure& closure) override; const base::Closure& closure) override;
bool ShowToolbarActionPopup(const std::string& id, bool ShowToolbarActionPopupForAPICall(const std::string& id) override;
bool grant_active_tab) override;
void ShowToolbarActionBubble( void ShowToolbarActionBubble(
std::unique_ptr<ToolbarActionsBarBubbleDelegate> bubble) override; std::unique_ptr<ToolbarActionsBarBubbleDelegate> bubble) override;
void ShowToolbarActionBubbleAsync( void ShowToolbarActionBubbleAsync(
......
...@@ -325,15 +325,17 @@ void ExtensionsToolbarContainer::PopOutAction( ...@@ -325,15 +325,17 @@ void ExtensionsToolbarContainer::PopOutAction(
animating_layout_manager()->PostOrQueueAction(closure); animating_layout_manager()->PostOrQueueAction(closure);
} }
bool ExtensionsToolbarContainer::ShowToolbarActionPopup( bool ExtensionsToolbarContainer::ShowToolbarActionPopupForAPICall(
const std::string& action_id, const std::string& action_id) {
bool grant_active_tab) {
// Don't override another popup, and only show in the active window. // Don't override another popup, and only show in the active window.
if (popped_out_action_ || !browser_->window()->IsActive()) if (popped_out_action_ || !browser_->window()->IsActive())
return false; return false;
ToolbarActionViewController* action = GetActionForId(action_id); ToolbarActionViewController* action = GetActionForId(action_id);
return action && action->ExecuteAction(grant_active_tab); // Since this was triggered by an API call, we never want to grant activeTab
// to the extension.
constexpr bool kGrantActiveTab = false;
return action && action->ExecuteAction(kGrantActiveTab);
} }
void ExtensionsToolbarContainer::ShowToolbarActionBubble( void ExtensionsToolbarContainer::ShowToolbarActionBubble(
......
...@@ -111,8 +111,7 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView, ...@@ -111,8 +111,7 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView,
void PopOutAction(ToolbarActionViewController* action, void PopOutAction(ToolbarActionViewController* action,
bool is_sticky, bool is_sticky,
const base::Closure& closure) override; const base::Closure& closure) override;
bool ShowToolbarActionPopup(const std::string& action_id, bool ShowToolbarActionPopupForAPICall(const std::string& action_id) override;
bool grant_active_tab) override;
void ShowToolbarActionBubble( void ShowToolbarActionBubble(
std::unique_ptr<ToolbarActionsBarBubbleDelegate> bubble) override; std::unique_ptr<ToolbarActionsBarBubbleDelegate> bubble) override;
void ShowToolbarActionBubbleAsync( void ShowToolbarActionBubbleAsync(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment