diff options
24 files changed, 470 insertions, 25 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 222cbd5..518cf5f 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -3618,8 +3618,8 @@ Even if you have downloaded files from this website before, the website might ha <ph name="EXTENSION_NAME">$1<ex>Adblock</ex></ph> was installed remotely </message> - <!-- Extension toolbar bubble --> <if expr="enable_extensions"> + <!-- Extension toolbar bubble --> <message name="IDS_EXTENSION_TOOLBAR_BUBBLE_HEADING" desc="Heading of the bubble to tell users that all extensions are now visible in the toolbar (the extension toolbar info bubble)."> All your extensions are here </message> @@ -3629,6 +3629,20 @@ Even if you have downloaded files from this website before, the website might ha <message name="IDS_EXTENSION_TOOLBAR_BUBBLE_OK" desc="Button label for the extension toolbar info bubble."> Got it </message> + + <!-- Extension blocked action bubble --> + <message name="IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_HEADING" desc="Heading of the bubble to tell users that in order to run an extension, they'll need to refresh the page."> + Refresh required + </message> + <message name="IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_CONTENT" desc="The content of the bubble to tell users that in order to run an extension, they'll need to refresh the page."> + In order to run this extension, you need to refresh the page. You can run this extension automatically on this site by right-clicking on the extension icon. + </message> + <message name="IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_OK_BUTTON" desc="The text of the button to proceed with the page refresh for running an extension."> + OK, refresh + </message> + <message name="IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_CANCEL_BUTTON" desc="The text of the button to close the bubble and not refresh the page or run the extension."> + Never mind + </message> </if> <!-- Extension install prompt --> diff --git a/chrome/browser/extensions/api/web_request/web_request_apitest.cc b/chrome/browser/extensions/api/web_request/web_request_apitest.cc index 15458a5..8522337 100644 --- a/chrome/browser/extensions/api/web_request/web_request_apitest.cc +++ b/chrome/browser/extensions/api/web_request/web_request_apitest.cc @@ -527,10 +527,22 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, // Grant activeTab permission, and perform another XHR. The extension should // receive the event. EXPECT_EQ(BLOCKED_ACTION_WEB_REQUEST, runner->GetBlockedActions(extension)); + runner->set_default_bubble_close_action_for_testing( + make_scoped_ptr(new ToolbarActionsBarBubbleDelegate::CloseAction( + ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE))); runner->RunAction(extension, true); + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(content::WaitForLoadStop(web_contents)); + // The runner will have refreshed the page... EXPECT_EQ(BLOCKED_ACTION_NONE, runner->GetBlockedActions(extension)); + int xhr_count = GetWebRequestCountFromBackgroundPage(extension, profile()); + // ... which means that we should have a non-zero xhr count. + EXPECT_GT(xhr_count, 0); + // And the extension should receive future events. PerformXhrInPage(web_contents, kHost, port, kXhrPath); - EXPECT_EQ(1, GetWebRequestCountFromBackgroundPage(extension, profile())); + ++xhr_count; + EXPECT_EQ(xhr_count, + GetWebRequestCountFromBackgroundPage(extension, profile())); // If we revoke the extension's tab permissions, it should no longer receive // webRequest events. @@ -540,7 +552,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, granter->RevokeForTesting(); base::RunLoop().RunUntilIdle(); PerformXhrInPage(web_contents, kHost, port, kXhrPath); - EXPECT_EQ(1, GetWebRequestCountFromBackgroundPage(extension, profile())); + EXPECT_EQ(xhr_count, + GetWebRequestCountFromBackgroundPage(extension, profile())); EXPECT_EQ(BLOCKED_ACTION_WEB_REQUEST, runner->GetBlockedActions(extension)); } diff --git a/chrome/browser/extensions/extension_action_runner.cc b/chrome/browser/extensions/extension_action_runner.cc index 8fa7f01..8be229b 100644 --- a/chrome/browser/extensions/extension_action_runner.cc +++ b/chrome/browser/extensions/extension_action_runner.cc @@ -4,6 +4,7 @@ #include "chrome/browser/extensions/extension_action_runner.h" +#include "base/auto_reset.h" #include "base/bind.h" #include "base/bind_helpers.h" #include "base/memory/scoped_ptr.h" @@ -17,6 +18,10 @@ #include "chrome/browser/extensions/tab_helper.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sessions/session_tab_helper.h" +#include "chrome/browser/ui/browser_finder.h" +#include "chrome/browser/ui/browser_window.h" +#include "chrome/browser/ui/extensions/blocked_action_bubble_delegate.h" +#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h" #include "chrome/common/extensions/api/extension_action/action_info.h" #include "components/crx_file/id_util.h" #include "content/public/browser/navigation_controller.h" @@ -35,6 +40,13 @@ namespace extensions { +namespace { + +// The blocked actions that require a page refresh to run. +const int kRefreshRequiredActionsMask = + BLOCKED_ACTION_WEB_REQUEST | BLOCKED_ACTION_SCRIPT_AT_START; +} + ExtensionActionRunner::PendingScript::PendingScript( UserScript::RunLocation run_location, const base::Closure& permit_script) @@ -50,7 +62,9 @@ ExtensionActionRunner::ExtensionActionRunner(content::WebContents* web_contents) num_page_requests_(0), browser_context_(web_contents->GetBrowserContext()), was_used_on_page_(false), - extension_registry_observer_(this) { + ignore_active_tab_granted_(false), + extension_registry_observer_(this), + weak_factory_(this) { CHECK(web_contents); extension_registry_observer_.Add(ExtensionRegistry::Get(browser_context_)); } @@ -71,15 +85,19 @@ ExtensionActionRunner* ExtensionActionRunner::GetForWebContents( ExtensionAction::ShowAction ExtensionActionRunner::RunAction( const Extension* extension, bool grant_tab_permissions) { - bool has_pending_scripts = WantsToRun(extension); if (grant_tab_permissions) { + int blocked = GetBlockedActions(extension); + if ((blocked & kRefreshRequiredActionsMask) != 0) { + ShowBlockedActionBubble(extension); + return ExtensionAction::ACTION_NONE; + } TabHelper::FromWebContents(web_contents()) ->active_tab_permission_granter() ->GrantIfRequested(extension); // If the extension had blocked actions, granting active tab will have // run the extension. Don't execute further since clicking should run // blocked actions *or* the normal extension action, not both. - if (has_pending_scripts) + if (blocked != BLOCKED_ACTION_NONE) return ExtensionAction::ACTION_NONE; } @@ -123,7 +141,7 @@ void ExtensionActionRunner::RunBlockedActions(const Extension* extension) { void ExtensionActionRunner::OnActiveTabPermissionGranted( const Extension* extension) { - if (WantsToRun(extension)) + if (!ignore_active_tab_granted_ && WantsToRun(extension)) RunBlockedActions(extension); } @@ -162,6 +180,14 @@ bool ExtensionActionRunner::WantsToRun(const Extension* extension) { return GetBlockedActions(extension) != BLOCKED_ACTION_NONE; } +void ExtensionActionRunner::RunForTesting(const Extension* extension) { + if (WantsToRun(extension)) { + TabHelper::FromWebContents(web_contents()) + ->active_tab_permission_granter() + ->GrantIfRequested(extension); + } +} + PermissionsData::AccessType ExtensionActionRunner::RequiresUserConsentForScriptInjection( const Extension* extension, @@ -312,6 +338,49 @@ void ExtensionActionRunner::LogUMA() const { } } +void ExtensionActionRunner::ShowBlockedActionBubble( + const Extension* extension) { + Browser* browser = chrome::FindBrowserWithWebContents(web_contents()); + ToolbarActionsBar* toolbar_actions_bar = + browser ? browser->window()->GetToolbarActionsBar() : nullptr; + if (toolbar_actions_bar) { + auto callback = + base::Bind(&ExtensionActionRunner::OnBlockedActionBubbleClosed, + weak_factory_.GetWeakPtr(), extension->id()); + if (default_bubble_close_action_for_testing_) { + base::MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(callback, *default_bubble_close_action_for_testing_)); + } else { + toolbar_actions_bar->ShowToolbarActionBubble(make_scoped_ptr( + new BlockedActionBubbleDelegate(callback, extension->id()))); + } + } +} + +void ExtensionActionRunner::OnBlockedActionBubbleClosed( + const std::string& extension_id, + ToolbarActionsBarBubbleDelegate::CloseAction action) { + // If the user agreed to refresh the page, do so. + if (action == ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE) { + const Extension* extension = ExtensionRegistry::Get(browser_context_) + ->enabled_extensions() + .GetByID(extension_id); + if (!extension) + return; + { + // Ignore the active tab permission being granted because we don't want + // to run scripts right before we refresh the page. + base::AutoReset<bool> ignore_active_tab(&ignore_active_tab_granted_, + true); + TabHelper::FromWebContents(web_contents()) + ->active_tab_permission_granter() + ->GrantIfRequested(extension); + } + web_contents()->GetController().Reload(false); + } +} + bool ExtensionActionRunner::OnMessageReceived( const IPC::Message& message, content::RenderFrameHost* render_frame_host) { @@ -336,6 +405,7 @@ void ExtensionActionRunner::DidNavigateMainFrame( pending_scripts_.clear(); web_request_blocked_.clear(); was_used_on_page_ = false; + weak_factory_.InvalidateWeakPtrs(); } void ExtensionActionRunner::OnExtensionUnloaded( diff --git a/chrome/browser/extensions/extension_action_runner.h b/chrome/browser/extensions/extension_action_runner.h index d985a28..d260ee0 100644 --- a/chrome/browser/extensions/extension_action_runner.h +++ b/chrome/browser/extensions/extension_action_runner.h @@ -15,8 +15,10 @@ #include "base/callback.h" #include "base/compiler_specific.h" #include "base/macros.h" +#include "base/memory/weak_ptr.h" #include "base/scoped_observer.h" #include "chrome/browser/extensions/extension_action.h" +#include "chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h" #include "content/public/browser/web_contents_observer.h" #include "extensions/browser/blocked_action_type.h" #include "extensions/browser/extension_registry_observer.h" @@ -76,8 +78,17 @@ class ExtensionActionRunner : public content::WebContentsObserver, // Returns true if the given |extension| has any blocked actions. bool WantsToRun(const Extension* extension); + // Runs any blocked actions the extension has, but does not handle any page + // refreshes for document_start/webRequest. + void RunForTesting(const Extension* extension); + int num_page_requests() const { return num_page_requests_; } + void set_default_bubble_close_action_for_testing( + scoped_ptr<ToolbarActionsBarBubbleDelegate::CloseAction> action) { + default_bubble_close_action_for_testing_ = std::move(action); + } + #if defined(UNIT_TEST) // Only used in tests. PermissionsData::AccessType RequiresUserConsentForScriptInjectionForTesting( @@ -141,6 +152,15 @@ class ExtensionActionRunner : public content::WebContentsObserver, // Log metrics. void LogUMA() const; + // Shows the bubble to prompt the user to refresh the page to run the blocked + // actions for the given |extension|. + void ShowBlockedActionBubble(const Extension* extension); + + // Called when the blocked actions bubble is closed. + void OnBlockedActionBubbleClosed( + const std::string& extension_id, + ToolbarActionsBarBubbleDelegate::CloseAction action); + // content::WebContentsObserver implementation. bool OnMessageReceived(const IPC::Message& message, content::RenderFrameHost* render_frame_host) override; @@ -177,9 +197,19 @@ class ExtensionActionRunner : public content::WebContentsObserver, // should incorporate more fully with ActiveTab. std::set<std::string> permitted_extensions_; + // If true, ignore active tab being granted rather than running pending + // actions. + bool ignore_active_tab_granted_; + + // If non-null, the bubble action to simulate for testing. + scoped_ptr<ToolbarActionsBarBubbleDelegate::CloseAction> + default_bubble_close_action_for_testing_; + ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver> extension_registry_observer_; + base::WeakPtrFactory<ExtensionActionRunner> weak_factory_; + DISALLOW_COPY_AND_ASSIGN(ExtensionActionRunner); }; diff --git a/chrome/browser/extensions/extension_action_runner_browsertest.cc b/chrome/browser/extensions/extension_action_runner_browsertest.cc index f55091d..5428c3c 100644 --- a/chrome/browser/extensions/extension_action_runner_browsertest.cc +++ b/chrome/browser/extensions/extension_action_runner_browsertest.cc @@ -20,6 +20,8 @@ #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/test/base/ui_test_utils.h" +#include "content/public/browser/navigation_entry.h" +#include "content/public/browser/web_contents.h" #include "content/public/test/browser_test_utils.h" #include "extensions/common/feature_switch.h" #include "extensions/common/switches.h" @@ -63,6 +65,22 @@ bool RunAllPendingInRenderer(content::WebContents* web_contents) { return content::ExecuteScript(web_contents, "1 == 1;"); } +// For use with blocked actions browsertests that put the result in +// window.localStorage. Returns the result or "undefined" if the result is not +// set. +std::string GetValue(content::WebContents* web_contents) { + std::string out; + if (!content::ExecuteScriptAndExtractString( + web_contents, + "var res = window.localStorage.getItem('extResult') || 'undefined';" + "window.localStorage.removeItem('extResult');" + "window.domAutomationController.send(res);", + &out)) { + out = "Failed to inject script"; + } + return out; +} + } // namespace class ExtensionActionRunnerBrowserTest : public ExtensionBrowserTest { @@ -119,7 +137,7 @@ const Extension* ExtensionActionRunnerBrowserTest::CreateExtension( " {" " \"matches\": [\"%s\"]," " \"js\": [\"script.js\"]," - " \"run_at\": \"document_start\"" + " \"run_at\": \"document_end\"" " }" "]", permission_scheme); @@ -427,6 +445,93 @@ IN_PROC_BROWSER_TEST_F(ExtensionActionRunnerBrowserTest, EXPECT_FALSE(inject_success_listener.was_satisfied()); } +IN_PROC_BROWSER_TEST_F(ExtensionActionRunnerBrowserTest, + BlockedActionBrowserTest) { + // Load an extension that wants to run on every page at document start, and + // load a test page. + ASSERT_TRUE(embedded_test_server()->Start()); + const GURL url = embedded_test_server()->GetURL("/simple.html"); + const Extension* extension = LoadExtension( + test_data_dir_.AppendASCII("blocked_actions/content_scripts")); + ASSERT_TRUE(extension); + ui_test_utils::NavigateToURL(browser(), url); + content::WebContents* web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + EXPECT_TRUE(content::WaitForLoadStop(web_contents)); + + // The extension should want to run on the page, and should not have + // injected. + ExtensionActionRunner* runner = + ExtensionActionRunner::GetForWebContents(web_contents); + ASSERT_TRUE(runner); + EXPECT_TRUE(runner->WantsToRun(extension)); + EXPECT_EQ("undefined", GetValue(web_contents)); + + // Wire up the runner to automatically accept the bubble to prompt for page + // refresh. + runner->set_default_bubble_close_action_for_testing( + make_scoped_ptr(new ToolbarActionsBarBubbleDelegate::CloseAction( + ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE))); + + content::NavigationEntry* entry = + web_contents->GetController().GetLastCommittedEntry(); + ASSERT_TRUE(entry); + const int first_nav_id = entry->GetUniqueID(); + + // Run the extension action, which should cause a page refresh (since we + // automatically accepted the bubble prompting us), and the extension should + // have injected at document start. + runner->RunAction(extension, true); + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(content::WaitForLoadStop(web_contents)); + entry = web_contents->GetController().GetLastCommittedEntry(); + ASSERT_TRUE(entry); + // Confirm that we refreshed the page. + EXPECT_GE(entry->GetUniqueID(), first_nav_id); + EXPECT_EQ("success", GetValue(web_contents)); + EXPECT_FALSE(runner->WantsToRun(extension)); + + // Revoke permission and reload to try different bubble options. + ActiveTabPermissionGranter* active_tab_granter = + TabHelper::FromWebContents(web_contents)->active_tab_permission_granter(); + ASSERT_TRUE(active_tab_granter); + active_tab_granter->RevokeForTesting(); + web_contents->GetController().Reload(true); + EXPECT_TRUE(content::WaitForLoadStop(web_contents)); + + // The extension should again want to run. Automatically dismiss the bubble + // that pops up prompting for page refresh. + EXPECT_TRUE(runner->WantsToRun(extension)); + EXPECT_EQ("undefined", GetValue(web_contents)); + const int next_nav_id = + web_contents->GetController().GetLastCommittedEntry()->GetUniqueID(); + runner->set_default_bubble_close_action_for_testing( + make_scoped_ptr(new ToolbarActionsBarBubbleDelegate::CloseAction( + ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION))); + + // Try running the extension. Nothing should happen, because the user + // didn't agree to refresh the page. The extension should still want to run. + runner->RunAction(extension, true); + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(content::WaitForLoadStop(web_contents)); + EXPECT_EQ("undefined", GetValue(web_contents)); + EXPECT_EQ( + next_nav_id, + web_contents->GetController().GetLastCommittedEntry()->GetUniqueID()); + + // Repeat with a dismissal from bubble deactivation - same story. + runner->set_default_bubble_close_action_for_testing( + make_scoped_ptr(new ToolbarActionsBarBubbleDelegate::CloseAction( + ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_DEACTIVATION))); + runner->RunAction(extension, true); + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(content::WaitForLoadStop(web_contents)); + EXPECT_EQ("undefined", GetValue(web_contents)); + EXPECT_EQ( + next_nav_id, + web_contents->GetController().GetLastCommittedEntry()->GetUniqueID()); +} + // A version of the test with the flag off, in order to test that everything // still works as expected. class FlagOffExtensionActionRunnerBrowserTest diff --git a/chrome/browser/extensions/extension_action_runner_unittest.cc b/chrome/browser/extensions/extension_action_runner_unittest.cc index 2aaff8c..edacb84 100644 --- a/chrome/browser/extensions/extension_action_runner_unittest.cc +++ b/chrome/browser/extensions/extension_action_runner_unittest.cc @@ -209,7 +209,7 @@ TEST_F(ExtensionActionRunnerUnitTest, RequestPermissionAndExecute) { EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id())); // Click to accept the extension executing. - runner()->RunAction(extension, true); + runner()->RunForTesting(extension); // The extension should execute, and the extension shouldn't want to run. EXPECT_EQ(1u, GetExecutionCountForExtension(extension->id())); @@ -234,7 +234,7 @@ TEST_F(ExtensionActionRunnerUnitTest, RequestPermissionAndExecute) { // Grant access. RequestInjection(extension); - runner()->RunAction(extension, true); + runner()->RunForTesting(extension); EXPECT_EQ(2u, GetExecutionCountForExtension(extension->id())); EXPECT_FALSE(runner()->WantsToRun(extension)); @@ -266,7 +266,7 @@ TEST_F(ExtensionActionRunnerUnitTest, PendingInjectionsRemovedAtNavigation) { // Request and accept a new injection. RequestInjection(extension); - runner()->RunAction(extension, true); + runner()->RunForTesting(extension); // The extension should only have executed once, even though a grand total // of two executions were requested. @@ -290,7 +290,7 @@ TEST_F(ExtensionActionRunnerUnitTest, MultiplePendingInjection) { EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id())); - runner()->RunAction(extension, true); + runner()->RunForTesting(extension); // All pending injections should have executed. EXPECT_EQ(kNumInjections, GetExecutionCountForExtension(extension->id())); @@ -388,7 +388,7 @@ TEST_F(ExtensionActionRunnerUnitTest, TestAlwaysRun) { // Allow the extension to always run on this origin. ScriptingPermissionsModifier modifier(profile(), extension); modifier.GrantHostPermission(web_contents()->GetLastCommittedURL()); - runner()->RunAction(extension, true); + runner()->RunForTesting(extension); // The extension should execute, and the extension shouldn't want to run. EXPECT_EQ(1u, GetExecutionCountForExtension(extension->id())); @@ -444,7 +444,7 @@ TEST_F(ExtensionActionRunnerUnitTest, TestDifferentScriptRunLocations) { EXPECT_EQ(BLOCKED_ACTION_SCRIPT_AT_START | BLOCKED_ACTION_SCRIPT_OTHER, runner()->GetBlockedActions(extension)); - runner()->RunAction(extension, true); + runner()->RunForTesting(extension); EXPECT_EQ(BLOCKED_ACTION_NONE, runner()->GetBlockedActions(extension)); } diff --git a/chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm b/chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm index 3f97173..d6b7bb5 100644 --- a/chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm +++ b/chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm @@ -167,11 +167,11 @@ const CGFloat kBrowserActionBubbleYOffset = 3.0; // Returns the associated ToolbarController. - (ToolbarController*)toolbarController; -// Creates a message bubble anchored to the given |anchorAction|, or the app -// menu if no |anchorAction| is null. +// Creates a message bubble with the given |delegate| that is anchored to the +// given |anchorView|. - (ToolbarActionsBarBubbleMac*)createMessageBubble: (scoped_ptr<ToolbarActionsBarBubbleDelegate>)delegate - anchorToSelf:(BOOL)anchorToSelf; + anchorView:(NSView*)anchorView; // Called when the window for the active bubble is closing, and sets the active // bubble to nil. @@ -223,6 +223,8 @@ class ToolbarActionsBarBridge : public ToolbarActionsBarDelegate { void ShowExtensionMessageBubble( scoped_ptr<extensions::ExtensionMessageBubbleController> controller, ToolbarActionViewController* anchor_action) override; + void ShowToolbarActionBubble( + scoped_ptr<ToolbarActionsBarBubbleDelegate> bubble) override; // The owning BrowserActionsController; weak. BrowserActionsController* controller_; @@ -297,6 +299,19 @@ int ToolbarActionsBarBridge::GetChevronWidth() const { void ToolbarActionsBarBridge::ShowExtensionMessageBubble( scoped_ptr<extensions::ExtensionMessageBubbleController> bubble_controller, ToolbarActionViewController* anchor_action) { + NSView* anchorView = nil; + BOOL anchoredToAction = NO; + if (anchor_action) { + BrowserActionButton* actionButton = + [controller_ buttonForId:anchor_action->GetId()]; + if (actionButton && [actionButton superview]) { + anchorView = actionButton; + anchoredToAction = YES; + } + } + if (!anchorView) + anchorView = [[controller_ toolbarController] appMenuButton]; + // This goop is a by-product of needing to wire together abstract classes, // C++/Cocoa bridges, and ExtensionMessageBubbleController's somewhat strange // Show() interface. It's ugly, but it's pretty confined, so it's probably @@ -305,14 +320,32 @@ void ToolbarActionsBarBridge::ShowExtensionMessageBubble( bubble_controller.get(); scoped_ptr<ExtensionMessageBubbleBridge> bridge( new ExtensionMessageBubbleBridge(std::move(bubble_controller), - anchor_action != nullptr)); + anchoredToAction)); ToolbarActionsBarBubbleMac* bubble = [controller_ createMessageBubble:std::move(bridge) - anchorToSelf:anchor_action != nil]; + anchorView:anchorView]; weak_controller->OnShown(); [bubble showWindow:nil]; } +void ToolbarActionsBarBridge::ShowToolbarActionBubble( + scoped_ptr<ToolbarActionsBarBubbleDelegate> bubble) { + NSView* anchorView = nil; + if (!bubble->GetAnchorActionId().empty()) { + BrowserActionButton* button = + [controller_ buttonForId:bubble->GetAnchorActionId()]; + anchorView = button && [button superview] ? button : + [[controller_ toolbarController] appMenuButton]; + } else { + anchorView = [controller_ containerView]; + } + + ToolbarActionsBarBubbleMac* bubbleView = + [controller_ createMessageBubble:std::move(bubble) + anchorView:anchorView]; + [bubbleView showWindow:nil]; +} + } // namespace @implementation BrowserActionsController @@ -826,7 +859,8 @@ void ToolbarActionsBarBridge::ShowExtensionMessageBubble( scoped_ptr<ToolbarActionsBarBubbleDelegate> delegate( new ExtensionToolbarIconSurfacingBubbleDelegate(browser_->profile())); ToolbarActionsBarBubbleMac* bubble = - [self createMessageBubble:std::move(delegate) anchorToSelf:YES]; + [self createMessageBubble:std::move(delegate) + anchorView:containerView_]; [bubble showWindow:nil]; } [containerView_ setTrackingEnabled:NO]; @@ -1042,10 +1076,9 @@ void ToolbarActionsBarBridge::ShowExtensionMessageBubble( - (ToolbarActionsBarBubbleMac*)createMessageBubble: (scoped_ptr<ToolbarActionsBarBubbleDelegate>)delegate - anchorToSelf:(BOOL)anchorToSelf { + anchorView:(NSView*)anchorView { + DCHECK(anchorView); DCHECK_GE([buttons_ count], 0u); - NSView* anchorView = - anchorToSelf ? containerView_ : [[self toolbarController] appMenuButton]; NSPoint anchor = [self popupPointForView:anchorView withBounds:[anchorView bounds]]; diff --git a/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.h b/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.h index 5a86977..ab925a4 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.h +++ b/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.h @@ -32,6 +32,7 @@ class ExtensionMessageBubbleBridge : public ToolbarActionsBarBubbleDelegate { base::string16 GetActionButtonText() override; base::string16 GetDismissButtonText() override; base::string16 GetLearnMoreButtonText() override; + std::string GetAnchorActionId() override; void OnBubbleShown() override; void OnBubbleClosed(CloseAction action) override; diff --git a/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.mm b/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.mm index f7cb1f5..ae34ee2 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.mm @@ -44,6 +44,11 @@ base::string16 ExtensionMessageBubbleBridge::GetLearnMoreButtonText() { return controller_->delegate()->GetLearnMoreLabel(); } +std::string ExtensionMessageBubbleBridge::GetAnchorActionId() { + return controller_->GetExtensionIdList().size() == 1u ? + controller_->GetExtensionIdList()[0] : std::string(); +} + void ExtensionMessageBubbleBridge::OnBubbleShown() { } diff --git a/chrome/browser/ui/extensions/blocked_action_bubble_delegate.cc b/chrome/browser/ui/extensions/blocked_action_bubble_delegate.cc new file mode 100644 index 0000000..288ffef --- /dev/null +++ b/chrome/browser/ui/extensions/blocked_action_bubble_delegate.cc @@ -0,0 +1,53 @@ +// Copyright 2016 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/extensions/blocked_action_bubble_delegate.h" + +#include "base/callback_helpers.h" +#include "base/strings/string16.h" +#include "grit/generated_resources.h" +#include "ui/base/l10n/l10n_util.h" + +BlockedActionBubbleDelegate::BlockedActionBubbleDelegate( + const base::Callback<void(CloseAction)>& callback, + const std::string& extension_id) + : callback_(callback), extension_id_(extension_id) {} + +BlockedActionBubbleDelegate::~BlockedActionBubbleDelegate() {} + +base::string16 BlockedActionBubbleDelegate::GetHeadingText() { + return l10n_util::GetStringUTF16(IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_HEADING); +} + +base::string16 BlockedActionBubbleDelegate::GetBodyText() { + return l10n_util::GetStringUTF16(IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_CONTENT); +} + +base::string16 BlockedActionBubbleDelegate::GetItemListText() { + return base::string16(); // No item list. +} + +base::string16 BlockedActionBubbleDelegate::GetActionButtonText() { + return l10n_util::GetStringUTF16( + IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_OK_BUTTON); +} + +base::string16 BlockedActionBubbleDelegate::GetDismissButtonText() { + return l10n_util::GetStringUTF16( + IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_CANCEL_BUTTON); +} + +base::string16 BlockedActionBubbleDelegate::GetLearnMoreButtonText() { + return base::string16(); // No learn more link. +} + +std::string BlockedActionBubbleDelegate::GetAnchorActionId() { + return extension_id_; +} + +void BlockedActionBubbleDelegate::OnBubbleShown() {} + +void BlockedActionBubbleDelegate::OnBubbleClosed(CloseAction action) { + base::ResetAndReturn(&callback_).Run(action); +} diff --git a/chrome/browser/ui/extensions/blocked_action_bubble_delegate.h b/chrome/browser/ui/extensions/blocked_action_bubble_delegate.h new file mode 100644 index 0000000..8e2200e --- /dev/null +++ b/chrome/browser/ui/extensions/blocked_action_bubble_delegate.h @@ -0,0 +1,38 @@ +// Copyright 2016 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. + +#ifndef CHROME_BROWSER_UI_EXTENSIONS_BLOCKED_ACTION_BUBBLE_DELEGATE_H_ +#define CHROME_BROWSER_UI_EXTENSIONS_BLOCKED_ACTION_BUBBLE_DELEGATE_H_ + +#include "base/callback.h" +#include "base/macros.h" +#include "chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h" + +// The delegate for the bubble to ask the user if they want to refresh the page +// in order to run any blocked actions the extension may have. +class BlockedActionBubbleDelegate : public ToolbarActionsBarBubbleDelegate { + public: + BlockedActionBubbleDelegate(const base::Callback<void(CloseAction)>& callback, + const std::string& extension_id); + ~BlockedActionBubbleDelegate() override; + + private: + // ToolbarActionsBarBubbleDelegate: + base::string16 GetHeadingText() override; + base::string16 GetBodyText() override; + base::string16 GetItemListText() override; + base::string16 GetActionButtonText() override; + base::string16 GetDismissButtonText() override; + base::string16 GetLearnMoreButtonText() override; + std::string GetAnchorActionId() override; + void OnBubbleShown() override; + void OnBubbleClosed(CloseAction action) override; + + base::Callback<void(CloseAction)> callback_; + std::string extension_id_; + + DISALLOW_COPY_AND_ASSIGN(BlockedActionBubbleDelegate); +}; + +#endif // CHROME_BROWSER_UI_EXTENSIONS_BLOCKED_ACTION_BUBBLE_DELEGATE_H_ diff --git a/chrome/browser/ui/extensions/extension_action_view_controller.cc b/chrome/browser/ui/extensions/extension_action_view_controller.cc index bdbb532..4b9f123 100644 --- a/chrome/browser/ui/extensions/extension_action_view_controller.cc +++ b/chrome/browser/ui/extensions/extension_action_view_controller.cc @@ -216,7 +216,7 @@ bool ExtensionActionViewController::ExecuteAction(PopupShowAction show_action, if (action_runner->RunAction(extension(), grant_tab_permissions) == ExtensionAction::ACTION_SHOW_POPUP) { GURL popup_url = extension_action_->GetPopupUrl( - SessionTabHelper::IdForTab(view_delegate_->GetCurrentWebContents())); + SessionTabHelper::IdForTab(web_contents)); return GetPreferredPopupViewController() ->TriggerPopupWithUrl(show_action, popup_url, grant_tab_permissions); } diff --git a/chrome/browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.cc b/chrome/browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.cc index a2c8ffc..1151155 100644 --- a/chrome/browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.cc +++ b/chrome/browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.cc @@ -91,6 +91,10 @@ ExtensionToolbarIconSurfacingBubbleDelegate::GetLearnMoreButtonText() { return base::string16(); // No learn more link. } +std::string ExtensionToolbarIconSurfacingBubbleDelegate::GetAnchorActionId() { + return std::string(); // Point to the whole set of actions. +} + void ExtensionToolbarIconSurfacingBubbleDelegate::OnBubbleShown() { // Record the last time the bubble was shown. profile_->GetPrefs()->SetInt64( diff --git a/chrome/browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.h b/chrome/browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.h index 527201a..85e4caf 100644 --- a/chrome/browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.h +++ b/chrome/browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.h @@ -29,6 +29,7 @@ class ExtensionToolbarIconSurfacingBubbleDelegate base::string16 GetActionButtonText() override; base::string16 GetDismissButtonText() override; base::string16 GetLearnMoreButtonText() override; + std::string GetAnchorActionId() override; void OnBubbleShown() override; void OnBubbleClosed(CloseAction action) override; diff --git a/chrome/browser/ui/toolbar/test_toolbar_actions_bar_bubble_delegate.cc b/chrome/browser/ui/toolbar/test_toolbar_actions_bar_bubble_delegate.cc index 957ad6f..fa13f07 100644 --- a/chrome/browser/ui/toolbar/test_toolbar_actions_bar_bubble_delegate.cc +++ b/chrome/browser/ui/toolbar/test_toolbar_actions_bar_bubble_delegate.cc @@ -23,6 +23,7 @@ class TestToolbarActionsBarBubbleDelegate::DelegateImpl base::string16 GetLearnMoreButtonText() override { return parent_->learn_more_; } + std::string GetAnchorActionId() override { return std::string(); } void OnBubbleShown() override { CHECK(!parent_->shown_); parent_->shown_ = true; diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc index 546acd4..cb62c0c 100644 --- a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc +++ b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc @@ -493,6 +493,8 @@ void ToolbarActionsBar::OnAnimationEnded() { // message bubble, or to show a popup. if (pending_extension_bubble_controller_) { MaybeShowExtensionBubble(std::move(pending_extension_bubble_controller_)); + } else if (pending_toolbar_bubble_controller_) { + ShowToolbarActionBubble(std::move(pending_toolbar_bubble_controller_)); } else if (!popped_out_closure_.is_null()) { popped_out_closure_.Run(); popped_out_closure_.Reset(); @@ -575,6 +577,16 @@ void ToolbarActionsBar::RemoveObserver(ToolbarActionsBarObserver* observer) { observers_.RemoveObserver(observer); } +void ToolbarActionsBar::ShowToolbarActionBubble( + scoped_ptr<ToolbarActionsBarBubbleDelegate> bubble) { + DCHECK(bubble->GetAnchorActionId().empty() || + GetActionForId(bubble->GetAnchorActionId())); + if (delegate_->IsAnimating()) + pending_toolbar_bubble_controller_ = std::move(bubble); + else + delegate_->ShowToolbarActionBubble(std::move(bubble)); +} + void ToolbarActionsBar::MaybeShowExtensionBubble( scoped_ptr<extensions::ExtensionMessageBubbleController> controller) { controller->HighlightExtensionsIfNecessary(); // Safe to call multiple times. diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar.h b/chrome/browser/ui/toolbar/toolbar_actions_bar.h index 1c04dae..871db82 100644 --- a/chrome/browser/ui/toolbar/toolbar_actions_bar.h +++ b/chrome/browser/ui/toolbar/toolbar_actions_bar.h @@ -211,6 +211,10 @@ class ToolbarActionsBar : public ToolbarActionsModel::Observer { void AddObserver(ToolbarActionsBarObserver* observer); void RemoveObserver(ToolbarActionsBarObserver* observer); + // Displays the given |bubble| once the toolbar is no longer animating. + void ShowToolbarActionBubble( + scoped_ptr<ToolbarActionsBarBubbleDelegate> bubble); + // Returns the underlying toolbar actions, but does not order them. Primarily // for use in testing. const std::vector<ToolbarActionViewController*>& toolbar_actions_unordered() @@ -336,10 +340,17 @@ class ToolbarActionsBar : public ToolbarActionsModel::Observer { // it is fully popped out. base::Closure popped_out_closure_; - // The controller of the bubble to show once animation finishes, if any. + // The controller of the extension message bubble to show once animation + // finishes, if any. This has priority over + // |pending_toolbar_bubble_controller_|. scoped_ptr<extensions::ExtensionMessageBubbleController> pending_extension_bubble_controller_; + // The controller for the toolbar action bubble to show once animation + // finishes, if any. + scoped_ptr<ToolbarActionsBarBubbleDelegate> + pending_toolbar_bubble_controller_; + base::ObserverList<ToolbarActionsBarObserver> observers_; base::WeakPtrFactory<ToolbarActionsBar> weak_ptr_factory_; diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h b/chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h index 508371e..d07dc71 100644 --- a/chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h +++ b/chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h @@ -5,6 +5,8 @@ #ifndef CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_ACTIONS_BAR_BUBBLE_DELEGATE_H_ #define CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_ACTIONS_BAR_BUBBLE_DELEGATE_H_ +#include <string> + #include "base/strings/string16.h" // A delegate for a generic bubble that hangs off the toolbar actions bar. @@ -43,6 +45,10 @@ class ToolbarActionsBarBubbleDelegate { // string, no button will be added. virtual base::string16 GetLearnMoreButtonText() = 0; + // Returns the id of the action to point to, or the empty string if the + // bubble should point to the center of the actions container. + virtual std::string GetAnchorActionId() = 0; + // Called when the bubble is shown. virtual void OnBubbleShown() = 0; diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar_delegate.h b/chrome/browser/ui/toolbar/toolbar_actions_bar_delegate.h index 584a4b3..4c31025 100644 --- a/chrome/browser/ui/toolbar/toolbar_actions_bar_delegate.h +++ b/chrome/browser/ui/toolbar/toolbar_actions_bar_delegate.h @@ -12,6 +12,7 @@ #include "ui/gfx/geometry/size.h" class ToolbarActionViewController; +class ToolbarActionsBarBubbleDelegate; namespace extensions { class ExtensionMessageBubbleController; @@ -70,6 +71,10 @@ class ToolbarActionsBarDelegate { virtual void ShowExtensionMessageBubble( scoped_ptr<extensions::ExtensionMessageBubbleController> controller, ToolbarActionViewController* anchor_action) = 0; + + // Shows the given |bubble| if no other bubbles are showing. + virtual void ShowToolbarActionBubble( + scoped_ptr<ToolbarActionsBarBubbleDelegate> bubble) = 0; }; #endif // CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_ACTIONS_BAR_DELEGATE_H_ diff --git a/chrome/browser/ui/views/toolbar/browser_actions_container.cc b/chrome/browser/ui/views/toolbar/browser_actions_container.cc index 5ac758e..2d704d4 100644 --- a/chrome/browser/ui/views/toolbar/browser_actions_container.cc +++ b/chrome/browser/ui/views/toolbar/browser_actions_container.cc @@ -319,6 +319,25 @@ int BrowserActionsContainer::GetChevronWidth() const { chevron_->GetPreferredSize().width() + GetChevronSpacing() : 0; } +void BrowserActionsContainer::ShowToolbarActionBubble( + scoped_ptr<ToolbarActionsBarBubbleDelegate> controller) { + // The container shouldn't be asked to show a bubble if it's animating. + DCHECK(!animating()); + views::View* anchor_view = nullptr; + if (!controller->GetAnchorActionId().empty()) { + ToolbarActionView* action_view = + GetViewForId(controller->GetAnchorActionId()); + anchor_view = + action_view->visible() ? action_view : GetOverflowReferenceView(); + } else { + anchor_view = this; + } + ToolbarActionsBarBubbleViews* bubble = + new ToolbarActionsBarBubbleViews(anchor_view, std::move(controller)); + views::BubbleDelegateView::CreateBubble(bubble); + bubble->Show(); +} + void BrowserActionsContainer::ShowExtensionMessageBubble( scoped_ptr<extensions::ExtensionMessageBubbleController> controller, ToolbarActionViewController* anchor_action) { diff --git a/chrome/browser/ui/views/toolbar/browser_actions_container.h b/chrome/browser/ui/views/toolbar/browser_actions_container.h index 14517aa..f77f7cb 100644 --- a/chrome/browser/ui/views/toolbar/browser_actions_container.h +++ b/chrome/browser/ui/views/toolbar/browser_actions_container.h @@ -239,6 +239,8 @@ class BrowserActionsContainer : public views::View, bool IsAnimating() const override; void StopAnimating() override; int GetChevronWidth() const override; + void ShowToolbarActionBubble( + scoped_ptr<ToolbarActionsBarBubbleDelegate> controller) override; void ShowExtensionMessageBubble( scoped_ptr<extensions::ExtensionMessageBubbleController> controller, ToolbarActionViewController* anchor_action) override; diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi index 3239cda..75fa4985 100644 --- a/chrome/chrome_browser_ui.gypi +++ b/chrome/chrome_browser_ui.gypi @@ -2769,6 +2769,8 @@ 'browser/ui/extensions/extension_message_bubble_factory.h', 'browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.cc', 'browser/ui/extensions/extension_toolbar_icon_surfacing_bubble_delegate.h', + 'browser/ui/extensions/blocked_action_bubble_delegate.cc', + 'browser/ui/extensions/blocked_action_bubble_delegate.h', 'browser/ui/extensions/hosted_app_browser_controller.cc', 'browser/ui/extensions/hosted_app_browser_controller.h', 'browser/ui/extensions/icon_with_badge_image_source.cc', diff --git a/chrome/test/data/extensions/blocked_actions/content_scripts/manifest.json b/chrome/test/data/extensions/blocked_actions/content_scripts/manifest.json new file mode 100644 index 0000000..70d666b --- /dev/null +++ b/chrome/test/data/extensions/blocked_actions/content_scripts/manifest.json @@ -0,0 +1,11 @@ +{ + "name": "Blocked Actions Doc Start", + "description": "An extension that wants to run at document start on all urls", + "version": "0.1", + "manifest_version": 2, + "content_scripts": [{ + "matches": ["<all_urls>"], + "run_at": "document_start", + "js": ["script.js"] + }] +} diff --git a/chrome/test/data/extensions/blocked_actions/content_scripts/script.js b/chrome/test/data/extensions/blocked_actions/content_scripts/script.js new file mode 100644 index 0000000..c54b92c --- /dev/null +++ b/chrome/test/data/extensions/blocked_actions/content_scripts/script.js @@ -0,0 +1,9 @@ +// Copyright 2016 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. + +// If the script was really injected at document_start, then document.body will +// be null. If it's not null, then we didn't inject at document_start. +// Store the result in window.localStorage so that it's accessible from the +// main world script context in the browsertest. +window.localStorage.setItem('extResult', document.body ? 'failure' : 'success'); |