[Extensions] Support specifying specific frame IDs in chrome.scripting

Add support for specifying specific frame IDs (one or multiple) in the
chrome.scripting APIs (scripting.executeScript and scripting.insertCSS).
Add tests for the same.

Bug: 1148878
Change-Id: Idae2940c9bfe1d7b9d456799da941083eac2df0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2547432
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829593}

chrome/browser/extensions/api/scripting/scripting_api.cc

@@ -51,36 +51,9 @@ bool HasPermissionToInjectIntoFrame(const PermissionsData& permissions,
   return permissions.CanAccessPage(url, tab_id, error);
 }
 
-// Returns true if the `permissions` allow for injection into the given `tab`.
-// If false, populates `error`.
-bool HasPermissionToInject(const PermissionsData& permissions,
-                           int tab_id,
-                           content::WebContents* tab,
-                           std::string* error) {
-  // TODO(devlin): Support specifying multiple frames.
-  return HasPermissionToInjectIntoFrame(permissions, tab_id,
-                                        tab->GetMainFrame(), error);
-}
-
-void ExecuteScript(ScriptExecutor* script_executor,
-                   const std::string& code,
-                   const Extension& extension,
-                   ScriptExecutor::FrameScope frame_scope,
-                   bool user_gesture,
-                   ScriptExecutor::ScriptFinishedCallback callback) {
-  script_executor->ExecuteScript(
-      HostID(HostID::EXTENSIONS, extension.id()), UserScript::ADD_JAVASCRIPT,
-      code, frame_scope, {ExtensionApiFrameIdMap::kTopFrameId},
-      ScriptExecutor::MATCH_ABOUT_BLANK, UserScript::DOCUMENT_IDLE,
-      ScriptExecutor::DEFAULT_PROCESS,
-      /* webview_src */ GURL(), /* script_url */ GURL(), user_gesture,
-      base::nullopt, ScriptExecutor::JSON_SERIALIZED_RESULT,
-      std::move(callback));
-}
-
 // Returns true if the `target` can be accessed with the given `permissions`.
-// If the target can be accessed, populates `script_executor_out` with the
-// associated ScriptExecutor and `frame_scope_out` with the appropriate scope;
+// If the target can be accessed, populates `script_executor_out`,
+// `frame_scope_out`, and `frame_ids_out` with the appropriate values;
 // if the target cannot be accessed, populates `error_out`.
 bool CanAccessTarget(const PermissionsData& permissions,
                      const api::scripting::InjectionTarget& target,
@@ -88,6 +61,7 @@ bool CanAccessTarget(const PermissionsData& permissions,
                      bool include_incognito_information,
                      ScriptExecutor** script_executor_out,
                      ScriptExecutor::FrameScope* frame_scope_out,
+                     std::vector<int>* frame_ids_out,
                      std::string* error_out) {
   content::WebContents* tab = nullptr;
   TabHelper* tab_helper = nullptr;
@@ -99,6 +73,11 @@ bool CanAccessTarget(const PermissionsData& permissions,
     return false;
   }
 
+  if ((target.all_frames && *target.all_frames == true) && target.frame_ids) {
+    *error_out = "Cannot specify both 'allFrames' and 'frameIds'.";
+    return false;
+  }
+
   ScriptExecutor* script_executor = tab_helper->script_executor();
   DCHECK(script_executor);
 
@@ -107,17 +86,36 @@ bool CanAccessTarget(const PermissionsData& permissions,
           ? ScriptExecutor::INCLUDE_SUB_FRAMES
           : ScriptExecutor::SPECIFIED_FRAMES;
 
-  // TODO(devlin): It'd be best to do all the permission checks for the frames
-  // on the browser side, including child frames. Today, we only check the
-  // main frame, and then let the ScriptExecutor inject into all child frames
-  // (there's a permission check at the time of the injection in the renderer).
+  std::vector<int> frame_ids;
+  if (target.frame_ids) {
+    // NOTE: This creates a copy, but it's should always be very cheap, and it
+    // lets us keep |target| const.
+    frame_ids = *target.frame_ids;
+  } else {
+    frame_ids.push_back(ExtensionApiFrameIdMap::kTopFrameId);
+  }
+
   // TODO(devlin): We error out if the extension doesn't have access to the top
-  // frame, even if it may inject in child frames. This is inconsistent with
-  // content scripts (which can execute on child frames), but consistent with
-  // the old tabs.executeScript() API.
-  if (!HasPermissionToInject(permissions, target.tab_id, tab, error_out))
-    return false;
+  // frame, even if it may inject in child frames if allFrames is true. This is
+  // inconsistent with content scripts (which can execute on child frames), but
+  // consistent with the old tabs.executeScript() API.
+  for (int frame_id : frame_ids) {
+    content::RenderFrameHost* frame =
+        ExtensionApiFrameIdMap::GetRenderFrameHostById(tab, frame_id);
+    if (!frame) {
+      *error_out = base::StringPrintf("No frame with id %d in tab with id %d",
+                                      frame_id, target.tab_id);
+      return false;
+    }
 
+    DCHECK_EQ(content::WebContents::FromRenderFrameHost(frame), tab);
+    if (!HasPermissionToInjectIntoFrame(permissions, target.tab_id, frame,
+                                        error_out)) {
+      return false;
+    }
+  }
+
+  *frame_ids_out = std::move(frame_ids);
   *frame_scope_out = frame_scope;
   *script_executor_out = script_executor;
   return true;
@@ -141,9 +139,10 @@ ExtensionFunction::ResponseAction ScriptingExecuteScriptFunction::Run() {
   std::string error;
   ScriptExecutor* script_executor = nullptr;
   ScriptExecutor::FrameScope frame_scope = ScriptExecutor::SPECIFIED_FRAMES;
+  std::vector<int> frame_ids;
   if (!CanAccessTarget(*extension()->permissions_data(), injection.target,
                        browser_context(), include_incognito_information(),
-                       &script_executor, &frame_scope, &error)) {
+                       &script_executor, &frame_scope, &frame_ids, &error)) {
     return RespondNow(Error(std::move(error)));
   }
 
@@ -155,9 +154,13 @@ ExtensionFunction::ResponseAction ScriptingExecuteScriptFunction::Run() {
   std::string code_to_execute =
       base::StringPrintf("(%s)()", injection.function->c_str());
 
-  ExecuteScript(
-      script_executor, code_to_execute, *extension(), frame_scope,
-      user_gesture(),
+  script_executor->ExecuteScript(
+      HostID(HostID::EXTENSIONS, extension()->id()), UserScript::ADD_JAVASCRIPT,
+      code_to_execute, frame_scope, frame_ids,
+      ScriptExecutor::MATCH_ABOUT_BLANK, UserScript::DOCUMENT_IDLE,
+      ScriptExecutor::DEFAULT_PROCESS,
+      /* webview_src */ GURL(), /* script_url */ GURL(), user_gesture(),
+      base::nullopt, ScriptExecutor::JSON_SERIALIZED_RESULT,
       base::BindOnce(&ScriptingExecuteScriptFunction::OnScriptExecuted, this));
 
   return RespondLater();
@@ -202,9 +205,10 @@ ExtensionFunction::ResponseAction ScriptingInsertCSSFunction::Run() {
   std::string error;
   ScriptExecutor* script_executor = nullptr;
   ScriptExecutor::FrameScope frame_scope = ScriptExecutor::SPECIFIED_FRAMES;
+  std::vector<int> frame_ids;
   if (!CanAccessTarget(*extension()->permissions_data(), injection.target,
                        browser_context(), include_incognito_information(),
-                       &script_executor, &frame_scope, &error)) {
+                       &script_executor, &frame_scope, &frame_ids, &error)) {
     return RespondNow(Error(std::move(error)));
   }
   DCHECK(script_executor);
@@ -230,9 +234,8 @@ ExtensionFunction::ResponseAction ScriptingInsertCSSFunction::Run() {
   constexpr UserScript::RunLocation kRunLocation = UserScript::DOCUMENT_START;
   script_executor->ExecuteScript(
       HostID(HostID::EXTENSIONS, extension()->id()), UserScript::ADD_CSS,
-      *injection.css, frame_scope, {ExtensionApiFrameIdMap::kTopFrameId},
-      ScriptExecutor::MATCH_ABOUT_BLANK, kRunLocation,
-      ScriptExecutor::DEFAULT_PROCESS,
+      *injection.css, frame_scope, frame_ids, ScriptExecutor::MATCH_ABOUT_BLANK,
+      kRunLocation, ScriptExecutor::DEFAULT_PROCESS,
       /* webview_src */ GURL(), /* script_url */ GURL(), user_gesture(), origin,
       ScriptExecutor::NO_RESULT,
       base::BindOnce(&ScriptingInsertCSSFunction::OnCSSInserted, this));

chrome/common/extensions/api/scripting.idl

@@ -19,8 +19,13 @@ namespace scripting {
     // The ID of the tab into which to inject.
     long tabId;
 
+    // The <a href="https://developer.chrome.com/extensions/webNavigation#frame_ids">IDs</href>
+    // of specific frames to inject into.
+    long[]? frameIds;
+
     // Whether the script should inject into all frames within the tab. Defaults
     // to false.
+    // This must not be true if <code>frameIds</code> is specified.
     boolean? allFrames;
   };
 

chrome/test/data/extensions/api_test/scripting/css_injection/manifest.json

@@ -2,7 +2,7 @@
   "name": "Scripting API Test",
   "manifest_version": 3,
   "version": "0.1",
-  "permissions": ["scripting", "tabs"],
+  "permissions": ["scripting", "tabs", "webNavigation"],
   "background": {"service_worker": "worker.js"},
   "host_permissions": [
     "http://example.com/*",

chrome/test/data/extensions/api_test/scripting/css_injection/worker.js

@@ -2,8 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-const CSS = 'body { background-color: green !important }';
+const CSS_GREEN = 'body { background-color: green !important }';
 const GREEN = 'rgb(0, 128, 0)';
+const CSS_RED = 'body { background-color: red !important }';
+const RED = 'rgb(255, 0, 0)';
+const CSS_BLUE = 'body { background-color: blue !important }';
+const BLUE = 'rgb(0, 0, 255)';
+const CSS_CYAN = 'body { background-color: cyan !important }';
+const CYAN = 'rgb(0, 255, 255)';
 
 function getBodyColor() {
   const hostname = (new URL(location.href)).hostname;
@@ -33,6 +39,13 @@ async function getBodyColorsForTab(tabId) {
 }
 
 chrome.test.runTests([
+  // NOTE: These tests re-inject into (potentially) the same frames. This isn't
+  // a major problem, because more-recent stylesheets take precedent over
+  // previously-inserted ones, but it does put a somewhat unfortunate slight
+  // dependency between subtests. If this becomes a problem, we could reload
+  // tabs to ensure a "clean slate", but it's not worth the added complexity
+  // yet.
+  // Instead, each test uses a different color.
   async function changeBackground() {
     const query = {url: 'http://example.com/*'};
     const tab = await getSingleTab(query);
@@ -42,7 +55,7 @@ chrome.test.runTests([
             target: {
               tabId: tab.id,
             },
-            css: CSS,
+            css: CSS_GREEN,
           },
           resolve);
     });
@@ -64,7 +77,7 @@ chrome.test.runTests([
               tabId: tab.id,
               allFrames: true,
             },
-            css: CSS,
+            css: CSS_RED,
           },
           resolve);
     });
@@ -75,8 +88,42 @@ chrome.test.runTests([
     colors.sort();
     // Note: injected only in b.com and subframes.example, not c.com (which
     // the extension doesn't have permission to).
-    chrome.test.assertEq(`b.com ${GREEN}`, colors[0]);
-    chrome.test.assertEq(`subframes.example ${GREEN}`, colors[1]);
+    chrome.test.assertEq(`b.com ${RED}`, colors[0]);
+    chrome.test.assertEq(`subframes.example ${RED}`, colors[1]);
+    chrome.test.succeed();
+  },
+
+  async function specificFrames() {
+    const query = {url: 'http://subframes.example/*'};
+    const tab = await getSingleTab(query);
+    const frames = await new Promise(resolve => {
+      chrome.webNavigation.getAllFrames({tabId: tab.id}, resolve);
+    });
+    const bComFrame = frames.find(frame => {
+      return (new URL(frame.url)).hostname == 'b.com';
+    });
+    chrome.test.assertTrue(!!bComFrame);
+
+    const results = await new Promise(resolve => {
+      chrome.scripting.insertCSS(
+          {
+            target: {
+              tabId: tab.id,
+              frameIds: [bComFrame.frameId],
+            },
+            css: CSS_BLUE,
+          },
+          resolve);
+    });
+    chrome.test.assertNoLastError();
+    chrome.test.assertEq(undefined, results);
+
+    const colors = await getBodyColorsForTab(tab.id);
+    chrome.test.assertEq(2, colors.length);
+    colors.sort();
+    chrome.test.assertEq(`b.com ${BLUE}`, colors[0]);
+    // NOTE: subframes.example frame is still red from the previous test.
+    chrome.test.assertEq(`subframes.example ${RED}`, colors[1]);
     chrome.test.succeed();
   },
 
@@ -90,7 +137,7 @@ chrome.test.runTests([
           target: {
             tabId: nonExistentTabId,
           },
-          css: CSS,
+          css: CSS_CYAN,
         },
         results => {
           chrome.test.assertLastError(`No tab with id: ${nonExistentTabId}`);
@@ -107,7 +154,7 @@ chrome.test.runTests([
           target: {
             tabId: tab.id,
           },
-          css: CSS,
+          css: CSS_CYAN,
         },
         results => {
           chrome.test.assertLastError(

chrome/test/data/extensions/api_test/scripting/sub_frames/manifest.json

@@ -2,7 +2,7 @@
   "name": "Scripting API Test",
   "manifest_version": 3,
   "version": "0.1",
-  "permissions": ["scripting", "tabs"],
+  "permissions": ["scripting", "tabs", "webNavigation"],
   "background": {"service_worker": "worker.js"},
   "host_permissions": ["http://a.com/*", "http://b.com/*"]
 }

chrome/test/data/extensions/api_test/scripting/sub_frames/worker.js

@@ -6,14 +6,40 @@ function injectedFunction() {
   return location.href;
 }
 
+// Returns the error message used when the extension cannot access the contents
+// of a frame.
+function getAccessError(url) {
+    return `Cannot access contents of url "${url}". ` +
+           'Extension manifest must request permission ' +
+           'to access this host.';
+}
+
+// Returns the single tab matching the given `query`.
 async function getSingleTab(query) {
-  const tabs = await new Promise(resolve => {
-    chrome.tabs.query(query, resolve);
-  });
+  const tabs = await chrome.tabs.query(query);
   chrome.test.assertEq(1, tabs.length);
   return tabs[0];
 }
 
+// Returns all frames in the given tab.
+async function getFramesInTab(tabId) {
+  // TODO(devlin): Update this when webNavigation supports promises directly.
+  const frames = await new Promise(resolve => {
+    chrome.webNavigation.getAllFrames({tabId: tabId}, resolve);
+  });
+  chrome.test.assertTrue(frames.length > 0);
+  return frames;
+}
+
+// Returns the ID of the frame with the given `hostname`.
+function findFrameIdWithHostname(frames, hostname) {
+  const frame = frames.find(frame => {
+    return (new URL(frame.url)).hostname == hostname;
+  });
+  chrome.test.assertTrue(!!frame, 'No frame with hostname: ' + hostname);
+  return frame.frameId;
+}
+
 chrome.test.runTests([
   async function allowedTopFrameAccess() {
     const query = {url: 'http://a.com/*'};
@@ -54,10 +80,149 @@ chrome.test.runTests([
           function: injectedFunction,
         },
         results => {
+          chrome.test.assertLastError(getAccessError(tab.url));
+          chrome.test.assertEq(undefined, results);
+          chrome.test.succeed();
+        });
+  },
+
+  // Tests injecting into a single specified frame.
+  async function singleSpecificFrames() {
+    const query = {url: 'http://a.com/*'};
+    const tab = await getSingleTab(query);
+    const frames = await getFramesInTab(tab.id);
+    const frameId = findFrameIdWithHostname(frames, 'b.com');
+
+    const results = await new Promise(resolve => {
+      chrome.scripting.executeScript(
+          {
+            target: {
+              tabId: tab.id,
+              frameIds: [frameId],
+            },
+            function: injectedFunction,
+          },
+          resolve);
+    });
+    chrome.test.assertNoLastError();
+    chrome.test.assertEq(1, results.length);
+
+    const resultUrl = new URL(results[0].result);
+    chrome.test.assertEq('b.com', resultUrl.hostname);
+    chrome.test.succeed();
+  },
+
+  // Tests injecting when multiple frames are specified.
+  async function multipleSpecificFrames() {
+    const query = {url: 'http://a.com/*'};
+    const tab = await getSingleTab(query);
+    const frames = await getFramesInTab(tab.id);
+    const frameIds = [
+        findFrameIdWithHostname(frames, 'a.com'),
+        findFrameIdWithHostname(frames, 'b.com'),
+    ];
+
+    const results = await new Promise(resolve => {
+      chrome.scripting.executeScript(
+          {
+            target: {
+              tabId: tab.id,
+              frameIds: frameIds,
+            },
+            function: injectedFunction,
+          },
+          resolve);
+    });
+    chrome.test.assertNoLastError();
+    chrome.test.assertEq(2, results.length);
+
+    // Since we specified frame IDs, there's no guarantee as to the order
+    // of the result. Compare a sorted output.
+    const resultUrls = results.map(result => {
+      return (new URL(result.result)).hostname;
+    });
+    chrome.test.assertEq(['a.com', 'b.com'], resultUrls.sort());
+    chrome.test.succeed();
+  },
+
+  // Tests that an error is thrown when an extension doesn't have access to
+  // one of the frames specified.
+  async function disallowedSpecificFrame() {
+    const query = {url: 'http://a.com/*'};
+    const tab = await getSingleTab(query);
+    const frames = await getFramesInTab(tab.id);
+    const deniedFrame = frames.find((frame) => {
+      return (new URL(frame.url)).hostname == 'c.com';
+    });
+    const frameIds = [
+        findFrameIdWithHostname(frames, 'b.com'),
+        findFrameIdWithHostname(frames, 'c.com'),
+    ];
+
+    chrome.scripting.executeScript(
+        {
+          target: {
+            tabId: tab.id,
+            frameIds: frameIds,
+          },
+          function: injectedFunction,
+        },
+        async results => {
+          chrome.test.assertLastError(getAccessError(deniedFrame.url));
+          chrome.test.assertEq(undefined, results);
+          chrome.test.succeed();
+        });
+  },
+
+  // Tests that an error is thrown when specifying a non-existent frame ID.
+  async function nonExistentSpecificFrame() {
+    const query = {url: 'http://a.com/*'};
+    const tab = await getSingleTab(query);
+    const frames = await getFramesInTab(tab.id);
+    const nonExistentFrameId = 99999;
+    const frameIds = [
+        findFrameIdWithHostname(frames, 'b.com'),
+        nonExistentFrameId,
+    ];
+
+    chrome.scripting.executeScript(
+        {
+          target: {
+            tabId: tab.id,
+            frameIds: frameIds,
+          },
+          function: injectedFunction,
+        },
+        async results => {
+          chrome.test.assertLastError(
+              `No frame with id ${nonExistentFrameId} in ` +
+              `tab with id ${tab.id}`);
+          chrome.test.assertEq(undefined, results);
+          chrome.test.succeed();
+        });
+  },
+
+  // Test that an extension cannot specify both allFrames and frameIds.
+  async function specifyingBothFrameIdsAndAllFramesIsInvalid() {
+    const query = {url: 'http://a.com/*'};
+    const tab = await getSingleTab(query);
+    const frames = await getFramesInTab(tab.id);
+    const frameIds = [
+        findFrameIdWithHostname(frames, 'b.com'),
+    ];
+
+    chrome.scripting.executeScript(
+        {
+          target: {
+            tabId: tab.id,
+            frameIds: frameIds,
+            allFrames: true,
+          },
+          function: injectedFunction,
+        },
+        async results => {
           chrome.test.assertLastError(
-              `Cannot access contents of url "${tab.url}". ` +
-                'Extension manifest must request permission ' +
-                'to access this host.');
+              `Cannot specify both 'allFrames' and 'frameIds'.`);
           chrome.test.assertEq(undefined, results);
           chrome.test.succeed();
         });