Allow tabCapture API to be granted for chrome:// pages.

chrome/browser/extensions/active_tab_permission_granter.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/browser/extensions/active_tab_permission_granter.h"
 
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_id.h"
@@ skipping 30 matching lines @@
       !extension->HasAPIPermission(extensions::APIPermission::kTabCapture)) {
     return;
   }
 
   if (IsGranted(extension))
     return;
 
   URLPattern pattern(UserScript::ValidUserScriptSchemes());
   if (pattern.Parse(web_contents()->GetURL().spec()) !=
           URLPattern::PARSE_SUCCESS) {
+    // Allow tabCapture API to be granted access to chrome:// pages since it
+    // already requests all hosts permissions on extension install.
+    if (extension->HasAPIPermission(extensions::APIPermission::kTabCapture)) {
+      granted_extensions_.Insert(extension);

[not at google - send to devlin, 2013/06/17 18:14:13]

this means that an extension with the tab capture permission can get injection
access to chrome:// pages.

you probably want to check the active tab permission granter from the API
bindings instead.

[not at google - send to devlin, 2013/06/17 18:16:09]

checking it elsewhere wouldn't work either. maybe we need to just make it always
work for chrome:// pages and make sure we properly check it elsewhere. I think
that makes sense.

[justinlin, 2013/06/17 18:22:20]

Does it actually do that? I thought the injection stuff is only granted if the
below UpdateTabSpecificPermissions IPC is sent?

I'm not sure I understand what you're proposing?

[not at google - send to devlin, 2013/06/17 18:44:45]

I see; IsGranted is only used in the tab capture API. Yes you're right as it is
this wouldn't give script privileges.

It's nevertheless very strange to have IsGranted return true when in fact it
hasn't been granted, which is what will happen when an extension has tabCapture.
I can imagine a regression in which IsGranted *is* in fact used in that scenario
and a false sense of security is given by the permission granting logic.

This class is supposed to be general, not given API specific code; it just so
happens that the tabCapture API implies the activeTab permission. Come to think
of it a nicer way to do that now would be to use the features system, but,
another day.

I would like to suggest that we grant for chrome:// URLs regardless, and make
the chrome:// script injection logic part of the application logic, but I'm not
totally confident. We could test it easily enough. What would that take?

Or possibly activeTab is actually the wrong idiom for this after all and we
should make the process separate, e.g. make this class take a Grant callback
which takes a URL and returns the new permission data; then have both a
host_permission_granter and a tab_capture_permission_granter in TabHelper.

+    }
+
     // Pattern parsing could fail if this is an unsupported URL e.g. chrome://.
     return;
   }
 
   APIPermissionSet new_apis;
   new_apis.insert(APIPermission::kTab);
   URLPatternSet new_hosts;
   new_hosts.AddPattern(pattern);
   scoped_refptr<const PermissionSet> new_permissions =
       new PermissionSet(new_apis, new_hosts, URLPatternSet());
@@ skipping 52 matching lines @@
 
   Send(new ExtensionMsg_ClearTabSpecificPermissions(tab_id_, extension_ids));
   granted_extensions_.Clear();
 }
 
 int32 ActiveTabPermissionGranter::GetPageID() {
   return web_contents()->GetController().GetActiveEntry()->GetPageID();
 }
 
 }  // namespace extensions