Add frameId to chrome.tabs.executeScript/insertCSS

extensions/renderer/programmatic_script_injector.cc

Index: extensions/renderer/programmatic_script_injector.cc
diff --git a/extensions/renderer/programmatic_script_injector.cc b/extensions/renderer/programmatic_script_injector.cc
index f576305a7fa8c604c090a9d7623cfd95a7e09482..e19a4c98ef4388444b1910e6d42c1ab7c7c1a647 100644
--- a/extensions/renderer/programmatic_script_injector.cc
+++ b/extensions/renderer/programmatic_script_injector.cc
@@ -13,8 +13,10 @@
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/manifest_constants.h"
+#include "extensions/common/permissions/api_permission.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "extensions/renderer/injection_host.h"
+#include "extensions/renderer/renderer_extension_registry.h"
 #include "extensions/renderer/script_context.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
@@ -30,8 +32,10 @@ ProgrammaticScriptInjector::ProgrammaticScriptInjector(
       url_(
           ScriptContext::GetDataSourceURLForFrame(render_frame->GetWebFrame())),
       finished_(false) {
-  effective_url_ = ScriptContext::GetEffectiveDocumentURL(
-      render_frame->GetWebFrame(), url_, params.match_about_blank);
+  if (url_.SchemeIs(url::kAboutScheme)) {
+    origin_for_about_error_ =
+        render_frame->GetWebFrame()->securityOrigin().toString().utf8();
+  }
 }
 
 ProgrammaticScriptInjector::~ProgrammaticScriptInjector() {
@@ -129,16 +133,15 @@ void ProgrammaticScriptInjector::OnWillNotInject(
   std::string error;
   switch (reason) {
     case NOT_ALLOWED:
-      if (url_.SchemeIs(url::kAboutScheme)) {
+      if (!CanShowUrlInError()) {
+        error = manifest_errors::kCannotAccessPage;
+      } else if (!origin_for_about_error_.empty()) {
         error = ErrorUtils::FormatErrorMessage(
             manifest_errors::kCannotAccessAboutUrl, url_.spec(),
-            effective_url_.GetOrigin().spec());
+            origin_for_about_error_);
       } else {
-        // TODO(?) It would be nice to show kCannotAccessPageWithUrl here if
-        // this is triggered by an extension with tabs permission. See
-        // https://codereview.chromium.org/1414223005/diff/1/extensions/
-        // common/manifest_constants.cc#newcode269
-        error = manifest_errors::kCannotAccessPage;
+        error = ErrorUtils::FormatErrorMessage(
+            manifest_errors::kCannotAccessPageWithUrl, url_.spec());
       }
       break;
     case EXTENSION_REMOVED:  // no special error here.
@@ -148,6 +151,17 @@ void ProgrammaticScriptInjector::OnWillNotInject(
   Finish(error, render_frame);
 }
 
+bool ProgrammaticScriptInjector::CanShowUrlInError() const {
+  if (params_->host_id.type() != HostID::EXTENSIONS)
+    return false;
+  const Extension* extension =
+      RendererExtensionRegistry::Get()->GetByID(params_->host_id.id());
+  if (!extension)
+    return false;
+  return extension->permissions_data()->active_permissions().HasAPIPermission(

[Devlin, 2016/01/25 19:36:40]

It might be worth a note that we don't need to account for activeTab here
because if the extension has activeTab, it wouldn't have been rejected.

[robwu, 2016/01/26 11:03:02]

Even with activeTab, one does not get access to restricted URLs such as
chrome://. Should I add a check for activeTab as well?

[Devlin, 2016/01/26 20:40:36]

Nah - activeTab shouldn't really grant any access to chrome:// urls, so this is
okay.

[robwu, 2016/01/26 23:57:58]

Acknowledged.

+      APIPermission::kTab);
+}
+
 UserScript::RunLocation ProgrammaticScriptInjector::GetRunLocation() const {
   return static_cast<UserScript::RunLocation>(params_->run_at);
 }