Support "always allow" for runtime script execution

chrome/browser/extensions/active_script_controller.cc

 // 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/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/active_tab_permission_granter.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/location_bar_controller.h"
+#include "chrome/browser/extensions/permissions_updater.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/sessions/session_id.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/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/browser/extension_prefs.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/manifest.h"
+#include "extensions/common/manifest_handlers/permissions_parser.h"
+#include "extensions/common/permissions/permission_set.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "ipc/ipc_message_macros.h"
 
 namespace extensions {
 
 namespace {
 
 // Returns true if the extension should be regarded as a "permitted" extension
 // for the case of metrics. We need this because we only actually withhold
 // permissions if the switch is enabled, but want to record metrics in all
@@ skipping 54 matching lines @@
           ad_injectors, permitted_extensions_).size();
 
   UMA_HISTOGRAM_COUNTS_100(
       "Extensions.ActiveScriptController.PreventableAdInjectors",
       num_preventable_ad_injectors);
   UMA_HISTOGRAM_COUNTS_100(
       "Extensions.ActiveScriptController.UnpreventableAdInjectors",
       ad_injectors.size() - num_preventable_ad_injectors);
 }
 
+void ActiveScriptController::AlwaysRunOnVisibleHost(
+    const Extension* extension) {
+  GURL url = web_contents()->GetVisibleURL();
+  extensions::URLPatternSet new_explicit_hosts;
+  extensions::URLPatternSet new_scriptable_hosts;
+
+  scoped_refptr<const PermissionSet> withheld_permissions =
+      extension->permissions_data()->withheld_permissions();
+  if (withheld_permissions->explicit_hosts().MatchesURL(url)) {
+    new_explicit_hosts.AddOrigin(
+        extensions::UserScript::ValidUserScriptSchemes(), url.GetOrigin());

[not at google - send to devlin, 2014/08/12 19:49:27]

don't need extensions:: here or below.

[gpdavis, 2014/08/12 21:19:54]

Done.

+  }
+  if (withheld_permissions->scriptable_hosts().MatchesURL(url)) {
+    new_scriptable_hosts.AddOrigin(
+        extensions::UserScript::ValidUserScriptSchemes(), url.GetOrigin());
+  }
+
+  scoped_refptr<extensions::PermissionSet> new_permissions =
+      new extensions::PermissionSet(extensions::APIPermissionSet(),
+                                    extensions::ManifestPermissionSet(),
+                                    new_explicit_hosts,
+                                    new_scriptable_hosts);
+
+  // Update permissions for the session. This adds |new_permissions| to active
+  // permissions and granted permissions.
+  extensions::PermissionsUpdater updater(web_contents()->GetBrowserContext());
+  updater.AddPermissions(extension, new_permissions.get());
+
+  URLPatternSet permissions_union;
+  URLPatternSet::CreateUnion(
+      withheld_permissions->scriptable_hosts(),
+      PermissionsParser::GetRequiredPermissions(extension)->scriptable_hosts(),
+      &permissions_union);
+  UMA_HISTOGRAM_COUNTS_100(
+      "Extensions.ActiveScriptController.AlwaysRunCount",
+      extension->permissions_data()->GetEffectiveHostPermissions().size() -
+          permissions_union.size());
+
+  // Allow current tab to run injection.
+  OnClicked(extension);
+}
+
+bool ActiveScriptController::HasActiveScriptAction(const Extension* extension) {
+  return enabled_ && active_script_actions_.count(extension->id()) > 0;
+}
+
 ExtensionAction* ActiveScriptController::GetActionForExtension(
     const Extension* extension) {
   if (!enabled_ || pending_requests_.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();
 
@@ skipping 84 matching lines @@
   content::NavigationEntry* visible_entry =
       web_contents()->GetController().GetVisibleEntry();
   // Refuse to run if there's no visible entry, because we have no idea of
   // determining if it's the proper page. This should rarely, if ever, happen.
   if (!visible_entry)
     return;
 
   // We add this to the list of permitted extensions and erase pending entries
   // *before* running them to guard against the crazy case where running the
   // callbacks adds more entries.
   permitted_extensions_.insert(extension->id());

[not at google - send to devlin, 2014/08/12 19:49:27]

Ok - this has exposed a bug in our current UMA of this file.
RunPendingForExtension is called from 2 places: the ActiveTabPermissionGranter,
and OnClicked. The former will add to |permitted_extensions_| here even though
it may be that nothing was actually permitted.

I think the problem really is that we use |permitted_extensions_| for "this
extension was allowed to run" when really it's "this extension ran". We should
split those up - add 3 counters,
- ran due to active tab
- ran due to clicked on
- ran due to always-allow

and have the call sites of RunPendingForExtension set them (perhaps
RunPendingForExtension should be changed to return a bool whether it ran).

However - all of this UMA stuff is getting sufficiently complex that we should
probably do it in a follow-up.

[gpdavis, 2014/08/12 21:19:54]

Alright, let's do all of that in a follow-up CL then.  I'll address the non-UMA
comments.

[not at google - send to devlin, 2014/08/12 23:13:17]

Looks like you haven't removed the UMA from this patch? I'm saying you should
store away the UMA changes (delete them), submit this CL, then have a 2nd patch
to add them back + make those fixes.

and by "storage and delete" I mean both in this file and the histograms.xml
changes.

[gpdavis, 2014/08/13 00:08:24]

Oops-- sorry about that.  Had so many changes in this patch floating around in
my head and I forgot to remove those.  Removing now...

 
   PendingRequestMap::iterator iter = pending_requests_.find(extension->id());
   if (iter == pending_requests_.end())
     return;
 
   PendingRequestList requests;
   iter->second.swap(requests);
   pending_requests_.erase(extension->id());
 
   // Clicking to run the extension counts as granting it permission to run on
@@ skipping 90 matching lines @@
       pending_requests_.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(
         "Extensions.ActiveScriptController.PermittedExtensions",
         permitted_extensions_.size());
     UMA_HISTOGRAM_COUNTS_100(
         "Extensions.ActiveScriptController.DeniedExtensions",
         pending_requests_.size());

[not at google - send to devlin, 2014/08/12 19:49:27]

As Ilya suggests, let's also log the always-run extensions here. We need to make
sure that those aren't included in PermittedExtensions though.

   }
 }
 
 }  // namespace extensions