Chromium Code Reviews| Index: chrome/browser/extensions/active_script_controller.cc |
| diff --git a/chrome/browser/extensions/active_script_controller.cc b/chrome/browser/extensions/active_script_controller.cc |
| index 36209aff27729a13b0aa4b29b2aaf815cfed78b6..3653b0acaa9382f0f8d3fe10224a0a3b3536e19a 100644 |
| --- a/chrome/browser/extensions/active_script_controller.cc |
| +++ b/chrome/browser/extensions/active_script_controller.cc |
| @@ -4,6 +4,9 @@ |
| #include "chrome/browser/extensions/active_script_controller.h" |
| +#include "base/bind.h" |
| +#include "base/bind_helpers.h" |
| +#include "base/memory/scoped_ptr.h" |
| #include "base/metrics/histogram.h" |
| #include "base/stl_util.h" |
| #include "chrome/browser/extensions/extension_action.h" |
| @@ -12,6 +15,7 @@ |
| #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/render_view_host.h" |
| #include "content/public/browser/web_contents.h" |
| #include "extensions/browser/extension_registry.h" |
| #include "extensions/common/extension.h" |
| @@ -27,6 +31,7 @@ ActiveScriptController::ActiveScriptController( |
| content::WebContents* web_contents) |
| : content::WebContentsObserver(web_contents), |
| enabled_(FeatureSwitch::scripts_require_action()->IsEnabled()) { |
| + CHECK(web_contents); |
| } |
| ActiveScriptController::~ActiveScriptController() { |
| @@ -48,31 +53,40 @@ ActiveScriptController* ActiveScriptController::GetForWebContents( |
| return location_bar_controller->active_script_controller(); |
| } |
| -void ActiveScriptController::NotifyScriptExecuting( |
| - const std::string& extension_id, int page_id) { |
| +void ActiveScriptController::InjectScriptIfHasPermission( |
| + const std::string& extension_id, |
| + int page_id, |
| + const base::Closure& callback) { |
| content::NavigationEntry* visible_entry = |
| web_contents()->GetController().GetVisibleEntry(); |
| - if (!visible_entry || |
| - extensions_executing_scripts_.count(extension_id) || |
| - visible_entry->GetPageID() != page_id) { |
| + if (!visible_entry || visible_entry->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()); |
| - } |
| + if (!extension) |
| + return; |
| + |
| + AddOrProcessRequest(extension, callback); |
| } |
| 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(); |
| + ad_injectors, permitted_extensions_).size(); |
| + |
| + // We only log the permitted extensions metric if the feature is enabled, |
| + // because otherwise the data will be boring (100% allowed). |
| + if (enabled_) { |
| + UMA_HISTOGRAM_COUNTS_100( |
|
Devlin
2014/05/15 17:46:00
What did you want these to look like? (haven't ad
|
| + "Extensions.ActiveScriptController.PermittedExtensions", |
| + permitted_extensions_.size()); |
| + UMA_HISTOGRAM_COUNTS_100( |
| + "Extensions.ActiveScriptController.DeniedExtensions", |
| + pending_requests_.size()); |
| + } |
| UMA_HISTOGRAM_COUNTS_100( |
| "Extensions.ActiveScriptController.PreventableAdInjectors", |
| @@ -84,7 +98,7 @@ void ActiveScriptController::OnAdInjectionDetected( |
| ExtensionAction* ActiveScriptController::GetActionForExtension( |
| const Extension* extension) { |
| - if (!enabled_ || extensions_executing_scripts_.count(extension->id()) == 0) |
| + if (!enabled_ || pending_requests_.count(extension->id()) == 0) |
| return NULL; // No action for this extension. |
| ActiveScriptMap::iterator existing = |
| @@ -112,13 +126,73 @@ ExtensionAction* ActiveScriptController::GetActionForExtension( |
| LocationBarController::Action ActiveScriptController::OnClicked( |
| const Extension* extension) { |
| - DCHECK(extensions_executing_scripts_.count(extension->id()) > 0); |
| + DCHECK(extension); |
| + PendingRequestMap::iterator iter = |
| + pending_requests_.find(extension->id()); |
| + DCHECK(iter != pending_requests_.end()); |
| + |
| + // Run all pending injections for the given extension. |
| + PendingRequestList& list = iter->second; |
| + for (PendingRequestList::iterator request = list.begin(); |
| + request != list.end(); |
| + ++request) { |
| + request->Run(); |
| + } |
| + |
| + // Remove the extension's pending request entry, and indicate that it is |
| + // allowed to run on the current page. |
| + pending_requests_.erase(extension->id()); |
| + permitted_extensions_.insert(extension->id()); |
| + |
| + // Inform the location bar that the action is now gone. |
| + LocationBarController::NotifyChange(web_contents()); |
| + |
| return LocationBarController::ACTION_NONE; |
| } |
| void ActiveScriptController::OnNavigated() { |
| LogUMA(); |
| - extensions_executing_scripts_.clear(); |
| + permitted_extensions_.clear(); |
| + pending_requests_.clear(); |
| +} |
| + |
| +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; |
| + } |
| + |
| + InjectScriptIfHasPermission( |
| + extension_id, |
| + page_id, |
| + base::Closure(base::Bind(&base::DoNothing))); |
| +} |
| + |
| +void ActiveScriptController::AddOrProcessRequest( |
| + const Extension* extension, |
| + const base::Closure& request) { |
| + // If the feature is not enabled, we automatically allow all extensions to |
| + // run scripts. |
| + if (!enabled_) |
| + permitted_extensions_.insert(extension->id()); |
| + |
| + // If the extension does not require permissions, run it immediately. |
| + if (!PermissionsData::RequiresActionForScriptExecution(extension) || |
| + permitted_extensions_.count(extension->id())) { |
| + request.Run(); |
| + return; |
| + } |
| + |
| + PendingRequestList* list = &pending_requests_[extension->id()]; |
| + list->push_back(request); |
| + |
| + // If this was the first entry, notify the location bar that there's a new |
| + // icon. |
| + if (list->size() == 1u) |
| + LocationBarController::NotifyChange(web_contents()); |
| + |
| } |
| bool ActiveScriptController::OnMessageReceived(const IPC::Message& message) { |
| @@ -131,20 +205,10 @@ bool ActiveScriptController::OnMessageReceived(const IPC::Message& message) { |
| 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()); |
| + pending_requests_.size()); |
| } |
| } // namespace extensions |