diff options
32 files changed, 790 insertions, 172 deletions
diff --git a/chrome/browser/extensions/active_script_controller.cc b/chrome/browser/extensions/active_script_controller.cc new file mode 100644 index 0000000..886ee2d --- /dev/null +++ b/chrome/browser/extensions/active_script_controller.cc @@ -0,0 +1,131 @@ +// Copyright 2014 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/extensions/active_script_controller.h" + +#include "base/metrics/histogram.h" +#include "base/stl_util.h" +#include "chrome/browser/extensions/extension_action.h" +#include "chrome/common/extensions/api/extension_action/action_info.h" +#include "content/public/browser/navigation_controller.h" +#include "content/public/browser/navigation_entry.h" +#include "content/public/browser/web_contents.h" +#include "extensions/browser/extension_registry.h" +#include "extensions/common/extension.h" +#include "extensions/common/extension_messages.h" +#include "extensions/common/extension_set.h" +#include "extensions/common/feature_switch.h" +#include "extensions/common/permissions/permissions_data.h" +#include "ipc/ipc_message_macros.h" + +namespace extensions { + +ActiveScriptController::ActiveScriptController( + content::WebContents* web_contents) + : content::WebContentsObserver(web_contents), + enabled_(FeatureSwitch::scripts_require_action()->IsEnabled()) { +} + +ActiveScriptController::~ActiveScriptController() { + LogUMA(); +} + +void ActiveScriptController::NotifyScriptExecuting( + const std::string& extension_id, int page_id) { + if (extensions_executing_scripts_.count(extension_id) || + web_contents()->GetController().GetVisibleEntry()->GetPageID() != + page_id) { + return; + } + + const Extension* extension = + ExtensionRegistry::Get(web_contents()->GetBrowserContext()) + ->enabled_extensions().GetByID(extension_id); + if (extension && + PermissionsData::RequiresActionForScriptExecution(extension)) { + extensions_executing_scripts_.insert(extension_id); + LocationBarController::NotifyChange(web_contents()); + } +} + +void ActiveScriptController::OnAdInjectionDetected( + const std::vector<std::string> ad_injectors) { + size_t num_preventable_ad_injectors = + base::STLSetIntersection<std::set<std::string> >( + ad_injectors, extensions_executing_scripts_).size(); + + UMA_HISTOGRAM_COUNTS_100( + "Extensions.ActiveScriptController.PreventableAdInjectors", + num_preventable_ad_injectors); + UMA_HISTOGRAM_COUNTS_100( + "Extensions.ActiveScriptController.PreventableAdInjectors", + ad_injectors.size() - num_preventable_ad_injectors); +} + +ExtensionAction* ActiveScriptController::GetActionForExtension( + const Extension* extension) { + if (!enabled_ || extensions_executing_scripts_.count(extension->id()) == 0) + return NULL; // No action for this extension. + + ActiveScriptMap::iterator existing = + active_script_actions_.find(extension->id()); + if (existing != active_script_actions_.end()) + return existing->second.get(); + + linked_ptr<ExtensionAction> action(new ExtensionAction( + extension->id(), ActionInfo::TYPE_PAGE, ActionInfo())); + action->SetTitle(ExtensionAction::kDefaultTabId, extension->name()); + action->SetIsVisible(ExtensionAction::kDefaultTabId, true); + + const ActionInfo* action_info = ActionInfo::GetPageActionInfo(extension); + if (!action_info) + action_info = ActionInfo::GetBrowserActionInfo(extension); + + if (action_info && !action_info->default_icon.empty()) { + action->set_default_icon( + make_scoped_ptr(new ExtensionIconSet(action_info->default_icon))); + } + + active_script_actions_[extension->id()] = action; + return action.get(); +} + +LocationBarController::Action ActiveScriptController::OnClicked( + const Extension* extension) { + DCHECK(extensions_executing_scripts_.count(extension->id()) > 0); + return LocationBarController::ACTION_NONE; +} + +void ActiveScriptController::OnNavigated() { + LogUMA(); + extensions_executing_scripts_.clear(); +} + +bool ActiveScriptController::OnMessageReceived(const IPC::Message& message) { + bool handled = true; + IPC_BEGIN_MESSAGE_MAP(ActiveScriptController, message) + IPC_MESSAGE_HANDLER(ExtensionHostMsg_NotifyExtensionScriptExecution, + OnNotifyExtensionScriptExecution) + IPC_MESSAGE_UNHANDLED(handled = false) + IPC_END_MESSAGE_MAP() + return handled; +} + +void ActiveScriptController::OnNotifyExtensionScriptExecution( + const std::string& extension_id, + int page_id) { + if (!Extension::IdIsValid(extension_id)) { + NOTREACHED() << "'" << extension_id << "' is not a valid id."; + return; + } + NotifyScriptExecuting(extension_id, page_id); +} + +void ActiveScriptController::LogUMA() const { + UMA_HISTOGRAM_COUNTS_100( + "Extensions.ActiveScriptController.ShownActiveScriptsOnPage", + extensions_executing_scripts_.size()); +} + +} // namespace extensions diff --git a/chrome/browser/extensions/active_script_controller.h b/chrome/browser/extensions/active_script_controller.h new file mode 100644 index 0000000..8765833 --- /dev/null +++ b/chrome/browser/extensions/active_script_controller.h @@ -0,0 +1,88 @@ +// Copyright 2014 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_EXTENSIONS_ACTIVE_SCRIPT_CONTROLLER_H_ +#define CHROME_BROWSER_EXTENSIONS_ACTIVE_SCRIPT_CONTROLLER_H_ + +#include <map> +#include <set> +#include <string> + +#include "base/compiler_specific.h" +#include "base/memory/linked_ptr.h" +#include "chrome/browser/extensions/location_bar_controller.h" +#include "content/public/browser/web_contents_observer.h" + +namespace content { +class WebContents; +} + +namespace IPC { +class Message; +} + +class ExtensionAction; + +namespace extensions { +class Extension; + +// The provider for ExtensionActions corresponding to scripts which are actively +// running or need permission. +// TODO(rdevlin.cronin): This isn't really a controller, but it has good parity +// with PageAction"Controller". +class ActiveScriptController : public LocationBarController::ActionProvider, + public content::WebContentsObserver { + public: + explicit ActiveScriptController(content::WebContents* web_contents); + virtual ~ActiveScriptController(); + + // Notify the ActiveScriptController that an extension is running a script. + // TODO(rdevlin.cronin): Soon, this should be ask the user for permission, + // rather than simply notifying them. + void NotifyScriptExecuting(const std::string& extension_id, int page_id); + + // Notifies the ActiveScriptController of detected ad injection. + void OnAdInjectionDetected(const std::vector<std::string> ad_injectors); + + // LocationBarControllerProvider implementation. + virtual ExtensionAction* GetActionForExtension( + const Extension* extension) OVERRIDE; + virtual LocationBarController::Action OnClicked( + const Extension* extension) OVERRIDE; + virtual void OnNavigated() OVERRIDE; + + private: + // content::WebContentsObserver implementation. + virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; + + // Handle the NotifyExtensionScriptExecution message. + void OnNotifyExtensionScriptExecution(const std::string& extension_id, + int page_id); + + // Log metrics. + void LogUMA() const; + + // Whether or not the ActiveScriptController is enabled (corresponding to the + // kActiveScriptEnforcement switch). If it is not, it acts as an empty shell, + // always allowing scripts to run and never displaying actions. + bool enabled_; + + // The extensions that have called ExecuteScript on the current frame. + std::set<std::string> extensions_executing_scripts_; + + // The extensions which have injected ads. + std::set<std::string> ad_injectors_; + + // Script badges that have been generated for extensions. This is both those + // with actions already declared that are copied and normalised, and actions + // that get generated for extensions that haven't declared anything. + typedef std::map<std::string, linked_ptr<ExtensionAction> > ActiveScriptMap; + ActiveScriptMap active_script_actions_; + + DISALLOW_COPY_AND_ASSIGN(ActiveScriptController); +}; + +} // namespace extensions + +#endif // CHROME_BROWSER_EXTENSIONS_ACTIVE_SCRIPT_CONTROLLER_H_ diff --git a/chrome/browser/extensions/active_script_controller_browsertest.cc b/chrome/browser/extensions/active_script_controller_browsertest.cc new file mode 100644 index 0000000..5be5b71 --- /dev/null +++ b/chrome/browser/extensions/active_script_controller_browsertest.cc @@ -0,0 +1,141 @@ +// Copyright 2014 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 "base/macros.h" +#include "chrome/browser/extensions/active_script_controller.h" +#include "chrome/browser/extensions/extension_action.h" +#include "chrome/browser/extensions/extension_browsertest.h" +#include "chrome/browser/extensions/extension_test_message_listener.h" +#include "chrome/browser/extensions/location_bar_controller.h" +#include "chrome/browser/extensions/tab_helper.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/test/base/ui_test_utils.h" +#include "extensions/common/feature_switch.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace extensions { + +class ActiveScriptControllerBrowserTest : public ExtensionBrowserTest { + public: + ActiveScriptControllerBrowserTest() + : feature_override_(FeatureSwitch::scripts_require_action(), + FeatureSwitch::OVERRIDE_ENABLED) {} + private: + FeatureSwitch::ScopedOverride feature_override_; +}; + +class ActiveScriptTester { + public: + ActiveScriptTester(const std::string& name, + const Extension* extension, + bool expect_has_action); + ~ActiveScriptTester(); + + testing::AssertionResult Verify(Browser* browser); + + private: + // The name of the extension, and also the message it sends. + std::string name_; + + // The extension associated with this tester. + const Extension* extension_; + + // Whether or not we expect the extension to have an action for script + // injection. + bool expect_has_action_; + + // All of these extensions should inject a script (either through content + // scripts or through chrome.tabs.executeScript()) that sends a message with + // their names (for verification). + // Be prepared to wait for each extension to inject the script. + linked_ptr<ExtensionTestMessageListener> listener_; +}; + +ActiveScriptTester::ActiveScriptTester(const std::string& name, + const Extension* extension, + bool expect_has_action) + : name_(name), + extension_(extension), + expect_has_action_(expect_has_action), + listener_( + new ExtensionTestMessageListener(name, false /* won't reply */)) { +} + +ActiveScriptTester::~ActiveScriptTester() { +} + +testing::AssertionResult ActiveScriptTester::Verify(Browser* browser) { + if (!extension_) + return testing::AssertionFailure() << "Could not load extension: " << name_; + + listener_->WaitUntilSatisfied(); + content::WebContents* web_contents = + browser ? browser->tab_strip_model()->GetActiveWebContents() : NULL; + + if (!web_contents) + return testing::AssertionFailure() << "Could not find web contents."; + + TabHelper* tab_helper = TabHelper::FromWebContents(web_contents); + if (!tab_helper) + return testing::AssertionFailure() << "Could not find tab helper."; + + ActiveScriptController* controller = + tab_helper->location_bar_controller()->active_script_controller(); + if (!controller) + return testing::AssertionFailure() << "Could not find controller."; + + bool has_action = controller->GetActionForExtension(extension_) != NULL; + if (expect_has_action_ != has_action) { + return testing::AssertionFailure() + << "Improper action status: expected " << expect_has_action_ + << ", found " << has_action; + } + + return testing::AssertionSuccess(); +} + +IN_PROC_BROWSER_TEST_F(ActiveScriptControllerBrowserTest, + ActiveScriptsAreDisplayed) { + base::FilePath active_script_path = + test_data_dir_.AppendASCII("active_script"); + + const char* kExtensionNames[] = { + "content_scripts_all_hosts", + "inject_scripts_all_hosts", + "content_scripts_explicit_hosts" + }; + + // First, we load up three extensions: + // - An extension with a content script that runs on all hosts, + // - An extension that injects scripts into all hosts, + // - An extension with a content script that runs on explicit hosts. + ActiveScriptTester testers[] = { + ActiveScriptTester( + kExtensionNames[0], + LoadExtension(active_script_path.AppendASCII(kExtensionNames[0])), + true /* expect action */), + ActiveScriptTester( + kExtensionNames[1], + LoadExtension(active_script_path.AppendASCII(kExtensionNames[1])), + true /* expect action */), + ActiveScriptTester( + kExtensionNames[2], + LoadExtension(active_script_path.AppendASCII(kExtensionNames[2])), + false /* expect action */) + }; + + // Navigate to an URL (which matches the explicit host specified in the + // extension content_scripts_explicit_hosts). All three extensions should + // inject the script. + ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady()); + ui_test_utils::NavigateToURL( + browser(), embedded_test_server()->GetURL("/extensions/test_file.html")); + + for (size_t i = 0u; i < arraysize(testers); ++i) + ASSERT_TRUE(testers[i].Verify(browser())) << kExtensionNames[i]; +} + +} // namespace extensions diff --git a/chrome/browser/extensions/activity_log/uma_policy.cc b/chrome/browser/extensions/activity_log/uma_policy.cc index e94f781..ccccb42 100644 --- a/chrome/browser/extensions/activity_log/uma_policy.cc +++ b/chrome/browser/extensions/activity_log/uma_policy.cc @@ -7,8 +7,11 @@ #include "base/metrics/histogram.h" #include "base/strings/stringprintf.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/extensions/active_script_controller.h" #include "chrome/browser/extensions/activity_log/activity_action_constants.h" #include "chrome/browser/extensions/activity_log/ad_network_database.h" +#include "chrome/browser/extensions/location_bar_controller.h" +#include "chrome/browser/extensions/tab_helper.h" #include "chrome/browser/sessions/session_id.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_list.h" @@ -41,6 +44,9 @@ const int kAdInjected = 1 << UmaPolicy::AD_INJECTED; const int kAdRemoved = 1 << UmaPolicy::AD_REMOVED; const int kAdReplaced = 1 << UmaPolicy::AD_REPLACED; +// A mask of all the ad injection flags. +const int kAnyAdActivity = kAdInjected | kAdRemoved | kAdReplaced; + } // namespace // Class constants, also used in testing. -------------------------------------- @@ -159,34 +165,49 @@ int UmaPolicy::MatchActionToStatus(scoped_refptr<Action> action) { break; case Action::NUM_INJECTION_TYPES: NOTREACHED(); - }; + } return ret_bit; } -void UmaPolicy::HistogramOnClose(const std::string& url) { +void UmaPolicy::HistogramOnClose(const std::string& cleaned_url, + content::WebContents* web_contents) { // Let's try to avoid histogramming useless URLs. - if (url.empty() || url == content::kAboutBlankURL || - url == chrome::kChromeUINewTabURL) + if (cleaned_url.empty() || cleaned_url == content::kAboutBlankURL || + cleaned_url == chrome::kChromeUINewTabURL) return; int statuses[MAX_STATUS - 1]; std::memset(statuses, 0, sizeof(statuses)); - SiteMap::iterator site_lookup = url_status_.find(url); - ExtensionMap exts = site_lookup->second; - ExtensionMap::iterator ext_iter; - for (ext_iter = exts.begin(); ext_iter != exts.end(); ++ext_iter) { + // |web_contents| can be NULL in unit tests. + TabHelper* tab_helper = + web_contents ? TabHelper::FromWebContents(web_contents) : NULL; + ActiveScriptController* active_script_controller = + tab_helper && tab_helper->location_bar_controller() ? + tab_helper->location_bar_controller()->active_script_controller() : + NULL; + SiteMap::iterator site_lookup = url_status_.find(cleaned_url); + const ExtensionMap& exts = site_lookup->second; + std::vector<std::string> ad_injectors; + for (ExtensionMap::const_iterator ext_iter = exts.begin(); + ext_iter != exts.end(); + ++ext_iter) { if (ext_iter->first == kNumberOfTabs) continue; for (int i = NONE + 1; i < MAX_STATUS; ++i) { if (ext_iter->second & (1 << i)) statuses[i-1]++; } + + if ((ext_iter->second & kAnyAdActivity) && active_script_controller) + ad_injectors.push_back(ext_iter->first); } + if (active_script_controller) + active_script_controller->OnAdInjectionDetected(ad_injectors); std::string prefix = "ExtensionActivity."; - if (GURL(url).host() != "www.google.com") { + if (GURL(cleaned_url).host() != "www.google.com") { UMA_HISTOGRAM_COUNTS_100(prefix + GetHistogramName(CONTENT_SCRIPT), statuses[CONTENT_SCRIPT - 1]); UMA_HISTOGRAM_COUNTS_100(prefix + GetHistogramName(READ_DOM), @@ -292,7 +313,7 @@ void UmaPolicy::TabChangedAt(content::WebContents* contents, // Is this an existing tab whose URL has changed. if (tab_it != tab_list_.end()) { - CleanupClosedPage(tab_it->second); + CleanupClosedPage(tab_it->second, contents); tab_list_.erase(tab_id); } @@ -319,21 +340,22 @@ void UmaPolicy::TabClosingAt(TabStripModel* tab_strip_model, if (tab_it != tab_list_.end()) tab_list_.erase(tab_id); - CleanupClosedPage(url); + CleanupClosedPage(url, contents); } void UmaPolicy::SetupOpenedPage(const std::string& url) { url_status_[url][kNumberOfTabs]++; } -void UmaPolicy::CleanupClosedPage(const std::string& url) { - SiteMap::iterator old_site_lookup = url_status_.find(url); +void UmaPolicy::CleanupClosedPage(const std::string& cleaned_url, + content::WebContents* web_contents) { + SiteMap::iterator old_site_lookup = url_status_.find(cleaned_url); if (old_site_lookup == url_status_.end()) return; old_site_lookup->second[kNumberOfTabs]--; if (old_site_lookup->second[kNumberOfTabs] == 0) { - HistogramOnClose(url); - url_status_.erase(url); + HistogramOnClose(cleaned_url, web_contents); + url_status_.erase(cleaned_url); } } diff --git a/chrome/browser/extensions/activity_log/uma_policy.h b/chrome/browser/extensions/activity_log/uma_policy.h index e5ea34b..ef4894f 100644 --- a/chrome/browser/extensions/activity_log/uma_policy.h +++ b/chrome/browser/extensions/activity_log/uma_policy.h @@ -96,10 +96,12 @@ class UmaPolicy : public ActivityLogPolicy, void SetupOpenedPage(const std::string& url); // When a page is closing, remove it from the SiteMap url_status_. - void CleanupClosedPage(const std::string& url); + void CleanupClosedPage(const std::string& cleaned_url, + content::WebContents* web_contents); // When a page is closing, save statistics about the page to histograms. - void HistogramOnClose(const std::string& url); + void HistogramOnClose(const std::string& cleaned_url, + content::WebContents* web_contents); // Standardizes the way URLs are treated. static std::string CleanURL(const GURL& gurl); diff --git a/chrome/browser/extensions/activity_log/uma_policy_unittest.cc b/chrome/browser/extensions/activity_log/uma_policy_unittest.cc index 019feeb..86f57f8 100644 --- a/chrome/browser/extensions/activity_log/uma_policy_unittest.cc +++ b/chrome/browser/extensions/activity_log/uma_policy_unittest.cc @@ -89,8 +89,8 @@ TEST_F(UmaPolicyTest, MatchActionToStatusTest) { TEST_F(UmaPolicyTest, SiteUrlTest) { UmaPolicy* policy = new UmaPolicy(profile_.get()); - const std::string site0 = "http://www.zzz.com"; - const std::string site1 = "http://www.foo.com"; + const std::string site0 = "http://www.zzz.com/"; + const std::string site1 = "http://www.foo.com/"; const std::string site2 = "http://www.google.com#a"; const std::string site3 = "http://www.google.com#bb"; @@ -109,10 +109,10 @@ TEST_F(UmaPolicyTest, SiteUrlTest) { ASSERT_EQ(3, policy->url_status()[site3][UmaPolicy::kNumberOfTabs]); // Remove some sites. - policy->CleanupClosedPage(site0); - policy->CleanupClosedPage(site2); - policy->CleanupClosedPage(site2); - policy->CleanupClosedPage(site3); + policy->CleanupClosedPage(site0, NULL); + policy->CleanupClosedPage(site2, NULL); + policy->CleanupClosedPage(site2, NULL); + policy->CleanupClosedPage(site3, NULL); // Check that the removal worked. ASSERT_EQ(2u, policy->url_status().size()); diff --git a/chrome/browser/extensions/api/extension_action/extension_action_api.cc b/chrome/browser/extensions/api/extension_action/extension_action_api.cc index 3b552b3..70e6bbe 100644 --- a/chrome/browser/extensions/api/extension_action/extension_action_api.cc +++ b/chrome/browser/extensions/api/extension_action/extension_action_api.cc @@ -445,7 +445,7 @@ void ExtensionActionStorageManager::OnExtensionLoaded( AsWeakPtr(), extension->id())); } -}; +} void ExtensionActionStorageManager::Observe( int type, @@ -632,8 +632,7 @@ void ExtensionActionFunction::NotifyBrowserActionChange() { } void ExtensionActionFunction::NotifyLocationBarChange() { - TabHelper::FromWebContents(contents_)-> - location_bar_controller()->NotifyChange(); + LocationBarController::NotifyChange(contents_); } void ExtensionActionFunction::NotifySystemIndicatorChange() { @@ -909,8 +908,7 @@ bool PageActionsFunction::SetPageActionEnabled(bool enable) { // Set visibility and broadcast notifications that the UI should be updated. page_action->SetIsVisible(tab_id, enable); page_action->SetTitle(tab_id, title); - extensions::TabHelper::FromWebContents(contents)-> - location_bar_controller()->NotifyChange(); + extensions::LocationBarController::NotifyChange(contents); return true; } diff --git a/chrome/browser/extensions/location_bar_controller.cc b/chrome/browser/extensions/location_bar_controller.cc new file mode 100644 index 0000000..ee87a6b --- /dev/null +++ b/chrome/browser/extensions/location_bar_controller.cc @@ -0,0 +1,88 @@ +// Copyright 2014 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/extensions/location_bar_controller.h" + +#include "base/logging.h" +#include "chrome/browser/extensions/active_script_controller.h" +#include "chrome/browser/extensions/extension_action.h" +#include "chrome/browser/extensions/page_action_controller.h" +#include "chrome/common/extensions/api/extension_action/action_info.h" +#include "content/public/browser/invalidate_type.h" +#include "content/public/browser/navigation_details.h" +#include "content/public/browser/web_contents.h" +#include "extensions/browser/extension_registry.h" + +namespace extensions { + +LocationBarController::LocationBarController( + content::WebContents* web_contents) + : WebContentsObserver(web_contents), + web_contents_(web_contents), + active_script_controller_(new ActiveScriptController(web_contents_)), + page_action_controller_(new PageActionController(web_contents_)) { +} + +LocationBarController::~LocationBarController() { +} + +std::vector<ExtensionAction*> LocationBarController::GetCurrentActions() { + const ExtensionSet& extensions = + ExtensionRegistry::Get(web_contents_->GetBrowserContext()) + ->enabled_extensions(); + std::vector<ExtensionAction*> current_actions; + for (ExtensionSet::const_iterator iter = extensions.begin(); + iter != extensions.end(); + ++iter) { + // Right now, we can consolidate these actions because we only want to show + // one action per extension. If clicking on an active script action ever + // has a response, then we will need to split the actions. + ExtensionAction* action = + page_action_controller_->GetActionForExtension(*iter); + if (!action) + action = active_script_controller_->GetActionForExtension(*iter); + if (action) + current_actions.push_back(action); + } + + return current_actions; +} + +LocationBarController::Action LocationBarController::OnClicked( + const ExtensionAction* action) { + const Extension* extension = + ExtensionRegistry::Get(web_contents_->GetBrowserContext()) + ->enabled_extensions().GetByID(action->extension_id()); + CHECK(extension) << action->extension_id(); + + Action page_action = + page_action_controller_->GetActionForExtension(extension) ? + page_action_controller_->OnClicked(extension) : + ACTION_NONE; + Action active_script_action = + active_script_controller_->GetActionForExtension(extension) ? + active_script_controller_->OnClicked(extension) : + ACTION_NONE; + + // PageAction response takes priority. + return page_action != ACTION_NONE ? page_action : active_script_action; +} + +// static +void LocationBarController::NotifyChange(content::WebContents* web_contents) { + web_contents->NotifyNavigationStateChanged( + content::INVALIDATE_TYPE_PAGE_ACTIONS); +} + +void LocationBarController::DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) { + if (details.is_in_page) + return; + + page_action_controller_->OnNavigated(); + active_script_controller_->OnNavigated(); +} + +} // namespace extensions diff --git a/chrome/browser/extensions/location_bar_controller.h b/chrome/browser/extensions/location_bar_controller.h index ee99c44..a8bf3b4 100644 --- a/chrome/browser/extensions/location_bar_controller.h +++ b/chrome/browser/extensions/location_bar_controller.h @@ -5,40 +5,87 @@ #ifndef CHROME_BROWSER_EXTENSIONS_LOCATION_BAR_CONTROLLER_H_ #define CHROME_BROWSER_EXTENSIONS_LOCATION_BAR_CONTROLLER_H_ -#include <set> -#include <string> #include <vector> +#include "base/macros.h" +#include "base/memory/scoped_ptr.h" +#include "content/public/browser/web_contents_observer.h" + +namespace content { +class WebContents; +} + class ExtensionAction; namespace extensions { +class ActiveScriptController; +class Extension; +class PageActionController; + // Interface for a class that controls the the extension icons that show up in // the location bar. Depending on switches, these icons can have differing // behavior. -class LocationBarController { +class LocationBarController : public content::WebContentsObserver { public: - // The reaction that the UI should take after executing |OnClicked|. + // The action that the UI should take after executing |OnClicked|. enum Action { ACTION_NONE, ACTION_SHOW_POPUP, ACTION_SHOW_CONTEXT_MENU, }; - virtual ~LocationBarController() {} + class ActionProvider { + public: + // Returns the action for the given extension, or NULL if there isn't one. + virtual ExtensionAction* GetActionForExtension( + const Extension* extension) = 0; + + // Handles a click on an extension action. + virtual LocationBarController::Action OnClicked( + const Extension* extension) = 0; + + // A notification that the WebContents has navigated in the main frame (and + // not in page), so any state relating to the current page should likely be + // reset. + virtual void OnNavigated() = 0; + }; + + explicit LocationBarController(content::WebContents* web_contents); + virtual ~LocationBarController(); + + // Returns the actions which should be displayed in the location bar. + std::vector<ExtensionAction*> GetCurrentActions(); + + // Notifies this that an ExtensionAction has been clicked, and returns the + // action which should be taken in response (if any). + Action OnClicked(const ExtensionAction* action); + + // Notifies the window that the actions have changed. + static void NotifyChange(content::WebContents* web_contents); + + ActiveScriptController* active_script_controller() { + return active_script_controller_.get(); + } + + private: + // content::WebContentsObserver implementation. + virtual void DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) OVERRIDE; - // Gets the action data for all extensions. - virtual std::vector<ExtensionAction*> GetCurrentActions() const = 0; + // The associated WebContents. + content::WebContents* web_contents_; - // Notifies this that the badge for an extension has been clicked with some - // mouse button (1 for left, 2 for middle, and 3 for right click), and - // returns the action that should be taken in response (if any). - // TODO(kalman): make mouse_button an enum. - virtual Action OnClicked(const std::string& extension_id, - int mouse_button) = 0; + // The controllers for different sources of actions in the location bar. + // Currently, this is only page actions and active script actions, so we + // explicitly own and create both. If there are ever more, it will be worth + // considering making this class own a list of LocationBarControllerProviders + // instead. + scoped_ptr<ActiveScriptController> active_script_controller_; + scoped_ptr<PageActionController> page_action_controller_; - // Notifies clients that the icons have changed. - virtual void NotifyChange() = 0; + DISALLOW_COPY_AND_ASSIGN(LocationBarController); }; } // namespace extensions diff --git a/chrome/browser/extensions/page_action_controller.cc b/chrome/browser/extensions/page_action_controller.cc index 6d9bfad..2d80da0 100644 --- a/chrome/browser/extensions/page_action_controller.cc +++ b/chrome/browser/extensions/page_action_controller.cc @@ -9,14 +9,11 @@ #include "base/lazy_instance.h" #include "base/metrics/histogram.h" #include "chrome/browser/extensions/api/extension_action/extension_action_api.h" -#include "chrome/browser/extensions/component_loader.h" #include "chrome/browser/extensions/extension_action.h" #include "chrome/browser/extensions/extension_action_manager.h" -#include "chrome/browser/extensions/extension_tab_util.h" #include "chrome/browser/extensions/tab_helper.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sessions/session_id.h" -#include "content/public/browser/invalidate_type.h" #include "content/public/browser/navigation_details.h" #include "content/public/browser/web_contents.h" #include "extensions/browser/extension_registry.h" @@ -29,111 +26,71 @@ base::LazyInstance<std::set<Profile*> > g_reported_for_profiles = LAZY_INSTANCE_INITIALIZER; PageActionController::PageActionController(content::WebContents* web_contents) - : content::WebContentsObserver(web_contents) {} - -PageActionController::~PageActionController() {} - -std::vector<ExtensionAction*> PageActionController::GetCurrentActions() const { - ExtensionRegistry* registry = GetExtensionRegistry(); - if (!registry) - return std::vector<ExtensionAction*>(); - - // Accumulate the list of all page actions to display. - std::vector<ExtensionAction*> current_actions; - - ExtensionActionManager* extension_action_manager = - ExtensionActionManager::Get(GetProfile()); - - const ExtensionSet& enabled_set = registry->enabled_extensions(); - for (ExtensionSet::const_iterator i = enabled_set.begin(); - i != enabled_set.end(); - ++i) { - ExtensionAction* action = - extension_action_manager->GetPageAction(*i->get()); - if (action) - current_actions.push_back(action); - } + : web_contents_(web_contents) { +} - if (!g_reported_for_profiles.Get().count(GetProfile())) { - UMA_HISTOGRAM_COUNTS_100("PageActionController.ExtensionsWithPageActions", - current_actions.size()); - g_reported_for_profiles.Get().insert(GetProfile()); - } +PageActionController::~PageActionController() { +} - return current_actions; +ExtensionAction* PageActionController::GetActionForExtension( + const Extension* extension) { + return ExtensionActionManager::Get(GetProfile())->GetPageAction(*extension); } LocationBarController::Action PageActionController::OnClicked( - const std::string& extension_id, int mouse_button) { - ExtensionRegistry* registry = GetExtensionRegistry(); - if (!registry) - return ACTION_NONE; - - const Extension* extension = - registry->enabled_extensions().GetByID(extension_id); - CHECK(extension); + const Extension* extension) { ExtensionAction* page_action = ExtensionActionManager::Get(GetProfile())->GetPageAction(*extension); CHECK(page_action); - int tab_id = ExtensionTabUtil::GetTabId(web_contents()); - extensions::TabHelper::FromWebContents(web_contents())-> + int tab_id = SessionID::IdForTab(web_contents_); + TabHelper::FromWebContents(web_contents_)-> active_tab_permission_granter()->GrantIfRequested(extension); - switch (mouse_button) { - case 1: // left - case 2: // middle - if (page_action->HasPopup(tab_id)) - return ACTION_SHOW_POPUP; + if (page_action->HasPopup(tab_id)) + return LocationBarController::ACTION_SHOW_POPUP; - ExtensionActionAPI::PageActionExecuted( - GetProfile(), *page_action, tab_id, - web_contents()->GetURL().spec(), mouse_button); - return ACTION_NONE; + ExtensionActionAPI::PageActionExecuted( + web_contents_->GetBrowserContext(), + *page_action, + tab_id, + web_contents_->GetLastCommittedURL().spec(), + 1 /* Button indication. We only ever pass left-click. */); - case 3: // right - return extension->ShowConfigureContextMenus() ? - ACTION_SHOW_CONTEXT_MENU : ACTION_NONE; - } - - return ACTION_NONE; -} - -void PageActionController::NotifyChange() { - web_contents()->NotifyNavigationStateChanged( - content::INVALIDATE_TYPE_PAGE_ACTIONS); + return LocationBarController::ACTION_NONE; } -void PageActionController::DidNavigateMainFrame( - const content::LoadCommittedDetails& details, - const content::FrameNavigateParams& params) { - if (details.is_in_page) - return; - - const std::vector<ExtensionAction*> current_actions = GetCurrentActions(); - - if (current_actions.empty()) - return; - - for (size_t i = 0; i < current_actions.size(); ++i) { - current_actions[i]->ClearAllValuesForTab( - SessionID::IdForTab(web_contents())); +void PageActionController::OnNavigated() { + const ExtensionSet& extensions = + ExtensionRegistry::Get(web_contents_->GetBrowserContext()) + ->enabled_extensions(); + int tab_id = SessionID::IdForTab(web_contents_); + size_t num_current_actions = 0u; + for (ExtensionSet::const_iterator iter = extensions.begin(); + iter != extensions.end(); + ++iter) { + ExtensionAction* action = GetActionForExtension(*iter); + if (action) { + action->ClearAllValuesForTab(tab_id); + ++num_current_actions; + } } - NotifyChange(); -} - -Profile* PageActionController::GetProfile() const { - content::WebContents* web_contents = this->web_contents(); - if (web_contents) - return Profile::FromBrowserContext(web_contents->GetBrowserContext()); + Profile* profile = GetProfile(); + // Report the number of page actions for this profile, if we haven't already. + // TODO(rdevlin.cronin): This is wrong. Instead, it should record the number + // of page actions displayed per page. + if (!g_reported_for_profiles.Get().count(profile)) { + UMA_HISTOGRAM_COUNTS_100("PageActionController.ExtensionsWithPageActions", + num_current_actions); + g_reported_for_profiles.Get().insert(profile); + } - return NULL; + LocationBarController::NotifyChange(web_contents_); } -ExtensionRegistry* PageActionController::GetExtensionRegistry() const { - Profile* profile = this->GetProfile(); - return profile ? ExtensionRegistry::Get(profile) : NULL; +Profile* PageActionController::GetProfile() { + return Profile::FromBrowserContext(web_contents_->GetBrowserContext()); } } // namespace extensions diff --git a/chrome/browser/extensions/page_action_controller.h b/chrome/browser/extensions/page_action_controller.h index 88320d3..af79037 100644 --- a/chrome/browser/extensions/page_action_controller.h +++ b/chrome/browser/extensions/page_action_controller.h @@ -7,40 +7,37 @@ #include <string> -#include "base/observer_list.h" #include "chrome/browser/extensions/location_bar_controller.h" -#include "content/public/browser/web_contents_observer.h" class Profile; namespace extensions { class ExtensionRegistry; -// A LocationBarController which populates the location bar with icons based -// on the page_action extension API. -class PageActionController : public LocationBarController, - public content::WebContentsObserver { +// A LocationBarControllerProvider which populates the location bar with icons +// based on the page_action extension API. +// TODO(rdevlin.cronin): This isn't really a controller. +class PageActionController : public LocationBarController::ActionProvider { public: explicit PageActionController(content::WebContents* web_contents); virtual ~PageActionController(); - // LocationBarController implementation. - virtual std::vector<ExtensionAction*> GetCurrentActions() const OVERRIDE; - virtual Action OnClicked(const std::string& extension_id, - int mouse_button) OVERRIDE; - virtual void NotifyChange() OVERRIDE; - - // content::WebContentsObserver implementation. - virtual void DidNavigateMainFrame( - const content::LoadCommittedDetails& details, - const content::FrameNavigateParams& params) OVERRIDE; + // LocationBarController::Provider implementation. + virtual ExtensionAction* GetActionForExtension(const Extension* extension) + OVERRIDE; + virtual LocationBarController::Action OnClicked( + const Extension* extension) OVERRIDE; + virtual void OnNavigated() OVERRIDE; private: - // Gets the Profile for the web contents. - Profile* GetProfile() const; + // Returns the associated Profile. + Profile* GetProfile(); + + // The current page actions for the associated WebContents. + std::vector<ExtensionAction*> current_actions_; - // Gets the ExtensionRegistry for the web contents. - ExtensionRegistry* GetExtensionRegistry() const; + // The associated WebContents. + content::WebContents* web_contents_; DISALLOW_COPY_AND_ASSIGN(PageActionController); }; diff --git a/chrome/browser/extensions/script_executor.cc b/chrome/browser/extensions/script_executor.cc index 6ff22d8..9f57f1b 100644 --- a/chrome/browser/extensions/script_executor.cc +++ b/chrome/browser/extensions/script_executor.cc @@ -7,6 +7,11 @@ #include "base/callback.h" #include "base/logging.h" #include "base/pickle.h" +#include "chrome/browser/extensions/active_script_controller.h" +#include "chrome/browser/extensions/location_bar_controller.h" +#include "chrome/browser/extensions/tab_helper.h" +#include "content/public/browser/navigation_controller.h" +#include "content/public/browser/navigation_entry.h" #include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" @@ -120,6 +125,17 @@ void ScriptExecutor::ExecuteScript(const std::string& extension_id, bool user_gesture, ScriptExecutor::ResultType result_type, const ExecuteScriptCallback& callback) { + TabHelper* tab_helper = TabHelper::FromWebContents(web_contents_); + if (tab_helper) { + LocationBarController* location_bar_controller = + TabHelper::FromWebContents(web_contents_)->location_bar_controller(); + // TODO(rdevlin.cronin): Now, this is just a notification. Soon, it should + // block until the user gives the OK to execute. + location_bar_controller->active_script_controller()->NotifyScriptExecuting( + extension_id, + web_contents_->GetController().GetVisibleEntry()->GetPageID()); + } + ExtensionMsg_ExecuteCode_Params params; params.request_id = next_request_id_++; params.extension_id = extension_id; diff --git a/chrome/browser/extensions/tab_helper.cc b/chrome/browser/extensions/tab_helper.cc index fda6175..3689523 100644 --- a/chrome/browser/extensions/tab_helper.cc +++ b/chrome/browser/extensions/tab_helper.cc @@ -9,6 +9,7 @@ #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/chrome_notification_types.h" +#include "chrome/browser/extensions/active_script_controller.h" #include "chrome/browser/extensions/activity_log/activity_log.h" #include "chrome/browser/extensions/api/declarative/rules_registry_service.h" #include "chrome/browser/extensions/api/declarative_content/content_rules_registry.h" @@ -20,7 +21,7 @@ #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_tab_util.h" #include "chrome/browser/extensions/image_loader.h" -#include "chrome/browser/extensions/page_action_controller.h" +#include "chrome/browser/extensions/location_bar_controller.h" #include "chrome/browser/extensions/script_executor.h" #include "chrome/browser/extensions/webstore_inline_installer.h" #include "chrome/browser/extensions/webstore_inline_installer_factory.h" @@ -103,7 +104,7 @@ TabHelper::TabHelper(content::WebContents* web_contents) pending_web_app_action_(NONE), script_executor_(new ScriptExecutor(web_contents, &script_execution_observers_)), - location_bar_controller_(new PageActionController(web_contents)), + location_bar_controller_(new LocationBarController(web_contents)), image_loader_ptr_factory_(this), webstore_inline_installer_factory_(new WebstoreInlineInstallerFactory()) { // The ActiveTabPermissionManager requires a session ID; ensure this diff --git a/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm b/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm index 805ddc1..4b1d6ce 100644 --- a/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm +++ b/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm @@ -104,8 +104,7 @@ bool PageActionDecoration::ActivatePageAction(NSRect frame) { extensions::TabHelper::FromWebContents(web_contents)-> location_bar_controller(); - // 1 is left click. - switch (controller->OnClicked(page_action_->extension_id(), 1)) { + switch (controller->OnClicked(page_action_)) { case LocationBarController::ACTION_NONE: break; diff --git a/chrome/browser/ui/views/location_bar/page_action_image_view.cc b/chrome/browser/ui/views/location_bar/page_action_image_view.cc index cfb05e3..793866d 100644 --- a/chrome/browser/ui/views/location_bar/page_action_image_view.cc +++ b/chrome/browser/ui/views/location_bar/page_action_image_view.cc @@ -93,7 +93,7 @@ void PageActionImageView::ExecuteAction( LocationBarController* controller = extensions_tab_helper->location_bar_controller(); - switch (controller->OnClicked(page_action_->extension_id(), 1)) { + switch (controller->OnClicked(page_action_)) { case LocationBarController::ACTION_NONE: break; diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi index 718906c..1ebbefc 100644 --- a/chrome/chrome_browser_extensions.gypi +++ b/chrome/chrome_browser_extensions.gypi @@ -96,6 +96,8 @@ 'browser/apps/shortcut_manager.h', 'browser/apps/shortcut_manager_factory.cc', 'browser/apps/shortcut_manager_factory.h', + 'browser/extensions/active_script_controller.cc', + 'browser/extensions/active_script_controller.h', 'browser/extensions/active_tab_permission_granter.cc', 'browser/extensions/active_tab_permission_granter.h', 'browser/extensions/activity_log/activity_action_constants.cc', @@ -826,6 +828,7 @@ 'browser/extensions/install_verifier.h', 'browser/extensions/launch_util.cc', 'browser/extensions/launch_util.h', + 'browser/extensions/location_bar_controller.cc', 'browser/extensions/location_bar_controller.h', 'browser/extensions/menu_manager.cc', 'browser/extensions/menu_manager.h', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index f3ffa47..a7e0aa7 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -999,6 +999,7 @@ 'browser/download/download_started_animation_browsertest.cc', 'browser/download/save_page_browsertest.cc', 'browser/errorpage_browsertest.cc', + 'browser/extensions/active_script_controller_browsertest.cc', 'browser/extensions/active_tab_apitest.cc', 'browser/extensions/activity_log/activity_log_browsertest.cc', 'browser/extensions/activity_log/ad_injection_browsertest.cc', diff --git a/chrome/test/data/extensions/active_script/content_scripts_all_hosts/content_script.js b/chrome/test/data/extensions/active_script/content_scripts_all_hosts/content_script.js new file mode 100644 index 0000000..cb1cdac --- /dev/null +++ b/chrome/test/data/extensions/active_script/content_scripts_all_hosts/content_script.js @@ -0,0 +1,5 @@ +// Copyright 2014 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. + +chrome.test.sendMessage('content_scripts_all_hosts'); diff --git a/chrome/test/data/extensions/active_script/content_scripts_all_hosts/manifest.json b/chrome/test/data/extensions/active_script/content_scripts_all_hosts/manifest.json new file mode 100644 index 0000000..b325375 --- /dev/null +++ b/chrome/test/data/extensions/active_script/content_scripts_all_hosts/manifest.json @@ -0,0 +1,12 @@ +{ + "name": "Active Script Test: Content Scripts All Hosts", + "description": "An extension with all hosts that has a content script.", + "version": "2.0", + "content_scripts": [ + { + "matches": ["*://*/*"], + "js": ["content_script.js"] + } + ], + "manifest_version": 2 +} diff --git a/chrome/test/data/extensions/active_script/content_scripts_explicit_hosts/content_script.js b/chrome/test/data/extensions/active_script/content_scripts_explicit_hosts/content_script.js new file mode 100644 index 0000000..cbbbc3f --- /dev/null +++ b/chrome/test/data/extensions/active_script/content_scripts_explicit_hosts/content_script.js @@ -0,0 +1,5 @@ +// Copyright 2014 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. + +chrome.test.sendMessage('content_scripts_explicit_hosts'); diff --git a/chrome/test/data/extensions/active_script/content_scripts_explicit_hosts/manifest.json b/chrome/test/data/extensions/active_script/content_scripts_explicit_hosts/manifest.json new file mode 100644 index 0000000..7308e27 --- /dev/null +++ b/chrome/test/data/extensions/active_script/content_scripts_explicit_hosts/manifest.json @@ -0,0 +1,12 @@ +{ + "name": "Active Script Test: Content Scripts Explicit Hosts", + "description": "An extension with explicit hosts that has a content script.", + "version": "2.0", + "content_scripts": [ + { + "matches": ["http://127.0.0.1/*"], + "js": ["content_script.js"] + } + ], + "manifest_version": 2 +} diff --git a/chrome/test/data/extensions/active_script/inject_scripts_all_hosts/background.js b/chrome/test/data/extensions/active_script/inject_scripts_all_hosts/background.js new file mode 100644 index 0000000..309f4d8 --- /dev/null +++ b/chrome/test/data/extensions/active_script/inject_scripts_all_hosts/background.js @@ -0,0 +1,9 @@ +// Copyright 2014 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. + +chrome.tabs.onUpdated.addListener(function(tabId) { + chrome.tabs.executeScript( + tabId, + { code: 'chrome.test.sendMessage("inject_scripts_all_hosts");' }); +}); diff --git a/chrome/test/data/extensions/active_script/inject_scripts_all_hosts/manifest.json b/chrome/test/data/extensions/active_script/inject_scripts_all_hosts/manifest.json new file mode 100644 index 0000000..95cd0c0 --- /dev/null +++ b/chrome/test/data/extensions/active_script/inject_scripts_all_hosts/manifest.json @@ -0,0 +1,12 @@ +{ + "name": "Active Script Test: Inject Scripts All Hosts", + "description": "An extension with all hosts that injects scripts.", + "version": "1.5", + "background": { + "scripts": ["background.js"] + }, + "permissions": [ + "tabs", "*://*/*" + ], + "manifest_version": 2 +} diff --git a/extensions/common/extension_messages.h b/extensions/common/extension_messages.h index fa85f49..76ce134 100644 --- a/extensions/common/extension_messages.h +++ b/extensions/common/extension_messages.h @@ -569,6 +569,10 @@ IPC_MESSAGE_ROUTED3(ExtensionHostMsg_ContentScriptsExecuting, int32 /* page_id of the _topmost_ frame */, GURL /* url of the _topmost_ frame */) +IPC_MESSAGE_ROUTED2(ExtensionHostMsg_NotifyExtensionScriptExecution, + std::string /* extension id */, + int /* page id */) + // Sent by the renderer when a web page is checking if its app is installed. IPC_MESSAGE_ROUTED3(ExtensionHostMsg_GetAppInstallState, GURL /* requestor_url */, diff --git a/extensions/common/feature_switch.cc b/extensions/common/feature_switch.cc index 91ebdda..121dceb 100644 --- a/extensions/common/feature_switch.cc +++ b/extensions/common/feature_switch.cc @@ -42,7 +42,9 @@ class CommonSwitches { FeatureSwitch::DEFAULT_DISABLED), enable_override_bookmarks_ui( switches::kEnableOverrideBookmarksUI, - FeatureSwitch::DEFAULT_DISABLED) {} + FeatureSwitch::DEFAULT_DISABLED), + scripts_require_action(switches::kScriptsRequireAction, + FeatureSwitch::DEFAULT_DISABLED) {} // Enables extensions to be easily installed from sites other than the web // store. @@ -57,6 +59,7 @@ class CommonSwitches { FeatureSwitch error_console; FeatureSwitch enable_override_bookmarks_ui; + FeatureSwitch scripts_require_action; }; base::LazyInstance<CommonSwitches> g_common_switches = @@ -83,6 +86,10 @@ FeatureSwitch* FeatureSwitch::enable_override_bookmarks_ui() { return &g_common_switches.Get().enable_override_bookmarks_ui; } +FeatureSwitch* FeatureSwitch::scripts_require_action() { + return &g_common_switches.Get().scripts_require_action; +} + FeatureSwitch::ScopedOverride::ScopedOverride(FeatureSwitch* feature, bool override_value) : feature_(feature), diff --git a/extensions/common/feature_switch.h b/extensions/common/feature_switch.h index 90db74a..4b9cdd9 100644 --- a/extensions/common/feature_switch.h +++ b/extensions/common/feature_switch.h @@ -25,6 +25,7 @@ class FeatureSwitch { static FeatureSwitch* prompt_for_external_extensions(); static FeatureSwitch* error_console(); static FeatureSwitch* enable_override_bookmarks_ui(); + static FeatureSwitch* scripts_require_action(); enum DefaultValue { DEFAULT_ENABLED, diff --git a/extensions/common/permissions/permissions_data.cc b/extensions/common/permissions/permissions_data.cc index 95dce8b..809865d 100644 --- a/extensions/common/permissions/permissions_data.cc +++ b/extensions/common/permissions/permissions_data.cc @@ -560,6 +560,14 @@ bool PermissionsData::CanCaptureVisiblePage(const Extension* extension, return false; } +// static +bool PermissionsData::RequiresActionForScriptExecution( + const Extension* extension) { + // For now, the user should be notified when an extension with all hosts + // permission tries to execute a script on a page. + return HasEffectiveAccessToAllHosts(extension); +} + bool PermissionsData::ParsePermissions(Extension* extension, base::string16* error) { initial_required_permissions_.reset(new InitialPermissions); diff --git a/extensions/common/permissions/permissions_data.h b/extensions/common/permissions/permissions_data.h index ee1715d..f4f8bce 100644 --- a/extensions/common/permissions/permissions_data.h +++ b/extensions/common/permissions/permissions_data.h @@ -177,6 +177,10 @@ class PermissionsData { int tab_id, std::string* error); + // Returns true if the user should be alerted that the |extension| is running + // a script. + static bool RequiresActionForScriptExecution(const Extension* extension); + // Parse the permissions of a given extension in the initialization process. bool ParsePermissions(Extension* extension, base::string16* error); diff --git a/extensions/common/switches.cc b/extensions/common/switches.cc index 57ea960..306f85a 100644 --- a/extensions/common/switches.cc +++ b/extensions/common/switches.cc @@ -59,6 +59,9 @@ const char kForceDevModeHighlighting[] = "force-dev-mode-highlighting"; // Enables setting global commands through the Extensions Commands API. const char kGlobalCommands[] = "global-commands"; +// Notify the user and require consent for extensions running scripts. +const char kScriptsRequireAction[] = "scripts-require-action"; + // Makes component extensions appear in chrome://settings/extensions. const char kShowComponentExtensionOptions[] = "show-component-extension-options"; diff --git a/extensions/common/switches.h b/extensions/common/switches.h index b758403..f27d782 100644 --- a/extensions/common/switches.h +++ b/extensions/common/switches.h @@ -26,6 +26,7 @@ extern const char kExtensionProcess[]; extern const char kExtensionsOnChromeURLs[]; extern const char kForceDevModeHighlighting[]; extern const char kGlobalCommands[]; +extern const char kScriptsRequireAction[]; extern const char kShowComponentExtensionOptions[]; extern const char kWhitelistedExtensionID[]; diff --git a/extensions/renderer/user_script_slave.cc b/extensions/renderer/user_script_slave.cc index 64cd3f6..f05d214 100644 --- a/extensions/renderer/user_script_slave.cc +++ b/extensions/renderer/user_script_slave.cc @@ -194,6 +194,10 @@ void UserScriptSlave::InjectScripts(WebFrame* frame, ExecutingScriptsMap extensions_executing_scripts; + blink::WebFrame* top_frame = frame->top(); + content::RenderView* top_render_view = + content::RenderView::FromWebView(top_frame->view()); + for (size_t i = 0; i < scripts_.size(); ++i) { std::vector<WebScriptSource> sources; UserScript* script = scripts_[i]; @@ -214,7 +218,7 @@ void UserScriptSlave::InjectScripts(WebFrame* frame, const int kNoProcessId = -1; if (!PermissionsData::CanExecuteScriptOnPage(extension, data_source_url, - frame->top()->document().url(), + top_frame->document().url(), kNoTabId, script, kNoProcessId, @@ -234,6 +238,15 @@ void UserScriptSlave::InjectScripts(WebFrame* frame, } if (script->run_location() == location) { + // TODO(rdevlin.cronin): Right now, this is just a notification, but soon + // we should block without user consent. + if (PermissionsData::RequiresActionForScriptExecution(extension)) { + top_render_view->Send( + new ExtensionHostMsg_NotifyExtensionScriptExecution( + top_render_view->GetRoutingID(), + extension->id(), + top_render_view->GetPageId())); + } num_scripts += script->js_scripts().size(); for (size_t j = 0; j < script->js_scripts().size(); ++j) { UserScript::File& file = script->js_scripts()[j]; @@ -282,13 +295,10 @@ void UserScriptSlave::InjectScripts(WebFrame* frame, // Notify the browser if any extensions are now executing scripts. if (!extensions_executing_scripts.empty()) { - blink::WebFrame* top_frame = frame->top(); - content::RenderView* render_view = - content::RenderView::FromWebView(top_frame->view()); - render_view->Send(new ExtensionHostMsg_ContentScriptsExecuting( - render_view->GetRoutingID(), + top_render_view->Send(new ExtensionHostMsg_ContentScriptsExecuting( + top_render_view->GetRoutingID(), extensions_executing_scripts, - render_view->GetPageId(), + top_render_view->GetPageId(), ScriptContext::GetDataSourceURLForFrame(top_frame))); } diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index eddcdb3..24c1244 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -5725,6 +5725,40 @@ Therefore, the affected-histogram name has to have at least one dot in it. </summary> </histogram> +<histogram name="Extensions.ActiveScriptController.PreventableAdInjectors" + units="Extension Count"> + <owner>kalman@chromium.org</owner> + <owner>rdevlin.cronin@chromium.org</owner> + <summary> + The number of extensions per page that injected an ad and could have been + stopped if the user had declined script injection. This is related to the + ActivityLog metrics, ExtensionActivity.Ad*. Recorded upon page close or + navigation. + </summary> +</histogram> + +<histogram name="Extensions.ActiveScriptController.ShownActiveScriptsOnPage" + units="Number of Actions"> + <owner>kalman@chromium.org</owner> + <owner>rdevlin.cronin@chromium.org</owner> + <summary> + The number of extensions which would display an Active Script Running + indiciation to the user. Recorded at page close. + </summary> +</histogram> + +<histogram name="Extensions.ActiveScriptController.UnpreventableAdInjectors" + units="Extension Count"> + <owner>kalman@chromium.org</owner> + <owner>rdevlin.cronin@chromium.org</owner> + <summary> + The number of extensions per page that injected an ad and that could not + have been stopped through script injection permission. This is related to + the ActivityLog metrics, ExtensionActivity.Ad*. Recorded upon page close or + navigation. + </summary> +</histogram> + <histogram name="Extensions.AllocatePortIdPairOverflow"> <owner>Please list the metric's owners. Add more owner tags as needed.</owner> <summary> @@ -7053,6 +7087,14 @@ Therefore, the affected-histogram name has to have at least one dot in it. </summary> </histogram> +<histogram name="FileBrowser.VolumeType" enum="FileManagerVolumeType"> + <owner>kinaba@chromium.org</owner> + <summary> + Chrome OS File Browser: counts the number of times volumes are mounted for + each volume type. + </summary> +</histogram> + <histogram name="GCM.APICallUnregister"> <owner>jianli@chromium.org</owner> <summary>Number of times when gcm.unregister API is called.</summary> @@ -7067,14 +7109,6 @@ Therefore, the affected-histogram name has to have at least one dot in it. </summary> </histogram> -<histogram name="FileBrowser.VolumeType" enum="FileManagerVolumeType"> - <owner>kinaba@chromium.org</owner> - <summary> - Chrome OS File Browser: counts the number of times volumes are mounted for - each volume type. - </summary> -</histogram> - <histogram name="GCM.CheckinRequestStatus" enum="GCMCheckinRequestStatus"> <owner>juyik@chromium.org</owner> <summary>Status code of the outcome of a GCM checkin request.</summary> |