[Extensions] Update debugger API checks to use committed URL

We should be using last committed URL for security checks, not visible
URL. This requires updating a number of the extension API tests in
order to wait for the tab to commit before attaching the debugger; also
take this as an opportunity to update a few with some async helpers
(like await, etc).

Bug: 237908
Change-Id: Ic9fcb010e4d029204d5d2e0b8e6bbbe76738bc1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2327462Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794853}

chrome/browser/extensions/api/debugger/debugger_api.cc

@@ -416,12 +416,9 @@ bool DebuggerFunction::InitAgentHost(std::string* error) {
         *debuggee_.tab_id, browser_context(), include_incognito_information(),
         &web_contents);
     if (result && web_contents) {
-      // TODO(rdevlin.cronin) This should definitely be GetLastCommittedURL().
-      GURL url = web_contents->GetVisibleURL();
-
       if (!ExtensionCanAttachToURL(
-              *extension(), url, Profile::FromBrowserContext(browser_context()),
-              error)) {
+              *extension(), web_contents->GetLastCommittedURL(),
+              Profile::FromBrowserContext(browser_context()), error)) {
         return false;
       }
 

chrome/test/data/extensions/api_test/bindings/uncaught_exception_logging/test.js

@@ -2,49 +2,49 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-function createTestFunction(expected_message) {
-  return function(tab) {
+function verifyException(expectedMessage, tabId) {
   function onDebuggerEvent(debuggee, method, params) {
-      if (debuggee.tabId == tab.id && method == 'Runtime.exceptionThrown') {
+    if (debuggee.tabId == tabId && method == 'Runtime.exceptionThrown') {
       var exception = params.exceptionDetails.exception;
-        if (exception.value.indexOf(expected_message) > -1) {
+      if (exception.value.indexOf(expectedMessage) > -1) {
         chrome.debugger.onEvent.removeListener(onDebuggerEvent);
         chrome.test.succeed();
       }
     }
   };
   chrome.debugger.onEvent.addListener(onDebuggerEvent);
-    chrome.debugger.attach({ tabId: tab.id }, "1.1", function() {
+  chrome.debugger.attach({ tabId: tabId }, "1.1", function() {
     // Enabling console provides both stored and new messages via the
     // Console.messageAdded event.
-      chrome.debugger.sendCommand({ tabId: tab.id }, "Runtime.enable");
+    chrome.debugger.sendCommand({ tabId: tabId }, "Runtime.enable");
   });
-  }
 }
 
+let openTab;
+
 chrome.test.runTests([
-  function testExceptionInExtensionPage() {
-    chrome.tabs.create(
-        {url: chrome.runtime.getURL('extension_page.html')},
-        createTestFunction('Exception thrown in extension page.'));
+  async function testExceptionInExtensionPage() {
+    ({openTab} = await import('/_test_resources/test_util/tabs_util.js'));
+    const tab = await openTab(chrome.runtime.getURL('extension_page.html'));
+    verifyException('Exception thrown in extension page.', tab.id);
   },
 
-  function testExceptionInInjectedScript() {
+  async function testExceptionInInjectedScript() {
     function injectScriptAndSendMessage(tab) {
       chrome.tabs.executeScript(
           tab.id,
           { file: 'content_script.js' },
           function() {
-            createTestFunction('Exception thrown in injected script.')(tab);
+            verifyException('Exception thrown in injected script.', tab.id);
           });
     }
 
-    chrome.test.getConfig(function(config) {
-      var test_url =
-          'http://localhost:PORT/extensions/test_file.html'
-              .replace(/PORT/, config.testServer.port);
-
-      chrome.tabs.create({ url: test_url }, injectScriptAndSendMessage);
+    chrome.test.getConfig(async function(config) {
+      const testUrl =
+          `http://localhost:${config.testServer.port}/` +
+          'extensions/test_file.html';
+      const tab = await openTab(testUrl);
+      injectScriptAndSendMessage(tab);
     });
   }
 ]);

chrome/test/data/extensions/api_test/debugger/background.js

@@ -16,6 +16,8 @@ var SILENT_FLAG_REQUIRED = "Cannot attach to this target unless " +
     "'silent-debugger-extension-api' flag is enabled.";
 var DETACHED_WHILE_HANDLING = "Detached while handling command.";
 
+let openTab;
+
 chrome.test.getConfig(config => chrome.test.runTests([
 
   function attachMalformedVersion() {
@@ -92,8 +94,9 @@ chrome.test.getConfig(config => chrome.test.runTests([
         fail("Debugger is not attached to the tab with id: " + tabId + "."));
   },
 
-  function closeTab() {
-    chrome.tabs.create({url:"inspected.html"}, function(tab) {
+  async function closeTab() {
+    ({openTab} = await import('/_test_resources/test_util/tabs_util.js'));
+    const tab = await openTab(chrome.runtime.getURL('inspected.html'));
     function onDetach(debuggee, reason) {
       chrome.test.assertEq(tab.id, debuggee.tabId);
       chrome.test.assertEq("target_closed", reason);
@@ -101,26 +104,24 @@ chrome.test.getConfig(config => chrome.test.runTests([
       chrome.test.succeed();
     }
 
-      var debuggee2 = {tabId: tab.id};
+    const debuggee2 = {tabId: tab.id};
     chrome.debugger.attach(debuggee2, protocolVersion, function() {
       chrome.debugger.onDetach.addListener(onDetach);
       chrome.tabs.remove(tab.id);
     });
-    });
   },
 
-  function attachToWebUI() {
-    chrome.tabs.create({url:"chrome://version"}, function(tab) {
-      var debuggee = {tabId: tab.id};
+  async function attachToWebUI() {
+    const tab = await openTab('chrome://version');
+    const debuggee = {tabId: tab.id};
     chrome.debugger.attach(debuggee, protocolVersion,
         fail("Cannot access a chrome:// URL"));
     chrome.tabs.remove(tab.id);
-    });
   },
 
-  function navigateToWebUI() {
-    chrome.tabs.create({url:"inspected.html"}, function(tab) {
-      var debuggee = {tabId: tab.id};
+  async function navigateToWebUI() {
+    const tab = await openTab(chrome.runtime.getURL('inspected.html'));
+    const debuggee = {tabId: tab.id};
     chrome.debugger.attach(debuggee, protocolVersion, function() {
       var responded = false;
 
@@ -145,12 +146,11 @@ chrome.test.getConfig(config => chrome.test.runTests([
       chrome.debugger.sendCommand(
         debuggee, "Page.navigate", {url: "chrome://version"}, onResponse);
     });
-    });
   },
 
-  function detachDuringCommand() {
-    chrome.tabs.create({url:"inspected.html"}, function(tab) {
-      var debuggee = {tabId: tab.id};
+  async function detachDuringCommand() {
+    const tab = await openTab(chrome.runtime.getURL('inspected.html'));
+    const debuggee = {tabId: tab.id};
     chrome.debugger.attach(debuggee, protocolVersion, function() {
       var responded = false;
 
@@ -172,7 +172,6 @@ chrome.test.getConfig(config => chrome.test.runTests([
       chrome.debugger.sendCommand(debuggee, "command", null, onResponse);
       chrome.debugger.detach(debuggee, onDetach);
     });
-    });
   },
 
   function attachToMissing() {
@@ -205,16 +204,13 @@ chrome.test.getConfig(config => chrome.test.runTests([
     chrome.debugger.detach(debuggee, pass());
   },
 
-  function createAndDiscoverTab() {
-    function onUpdated(tabId, changeInfo) {
-      if (changeInfo.status == 'loading')
-        return;
-      chrome.tabs.onUpdated.removeListener(onUpdated);
+  async function createAndDiscoverTab() {
+    const tab = await openTab(chrome.runtime.getURL('inspected.html'));
     chrome.debugger.getTargets(function(targets) {
       var page = targets.filter(
           function(t) {
             return t.type == 'page' &&
-                     t.tabId == tabId &&
+                   t.tabId == tab.id &&
                    t.title == 'Test page';
           })[0];
       if (page) {
@@ -224,9 +220,6 @@ chrome.test.getConfig(config => chrome.test.runTests([
         chrome.test.fail("Cannot discover a newly created tab");
       }
     });
-    }
-    chrome.tabs.onUpdated.addListener(onUpdated);
-    chrome.tabs.create({url: "inspected.html"});
   },
 
   function discoverWorker() {
@@ -250,9 +243,9 @@ chrome.test.getConfig(config => chrome.test.runTests([
     chrome.debugger.detach(debuggee, pass());
   },
 
-  function sendCommandDuringNavigation() {
-    chrome.tabs.create({url:"inspected.html"}, function(tab) {
-      var debuggee = {tabId: tab.id};
+  async function sendCommandDuringNavigation() {
+    const tab = await openTab(chrome.runtime.getURL('inspected.html'));
+    const debuggee = {tabId: tab.id};
 
     function checkError() {
       if (chrome.runtime.lastError) {
@@ -274,12 +267,11 @@ chrome.test.getConfig(config => chrome.test.runTests([
     }
 
     chrome.debugger.attach(debuggee, protocolVersion, onAttach);
-    });
   },
 
-  function sendCommandToDataUri() {
-    chrome.tabs.create({url:"data:text/html,<h1>hi</h1>"}, function(tab) {
-      var debuggee = {tabId: tab.id};
+  async function sendCommandToDataUri() {
+    const tab = await openTab('data:text/html,<h1>hi</h1>');
+    const debuggee = {tabId: tab.id};
 
     function checkError() {
       if (chrome.runtime.lastError) {
@@ -295,11 +287,10 @@ chrome.test.getConfig(config => chrome.test.runTests([
     }
 
     chrome.debugger.attach(debuggee, protocolVersion, onAttach);
-    });
   },
 
   // http://crbug.com/824174
-  function getResponseBodyInvalidChar() {
+  async function getResponseBodyInvalidChar() {
     let requestId;
 
     function onEvent(debuggeeId, message, params) {
@@ -319,7 +310,7 @@ chrome.test.getConfig(config => chrome.test.runTests([
     }
 
     chrome.debugger.onEvent.addListener(onEvent);
-    chrome.tabs.create({url: 'inspected.html'}, function(tab) {
+    const tab = await openTab(chrome.runtime.getURL('inspected.html'));
     const debuggee = {tabId: tab.id};
     chrome.debugger.attach(debuggee, protocolVersion, function() {
       chrome.debugger.sendCommand(
@@ -335,13 +326,12 @@ chrome.test.getConfig(config => chrome.test.runTests([
                 });
           });
     });
-    });
   },
 
-  function offlineErrorPage() {
+  async function offlineErrorPage() {
     const url = 'http://127.0.0.1//extensions/api_test/debugger/inspected.html';
-    chrome.tabs.create({url: url}, function(tab) {
-      var debuggee = {tabId: tab.id};
+    const tab = await openTab(url);
+    const debuggee = {tabId: tab.id};
     var finished = false;
     var failure = '';
     var expectingFrameNavigated = false;
@@ -417,7 +407,6 @@ chrome.test.getConfig(config => chrome.test.runTests([
     chrome.debugger.onDetach.addListener(onDetach);
     chrome.debugger.onEvent.addListener(onEvent);
     chrome.debugger.attach(debuggee, protocolVersion, onAttach);
-    });
   },
 
   function autoAttachToOOPIF() {

chrome/test/data/extensions/api_test/debugger_extension/background.js

@@ -10,13 +10,13 @@ var protocolVersion = "1.3";
 
 chrome.test.runTests([
 
-  function attachToWebUI() {
-    chrome.tabs.create({url:"chrome://version"}, function(tab) {
-      var debuggee = {tabId: tab.id};
+  async function attachToWebUI() {
+    const {openTab} = await import('/_test_resources/test_util/tabs_util.js');
+    const tab = await openTab('chrome://version');
+    const debuggee = {tabId: tab.id};
     chrome.debugger.attach(debuggee, protocolVersion,
                            fail("Cannot attach to this target."));
     chrome.tabs.remove(tab.id);
-    });
   },
 
   function attach() {

chrome/test/data/extensions/api_test/debugger_file_access/background.js

@@ -10,10 +10,12 @@ function checkUrlsEqual(expected, actual) {
                        new URL(actual).href);
 }
 
-function runNotAllowedTest(method, params, expectAllowed) {
+let openTab;
+
+async function runNotAllowedTest(method, params, expectAllowed) {
   const NOT_ALLOWED = "Not allowed";
-  chrome.tabs.create({url: 'dummy.html'}, function(tab) {
-    var debuggee = {tabId: tab.id};
+  const tab = await openTab(chrome.runtime.getURL('dummy.html'));
+  const debuggee = {tabId: tab.id};
   chrome.debugger.attach(debuggee, '1.2', function() {
     chrome.test.assertNoLastError();
     chrome.debugger.sendCommand(debuggee, method, params, onResponse);
@@ -33,7 +35,6 @@ function runNotAllowedTest(method, params, expectAllowed) {
       });
     }
   });
-  });
 }
 
 (async () => {
@@ -42,7 +43,8 @@ function runNotAllowedTest(method, params, expectAllowed) {
                  });
   const fileUrl = config.testDataDirectory + '/../body1.html';
   const expectFileAccess = !!config.customArg;
-  const { openTab } = await import('/_test_resources/test_util/tabs_util.js');
+
+  ({ openTab } = await import('/_test_resources/test_util/tabs_util.js'));
 
   console.log(fileUrl);
 

chrome/test/data/extensions/api_test/debugger_inspect_worker/background.js

@@ -6,8 +6,8 @@ const protocolVersion = '1.3';
 
 chrome.test.getConfig(config => chrome.test.runTests([
   async function testInspectWorkerForbidden() {
-    const tab = await new Promise(resolve =>
-        chrome.tabs.create({url: config.customArg}, resolve));
+    const {openTab} = await import('/_test_resources/test_util/tabs_util.js');
+    const tab = await openTab(config.customArg);
     const debuggee = {tabId: tab.id};
     await new Promise(resolve =>
         chrome.debugger.attach(debuggee, protocolVersion, resolve));