Making webRequest.addEventListener internal

chrome/common/extensions/extension_permission_set.cc

 // Copyright (c) 2012 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/common/extensions/extension_permission_set.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/command_line.h"
@@ skipping 304 matching lines @@
   info->RegisterPermission(
       kInputMethodPrivate, "inputMethodPrivate", 0,
       ExtensionPermissionMessage::kNone, kFlagCannotBeOptional);
   info->RegisterPermission(
       kEchoPrivate, "echoPrivate", 0, ExtensionPermissionMessage::kNone,
       kFlagCannotBeOptional);
   info->RegisterPermission(
       kTerminalPrivate, "terminalPrivate", 0, ExtensionPermissionMessage::kNone,
       kFlagCannotBeOptional);
   info->RegisterPermission(
+      kWebRequestInternal, "webRequestInternal", 0,
+      ExtensionPermissionMessage::kNone, kFlagCannotBeOptional);
+  info->RegisterPermission(
       kWebSocketProxyPrivate, "webSocketProxyPrivate", 0,
       ExtensionPermissionMessage::kNone,
       kFlagCannotBeOptional);
   info->RegisterPermission(
       kWebstorePrivate, "webstorePrivate", 0,
       ExtensionPermissionMessage::kNone,
       kFlagCannotBeOptional);
 
   // Full url access permissions.
   info->RegisterPermission(
@@ skipping 533 matching lines @@
 
 void ExtensionPermissionSet::InitImplicitExtensionPermissions(
     const Extension* extension) {
   // Add the implied permissions.
   if (!extension->plugins().empty())
     apis_.insert(ExtensionAPIPermission::kPlugin);
 
   if (!extension->devtools_url().is_empty())
     apis_.insert(ExtensionAPIPermission::kDevtools);
 
+  // The webRequest permission implies the internal version as well.
+  if (apis_.find(ExtensionAPIPermission::kWebRequest) != apis_.end())

[vabr (Chromium), 2012/05/14 18:10:35]

Is this too ad-hoc?

[battre, 2012/05/15 09:23:38]

I think this is fine, but somebody from the extensions team should judge.

[vabr (Chromium), 2012/05/15 11:57:52]

OK, I'll wait until further reviewers are roped in. My suggestion of a less
ad-hoc solution would be to do this automatically based on some dependency
stored in JSON.

[Matt Perry, 2012/05/16 23:47:00]

Wait, this is a good point. Why do we need this at all? Isn't the API
automatically exposed to the chromeHidden.internalAPIs? What does the
"declarative" (now "events") API do?

[battre, 2012/05/17 08:56:19]

It is added to kNonPermissionModuleNames[] in extension_permission_set.cc. Given
that lazy loading of .json files is already implemented or should be
implemented, I don't know whether we want to add webRequestInternal to all
extensions, even those not requesting webRequest.

[Matt Perry, 2012/05/17 18:44:45]

It's best if we only load webRequestInternal for extensions requesting
webRequest. If this code is the way to ensure that, let's keep it.

[vabr (Chromium), 2012/05/17 21:23:37]

Yes, I believe that this code does "add webRequestInternal permission if there
is already a webRequest permission". If this mechanism is to be used for other
APIs we might consider marking this kind of dependency in the JSON files instead
of hard-coding it here.

Given your last comment, Matt, I guess it is OK for me to add this to CQ. If you
object, please let me know, I will wait at least one more day (possibly longer).

+    apis_.insert(ExtensionAPIPermission::kWebRequestInternal);
+
   // Add the scriptable hosts.
   for (UserScriptList::const_iterator content_script =
            extension->content_scripts().begin();
        content_script != extension->content_scripts().end(); ++content_script) {
     URLPatternSet::const_iterator pattern =
         content_script->url_patterns().begin();
     for (; pattern != content_script->url_patterns().end(); ++pattern)
       scriptable_hosts_.AddPattern(*pattern);
   }
 }
@@ skipping 74 matching lines @@
   ExtensionOAuth2Scopes current_scopes = scopes();
   ExtensionOAuth2Scopes new_scopes = permissions->scopes();
   ExtensionOAuth2Scopes delta_scopes;
   std::set_difference(new_scopes.begin(), new_scopes.end(),
                       current_scopes.begin(), current_scopes.end(),
                       std::inserter(delta_scopes, delta_scopes.begin()));
 
   // We have less privileges if there are additional scopes present.
   return !delta_scopes.empty();
 }