Fix the origin access whitelist for extensions

extensions/renderer/dispatcher.cc

Index: extensions/renderer/dispatcher.cc
diff --git a/extensions/renderer/dispatcher.cc b/extensions/renderer/dispatcher.cc
index fa242177fa2341141849ce945ff916f1a5f9a356..0124a43048966a4fa52a6b7bf2adcc7b3077fd13 100644
--- a/extensions/renderer/dispatcher.cc
+++ b/extensions/renderer/dispatcher.cc
@@ -271,7 +271,7 @@ void Dispatcher::DidCreateScriptContext(
   // Initialize origin permissions for content scripts, which can't be
   // initialized in |OnActivateExtension|.
   if (context_type == Feature::CONTENT_SCRIPT_CONTEXT)
-    UpdateOriginPermissions(extension);
+    InitOriginPermissions(extension);
 
   {
     scoped_ptr<ModuleSystem> module_system(
@@ -506,7 +506,7 @@ void Dispatcher::WebKitInitialized() {
     const Extension* extension = extensions_.GetByID(*iter);
     CHECK(extension);
 
-    UpdateOriginPermissions(extension);
+    InitOriginPermissions(extension);
   }
 
   EnableCustomElementWhiteList();
@@ -565,7 +565,7 @@ void Dispatcher::OnActivateExtension(const std::string& extension_id) {
     extensions::DOMActivityLogger::AttachToWorld(
         extensions::DOMActivityLogger::kMainWorldId, extension_id);
 
-    UpdateOriginPermissions(extension);
+    InitOriginPermissions(extension);
   }
 
   UpdateActiveExtensions();
@@ -741,8 +741,27 @@ void Dispatcher::OnUpdatePermissions(
   scoped_refptr<const PermissionSet> withheld =
       params.withheld_permissions.ToPermissionSet();
 
+  // If webkit isn't initialized, this will be done when it finishes starting

[not at google - send to devlin, 2014/07/23 01:18:54]

s/webkit/blink/

though I think the comment isn't really necessary

[Devlin, 2014/07/23 15:41:51]

Done.

+  // up.
+  if (is_webkit_initialized_) {
+    bool was_added = params.was_added;

[not at google - send to devlin, 2014/07/23 01:18:54]

this alias seems like overkill.

[Devlin, 2014/07/23 15:41:51]

Done.

+    URLPatternSet difference;
+    if (was_added) {
+      URLPatternSet::CreateDifference(
+          active->effective_hosts(),
+          extension->permissions_data()->GetEffectiveHostPermissions(),
+          &difference);
+    } else {
+      URLPatternSet::CreateDifference(
+          extension->permissions_data()->GetEffectiveHostPermissions(),
+          active->effective_hosts(),
+          &difference);
+    }
+
+    UpdateOriginPermissions(extension, was_added, difference);
+  }
+
   extension->permissions_data()->SetPermissions(active, withheld);
-  UpdateOriginPermissions(extension);
   UpdateBindings(extension->id());
 }
 
@@ -771,24 +790,32 @@ void Dispatcher::UpdateActiveExtensions() {
   delegate_->OnActiveExtensionsUpdated(active_extensions);
 }
 
-void Dispatcher::UpdateOriginPermissions(const Extension* extension) {
-  const URLPatternSet& hosts =
-      extension->permissions_data()->GetEffectiveHostPermissions();
-  WebSecurityPolicy::resetOriginAccessWhitelists();

[Devlin, 2014/07/22 21:56:47]

This was introduced in https://codereview.chromium.org/348313003, when I for
some inexplicable reason thought this would only clear the extension's origin
whitelist.  Clearly, this isn't the case, and results in us clearing the
whitelist for extensions obscenely frequently.

[not at google - send to devlin, 2014/07/23 01:18:54]

right... good point.

This would be such a simpler change if we just added a
clearOriginAccessWhitelist method which takes the origin. doesn't look very
hard, but blink changes... eh.

though a only-slightly-less-complicated change in the vein of what you had
before would be to remove the existing permissions then add the new ones.

[Devlin, 2014/07/23 15:41:51]

But to add the new ones, we add the new ones for _each existing extension_ which
seems bad.  Also, this would definitely mean updating callers of this to make
sure we aren't doing what we're doing now - which is calling this for each
extension when a new renderer is created.  Overall, to me, that sounds like a
slightly more invasive change... WDYT?

[not at google - send to devlin, 2014/07/23 15:45:35]

I don't think so? add/remove take the origin which is unique for every extension
(takes extension->url()). Maybe I'm misunderstanding.
I was imagining a method like SetOriginPermissions instead of
UpdateOriginPermissions (or still call it UpdateOriginPermissions... whatever).
instead of taking a was_added boolean it'd take the old permissions. in the case
of InitOriginPermissions the old permissions would be an empty set.

[Devlin, 2014/07/23 16:14:01]

Ahhh, I see, I misunderstood (thought you meant still use
resetOriginAccessWhitelists(), and then re-add everything).  Yes, that will
work. :)

+void Dispatcher::InitOriginPermissions(const Extension* extension) {
   delegate_->InitOriginPermissions(extension,
                                    IsExtensionActive(extension->id()));
-  for (URLPatternSet::const_iterator iter = hosts.begin(); iter != hosts.end();
-       ++iter) {
-    const char* schemes[] = {
-        url::kHttpScheme,
-        url::kHttpsScheme,
-        url::kFileScheme,
-        content::kChromeUIScheme,
-        url::kFtpScheme,
-    };
+  UpdateOriginPermissions(
+      extension,
+      true,  // was added
+      extension->permissions_data()->GetEffectiveHostPermissions());
+}
+
+void Dispatcher::UpdateOriginPermissions(
+    const Extension* extension,
+    bool was_added,
+    const URLPatternSet& patterns) {
+  static const char* schemes[] = {

[not at google - send to devlin, 2014/07/23 01:18:55]

kSchemes?

[Devlin, 2014/07/23 15:41:51]

Done.

+      url::kHttpScheme,
+      url::kHttpsScheme,
+      url::kFileScheme,
+      content::kChromeUIScheme,
+      url::kFtpScheme,
+  };
+  for (URLPatternSet::const_iterator iter = patterns.begin();
+       iter != patterns.end(); ++iter) {
     for (size_t j = 0; j < arraysize(schemes); ++j) {
       if (iter->MatchesScheme(schemes[j])) {
-        WebSecurityPolicy::addOriginAccessWhitelistEntry(
+        (was_added ? WebSecurityPolicy::addOriginAccessWhitelistEntry
+                   : WebSecurityPolicy::removeOriginAccessWhitelistEntry)(
             extension->url(),
             WebString::fromUTF8(schemes[j]),
             WebString::fromUTF8(iter->host()),