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/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 19 matching lines @@
     content::WebContents* web_contents, int tab_id, Profile* profile)
     : WebContentsObserver(web_contents), tab_id_(tab_id) {
   registrar_.Add(this,
                  chrome::NOTIFICATION_EXTENSION_UNLOADED,
                  content::Source<Profile>(profile));
 }
 
 ActiveTabPermissionGranter::~ActiveTabPermissionGranter() {}
 
 void ActiveTabPermissionGranter::GrantIfRequested(const Extension* extension) {
-  if (!extension->HasAPIPermission(extensions::APIPermission::kActiveTab) &&
-      !extension->HasAPIPermission(extensions::APIPermission::kTabCapture)) {
+  if (!extension->HasAPIPermission(APIPermission::kActiveTab) &&
+      !extension->HasAPIPermission(APIPermission::kTabCapture)) {
     return;
   }

[not at google - send to devlin, 2013/09/13 03:22:44]

this check here isn't worth much anymore

[justinlin, 2013/09/13 19:27:32]

Still need this, otherwise there will be some un-needed IPC's.

[not at google - send to devlin, 2013/09/13 22:09:45]

why? you check new_apis.empty.

[justinlin, 2013/09/13 22:47:15]

Done. Well, now I do :)

 
-  if (IsGranted(extension))
+  if (granted_extensions_.Contains(extension->id()))
     return;
 
-  URLPattern pattern(UserScript::ValidUserScriptSchemes());
-  if (pattern.Parse(web_contents()->GetURL().spec()) !=
-          URLPattern::PARSE_SUCCESS) {
-    // Pattern parsing could fail if this is an unsupported URL e.g. chrome://.
-    return;
+  if (extension->HasAPIPermission(APIPermission::kActiveTab)) {
+    URLPattern pattern(UserScript::ValidUserScriptSchemes());
+    if (pattern.Parse(web_contents()->GetURL().spec()) !=
+            URLPattern::PARSE_SUCCESS) {
+      // 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());
+
+    PermissionsData::UpdateTabSpecificPermissions(extension,
+                                                  tab_id_,
+                                                  new_permissions);
+    Send(new ExtensionMsg_UpdateTabSpecificPermissions(GetPageID(),
+                                                       tab_id_,
+                                                       extension->id(),
+                                                       new_hosts));
   }
 
-  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());
+  if (extension->HasAPIPermission(APIPermission::kTabCapture)) {
+    APIPermissionSet new_apis;
+    new_apis.insert(APIPermission::kTabCaptureForTab);
+    scoped_refptr<const PermissionSet> new_permissions =
+        new PermissionSet(new_apis, URLPatternSet(), URLPatternSet());
+    PermissionsData::UpdateTabSpecificPermissions(extension,
+                                                  tab_id_,
+                                                  new_permissions);

[not at google - send to devlin, 2013/09/13 03:22:44]

could you factor this so that each case shares more code? like

APIPermissionSet new_apis;
URLPatternSet new_hosts;

if (extension->HasPermission(kActiveTab)) {
 ...
}
if (extension->HasPermission(kTabCapture)) {
 ...
}

scoped_refptr<...>
PermissionsData::Update...();
Send(..)  // might as well

[justinlin, 2013/09/13 19:27:32]

Done. Fixed a kind of subtle bug too with this second patch. If both activeTab
and tabCapture permissions were present, it would early return in the activeTab
URL check, so you wouldn't get the tabCaptureForTab permission..

+  }
 
-  PermissionsData::UpdateTabSpecificPermissions(extension,
-                                                tab_id_,
-                                                new_permissions);
   granted_extensions_.Insert(extension);
-  Send(new ExtensionMsg_UpdateTabSpecificPermissions(GetPageID(),
-                                                     tab_id_,
-                                                     extension->id(),
-                                                     new_hosts));
-}
-
-bool ActiveTabPermissionGranter::IsGranted(const Extension* extension) {
-  return granted_extensions_.Contains(extension->id());
 }
 
 void ActiveTabPermissionGranter::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
   if (details.is_in_page)
     return;
   DCHECK(details.is_main_frame);  // important: sub-frames don't get granted!
   ClearActiveExtensionsAndNotify();
 }
@@ skipping 29 matching lines @@
 
   Send(new ExtensionMsg_ClearTabSpecificPermissions(tab_id_, extension_ids));
   granted_extensions_.Clear();
 }
 
 int32 ActiveTabPermissionGranter::GetPageID() {
   return web_contents()->GetController().GetVisibleEntry()->GetPageID();
 }
 
 }  // namespace extensions