Making webRequest.addEventListener internal

chrome/common/extensions/extension_permission_set.cc

Index: chrome/common/extensions/extension_permission_set.cc
diff --git a/chrome/common/extensions/extension_permission_set.cc b/chrome/common/extensions/extension_permission_set.cc
index cf2e40f802a4d89f8f1e46748cda540280f978c6..f7af1104bb0a45031bafee5cc060bf16c35ec996 100644
--- a/chrome/common/extensions/extension_permission_set.cc
+++ b/chrome/common/extensions/extension_permission_set.cc
@@ -322,6 +322,9 @@ void ExtensionAPIPermission::RegisterAllPermissions(
       kTerminalPrivate, "terminalPrivate", 0, ExtensionPermissionMessage::kNone,
       kFlagCannotBeOptional);
   info->RegisterPermission(
+      kWebRequestInternal, "webRequestInternal", 0,
+      ExtensionPermissionMessage::kNone, kFlagCannotBeOptional);
+  info->RegisterPermission(
       kWebSocketProxyPrivate, "webSocketProxyPrivate", 0,
       ExtensionPermissionMessage::kNone,
       kFlagCannotBeOptional);
@@ -875,6 +878,10 @@ void ExtensionPermissionSet::InitImplicitExtensionPermissions(
   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();