Justin Lulejian (Gerrit)

unread,

Jun 25, 2026, 7:31:25 PM (3 days ago) Jun 25

to Devlin Cronin, Tim, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, msrame...@chromium.org, antoniosarto...@chromium.org, rginda...@chromium.org, mkwst+w...@chromium.org, arthursonzog...@chromium.org, dullweb...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Tim

Justin Lulejian voted and added 1 comment

Votes added by Justin Lulejian

Auto-Submit	+1
Commit-Queue	+1

1 comment

Patchset-level comments

File-level comment, Patchset 11:

Tim . resolved

Hmmm, this way of using choices and repeating different arguments in different positions seems like quite a hack from its normal intended use. I get that you do the custom work to disambiguate which signature you think has been passed in the custom bindings, but it feels bad to me that the schema says `chrome.test.assertThrows(chrome.tabs.create, "foo", "foo", "foo")` would be a valid call.
I'm going to pass this one over to Devlin for now to get their take on the general approach and if they have any alternative ideas on how to do this best.
rdevlin.cronin@: If you think the overall approach here is fine, feel free to pass it back to me to take a closer look.

Justin Lulejian

For extra context Devlin: the design said to move the old signature to assertThrowsSelf and then have the browser.test proposal's assertThrows signature be the default. But it seemed not too complex in practice to just support both so I tried this way.
There isn't much use of assertThrows currently so it wouldn't be too onerous to still do assertThrowsSelf. I could also try to just coerce the current usages to use the browser.test proposal's assertThrows signature so that we only have one definition. LMK which would be more preferred.

Justin Lulejian

I ended up attempting (in Patchset 12) to just only use the browser.test proposal's version (`browser.test.assertThrows(fn, expectedError, message)`) and updating existing tests to use it. Besides the large diff due to updating the tests, it ends up being a lot simpler.
Devlin: lmk which path you'd prefer us to take and I'll update the commit message as well if necessary.

Justin Lulejian

(actually the latest patchset, 12 was failing with a couple of tests I missed).

Devlin Cronin

Thanks for the details!
Given there aren't *too* many callsites here to update, I'd prefer we just update them all to use the new version (as appears to have been done here).

Justin Lulejian

Thanks Devlin! I went ahead and updated the commit message to reflect the latest patch.

Open in Gerrit

Related details

Attention is currently required from:

Tim

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Tim (Gerrit)

unread,

Jun 26, 2026, 2:10:49 PM (3 days ago) Jun 26

to Justin Lulejian, Devlin Cronin, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, msrame...@chromium.org, antoniosarto...@chromium.org, rginda...@chromium.org, mkwst+w...@chromium.org, arthursonzog...@chromium.org, dullweb...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Justin Lulejian

Tim voted and added 3 comments

Votes added by Tim

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 14 (Latest):

Tim . resolved

Thanks for iterating on this one and going through the process of updating all the callers. LGTM, just a couple more test cases I think we can add. It also might be worth throwing this one through the mega CQ, just in case there's some tests only run on random bots using the old signature.

File chrome/browser/extensions/api/test/apitest_apitest.cc

Line 749, Patchset 14 (Latest):// `assertThrows(fn, expectedError?, message?)`.

Tim . unresolved

I'm not seeing any cases in this file which actually use the custom error message and validate the ResultCatcher sees it on failure. Can we add coverage for that?

Line 787, Patchset 14 (Latest):IN_PROC_BROWSER_TEST_F(TestAPITest, AssertThrows_Success_WithArrowFunctions) {

Tim . unresolved

Want to throw in a bind case as well, since it's mentioned in the .md file.

Open in Gerrit

Related details

Attention is currently required from:

Justin Lulejian

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Justin Lulejian (Gerrit)

unread,

Jun 26, 2026, 4:28:57 PM (2 days ago) Jun 26

to Tim, Devlin Cronin, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, msrame...@chromium.org, antoniosarto...@chromium.org, rginda...@chromium.org, mkwst+w...@chromium.org, arthursonzog...@chromium.org, dullweb...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Devlin Cronin

Justin Lulejian voted and added 3 comments

Votes added by Justin Lulejian

Auto-Submit

+1

3 comments

File chrome/browser/extensions/api/browsing_data/browsing_data_test.cc

Line 427, Patchset 13: {'origins': []}, {'cookies': true}),

Devlin Cronin . resolved

for many of these, IMO, it's slightly cleaner to use bind() rather than wrapping in a function call:
```
chrome.browsingData.remove.bind(null, {'origins': []}, {'cookies': true})
```
(I think it's more obvious what function is expected to throw and it cleans up the stack trace a little, since there isn't an anonymous wrapper function in the trace)
But I don't feel super strongly. Feel free to use your best judgment.

Justin Lulejian

Stack trace clarity is the better way to go IMO so I'll change as many cases as I can to use bind() instead. I'll mention both ways in the testing doc so that future readers know that it is possible though!

File chrome/browser/extensions/api/test/apitest_apitest.cc

Line 749, Patchset 14:// `assertThrows(fn, expectedError?, message?)`.

Tim . resolved

I'm not seeing any cases in this file which actually use the custom error message and validate the ResultCatcher sees it on failure. Can we add coverage for that?

Justin Lulejian

Added as `AssertThrows_Failure_CustomMessage` test case. I tried to do it with ResultCatcher, but we don't get individual test error message to it (example: `EXPECT_EQ("Failed 1 of 1 tests", result_catcher.message())).` is the best we can do). So I did it instead by injecting a custom chrome.test.fail to test that.

Line 787, Patchset 14:IN_PROC_BROWSER_TEST_F(TestAPITest, AssertThrows_Success_WithArrowFunctions) {

Tim . resolved

Want to throw in a bind case as well, since it's mentioned in the .md file.

Justin Lulejian

Added as `AssertThrows_Success_WithBind `.

Open in Gerrit

Related details

Attention is currently required from:

Devlin Cronin

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Justin Lulejian (Gerrit)

unread,

Jun 26, 2026, 4:29:06 PM (2 days ago) Jun 26

to Tim, Devlin Cronin, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, msrame...@chromium.org, antoniosarto...@chromium.org, rginda...@chromium.org, mkwst+w...@chromium.org, arthursonzog...@chromium.org, dullweb...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Justin Lulejian voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Justin Lulejian (Gerrit)

unread,

Jun 26, 2026, 4:29:43 PM (2 days ago) Jun 26

to Tim, Devlin Cronin, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, msrame...@chromium.org, antoniosarto...@chromium.org, rginda...@chromium.org, mkwst+w...@chromium.org, arthursonzog...@chromium.org, dullweb...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Justin Lulejian voted Commit-Queue+1

Commit-Queue

+1

satisfied_requirement

open

diffy

Justin Lulejian (Gerrit)

unread,

Jun 26, 2026, 4:30:55 PM (2 days ago) Jun 26

to Tim, Devlin Cronin, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, msrame...@chromium.org, antoniosarto...@chromium.org, rginda...@chromium.org, mkwst+w...@chromium.org, arthursonzog...@chromium.org, dullweb...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Justin Lulejian voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Jun 26, 2026, 5:51:09 PM (2 days ago) Jun 26

to Justin Lulejian, Tim, Devlin Cronin, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, msrame...@chromium.org, antoniosarto...@chromium.org, rginda...@chromium.org, mkwst+w...@chromium.org, arthursonzog...@chromium.org, dullweb...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

14 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: chrome/test/data/extensions/api_test/tabs/prerendering_into_new_tab/test.js
Insertions: 3, Deletions: 2.

@@ -94,9 +94,10 @@
 // Checks that manifest v2 does not support `documentId`.
 async function testGetTitleByDocumentId() {
   await setup();
+  // Verify `executeScript` throws when using `documentId` in manifest v2.
   chrome.test.assertThrows(
-      () => chrome.tabs.executeScript(
-          prerenderingTabId,
+      chrome.tabs.executeScript.bind(
+          null, prerenderingTabId,
           {documentId: prerenderingDocumentId, code: 'document.title;'},
           results => chrome.test.fail('should not succeed.')),
       'Error in invocation of tabs.executeScript(optional integer tabId, ' +
```
```
The name of the file: chrome/test/data/extensions/api_test/protocol_handler/test_registration.js
Insertions: 26, Deletions: 22.

@@ -71,46 +71,50 @@
 
   chrome.test.runTests([
     function invalidScheme() {
+      // Verify `registerProtocolHandler` throws on invalid scheme syntax.
       chrome.test.assertThrows(
-          () => navigator.registerProtocolHandler(
-              'ext+@', SAME_ORIGIN_CHROME_EXTENSION_URL, TITLE),
+          navigator.registerProtocolHandler.bind(
+              navigator, 'ext+@', SAME_ORIGIN_CHROME_EXTENSION_URL, TITLE),
           MESSAGE_INVALID_URI_SYNTAX);
       chrome.test.assertThrows(
-          () => navigator.registerProtocolHandler(
-              'unknownscheme', SAME_ORIGIN_CHROME_EXTENSION_URL, TITLE),
+          navigator.registerProtocolHandler.bind(
+              navigator, 'unknownscheme', SAME_ORIGIN_CHROME_EXTENSION_URL,
+              TITLE),
           MESSAGE_NOT_SAFELISTED);
       chrome.test.assertThrows(
-          () => navigator.registerProtocolHandler(
-              'ext+', SAME_ORIGIN_CHROME_EXTENSION_URL, TITLE),
+          navigator.registerProtocolHandler.bind(
+              navigator, 'ext+', SAME_ORIGIN_CHROME_EXTENSION_URL, TITLE),
           MESSAGE_EXT_PLUS_SCHEME);
       chrome.test.succeed();
     },
 
     function invalidURL() {
+      // Verify `registerProtocolHandler` throws on invalid URL syntax.
       chrome.test.assertThrows(
-          () => navigator.registerProtocolHandler(
-              'mailto', 'invalidurl://%s', TITLE),
+          navigator.registerProtocolHandler.bind(
+              navigator, 'mailto', 'invalidurl://%s', TITLE),
           MESSAGE_INVALID_SCHEME);
       chrome.test.assertThrows(
-          () => navigator.registerProtocolHandler(
-              'mailto', `blob:${SAME_ORIGIN_CHROME_EXTENSION_URL}`, TITLE),
-          MESSAGE_INVALID_SCHEME);
-      chrome.test.assertThrows(
-          () => navigator.registerProtocolHandler(
-              'mailto', 'data:text/html,Hello?url=%s', TITLE),
-          MESSAGE_INVALID_SCHEME);
-      chrome.test.assertThrows(
-          () => navigator.registerProtocolHandler(
-              'mailto', `filesystem:${SAME_ORIGIN_CHROME_EXTENSION_URL}`,
+          navigator.registerProtocolHandler.bind(
+              navigator, 'mailto', `blob:${SAME_ORIGIN_CHROME_EXTENSION_URL}`,
               TITLE),
           MESSAGE_INVALID_SCHEME);
       chrome.test.assertThrows(
-          () => navigator.registerProtocolHandler(
-              'mailto', chrome.runtime.getURL('xhr.txt'), TITLE),
+          navigator.registerProtocolHandler.bind(
+              navigator, 'mailto', 'data:text/html,Hello?url=%s', TITLE),
+          MESSAGE_INVALID_SCHEME);
+      chrome.test.assertThrows(
+          navigator.registerProtocolHandler.bind(
+              navigator, 'mailto',
+              `filesystem:${SAME_ORIGIN_CHROME_EXTENSION_URL}`, TITLE),
+          MESSAGE_INVALID_SCHEME);
+      chrome.test.assertThrows(
+          navigator.registerProtocolHandler.bind(
+              navigator, 'mailto', chrome.runtime.getURL('xhr.txt'), TITLE),
           MESSAGE_MISSING_PERCENT);
       chrome.test.assertThrows(
-          () =>
-              navigator.registerProtocolHandler('mailto', 'https://%s', TITLE),
+          navigator.registerProtocolHandler.bind(
+              navigator, 'mailto', 'https://%s', TITLE),
           MESSAGE_INVALID_URL_CREATED);
       chrome.test.succeed();
     },
```
```
The name of the file: chrome/browser/extensions/api/test/apitest_apitest.cc
Insertions: 74, Deletions: 0.

@@ -808,6 +808,34 @@
   EXPECT_TRUE(result_catcher.GetNextResult());
 }
 
+// Verifies that `chrome.test.assertThrows` succeeds when using `bind`
+// to wrap function calls that require arguments.
+IN_PROC_BROWSER_TEST_F(TestAPITest, AssertThrows_Success_WithBind) {
+  ResultCatcher result_catcher;
+  static constexpr char kBackgroundJs[] =
+      R"(chrome.test.runTests([
+           /**
+            * Tests `assertThrows` succeeds with `bind` and args.
+            */
+           function testAssertThrowsSuccessWithBind() {
+             const obj = {
+               func: function(a, b) {
+                 if (a + b > 10) throw new Error('too big');
+               }
+             };
+             // Verify `assertThrows` succeeds with exact string match.
+             chrome.test.assertThrows(obj.func.bind(obj, 5, 6), 'too big');
+             // Verify `assertThrows` succeeds with regex match.
+             chrome.test.assertThrows(obj.func.bind(obj, 5, 6), /too+/);
+             chrome.test.succeed();
+           }
+         ]);)";
+
+  // Load the test extension and verify all tests succeed.
+  ASSERT_TRUE(LoadExtensionWithScript(kBackgroundJs));
+  EXPECT_TRUE(result_catcher.GetNextResult());
+}
+
 // Verifies that `chrome.test.assertThrows` fails when the passed function
 // does not throw any error.
 IN_PROC_BROWSER_TEST_F(TestAPITest, AssertThrows_Failure_NoThrow) {
@@ -875,6 +903,52 @@
   EXPECT_FALSE(listener.was_satisfied());
 }
 
+// Verifies that `chrome.test.assertThrows` outputs the custom error message
+// when the assertion fails.
+IN_PROC_BROWSER_TEST_F(TestAPITest, AssertThrows_Failure_CustomMessage) {
+  ResultCatcher result_catcher;
+  static constexpr char kBackgroundJs[] =
+      R"(chrome.test.runTests([
+           /**
+            * Tests `assertThrows` includes custom message on failure.
+            */
+           function testAssertThrowsFailureCustomMessage() {
+             // Intercept `chrome.test.fail` to capture failure messages.
+             const originalFail = chrome.test.fail;
+             let failureMessage = '';
+             chrome.test.fail = function(message) {
+               failureMessage = message;
+             };
+
+             // Verify failure message when the function does not throw.
+             chrome.test.assertThrows(
+                 () => {}, /* expectedError */ 'expected error',
+                 /* message */ 'Custom failure message 1');
+             chrome.test.assertEq(
+                 'Custom failure message 1\nDid not throw error: () => {}',
+                 failureMessage);
+
+             // Verify failure message when the function throws the wrong error.
+             chrome.test.assertThrows(
+                 () => { throw new Error('actual'); },
+                 /* expectedError */ 'expected',
+                 /* message */ 'Custom failure message 2');
+             chrome.test.assertEq(
+                 'Custom failure message 2\n' +
+                     'Expected error: "expected", actual: "actual"',
+                 failureMessage);
+
+             // Restore the original failure handler and succeed the test.
+             chrome.test.fail = originalFail;
+             chrome.test.succeed();
+           }
+         ]);)";
+
+  // Load the test extension and verify all tests succeed.
+  ASSERT_TRUE(LoadExtensionWithScript(kBackgroundJs));
+  EXPECT_TRUE(result_catcher.GetNextResult());
+}
+
 // Verifies that `chrome.test.assertThrows` succeeds when the function throws
 // falsy values (like `null` or `undefined`), both with and without
 // `expectedError` matching.
```
```
The name of the file: chrome/test/data/extensions/api_test/pdf_viewer_private/text_info_test.js
Insertions: 19, Deletions: 7.

@@ -48,8 +48,10 @@
     div.value = 'Test text';
     document.body.appendChild(div);
 
+    // Verify `getTextInfo` throws when passed a non-textarea element.
     await chrome.test.assertThrows(
-        () => chrome.pdfViewerPrivate.getTextInfo(div, []),
+        chrome.pdfViewerPrivate.getTextInfo.bind(
+            null, div, /* knownFontIds */[]),
         new RegExp('Value must be an instance of HTMLTextAreaElement'));
 
     chrome.test.succeed();
@@ -60,16 +62,20 @@
     textarea.value = 'Test text';
     document.body.appendChild(textarea);
 
+    // Verify `getTextInfo` throws when called with no arguments.
     await chrome.test.assertThrows(
-        () => chrome.pdfViewerPrivate.getTextInfo(),
+        chrome.pdfViewerPrivate.getTextInfo.bind(null),
         new RegExp('No matching signature'));
 
+    // Verify `getTextInfo` throws when called with missing second argument.
     await chrome.test.assertThrows(
-        () => chrome.pdfViewerPrivate.getTextInfo(textarea),
+        chrome.pdfViewerPrivate.getTextInfo.bind(null, textarea),
         new RegExp('No matching signature'));
 
+    // Verify `getTextInfo` throws when called with extra third argument.
     await chrome.test.assertThrows(
-        () => chrome.pdfViewerPrivate.getTextInfo(textarea, [], 123),
+        chrome.pdfViewerPrivate.getTextInfo.bind(
+            null, textarea, /* knownFontIds */[], /* callback */ 123),
         new RegExp('No matching signature'));
 
     chrome.test.succeed();
@@ -80,16 +86,22 @@
     textarea.value = 'Test text';
     document.body.appendChild(textarea);
 
+    // Verify `getTextInfo` throws when second argument is an integer instead of
+    // array.
     await chrome.test.assertThrows(
-        () => chrome.pdfViewerPrivate.getTextInfo(textarea, 1337),
+        chrome.pdfViewerPrivate.getTextInfo.bind(
+            null, textarea, /* knownFontIds */ 1337),
         new RegExp('No matching signature'));
 
     await chrome.test.assertPromiseRejects(
-        chrome.pdfViewerPrivate.getTextInfo(textarea, [-10]),
+        chrome.pdfViewerPrivate.getTextInfo(textarea, /* knownFontIds */[-10]),
         new RegExp('elements must be uint32'));
 
+    // Verify `getTextInfo` throws when array element is an array instead of
+    // integer.
     await chrome.test.assertThrows(
-        () => chrome.pdfViewerPrivate.getTextInfo(textarea, [10, []]),
+        chrome.pdfViewerPrivate.getTextInfo.bind(
+            null, textarea, /* knownFontIds */[10, []]),
         new RegExp('Invalid type: expected integer'));
 
     chrome.test.succeed();
```
```
The name of the file: chrome/test/data/extensions/api_test/file_browser/format_test/test.js
Insertions: 3, Deletions: 2.

@@ -19,8 +19,9 @@
 
     // Test an unsupported filesystem.
     chrome.test.assertThrows(
-        () => chrome.fileManagerPrivate.formatVolume(
-            'removable:mount_path3', 'invalid-fs', 'NEWLABEL3'),
+        chrome.fileManagerPrivate.formatVolume.bind(
+            null, /* volumeId */ 'removable:mount_path3',
+            /* filesystem */ 'invalid-fs', /* volumeLabel */ 'NEWLABEL3'),
         expectedError);
 
     // This test is also checked on the C++ side, which tests that
```
```
The name of the file: chrome/test/data/extensions/api_test/printer_provider/request_printers_second/test.js
Insertions: 4, Deletions: 2.

@@ -21,8 +21,9 @@
       }
 
       if (test === 'INVALID_VALUE') {
+        // Verify `callback` throws when passed an invalid argument type.
         chrome.test.assertThrows(
-            () => callback('XXX'),
+            callback.bind(null, /* printerInfo */ 'XXX'),
             'Error validating the callback argument: ' +
                 'Expected an object, found string.');
       } else if (test === 'EMPTY') {
@@ -42,8 +43,9 @@
         ]);
       }
 
+      // Verify `callback` throws when called more than once.
       chrome.test.assertThrows(
-          () => callback(),
+          callback.bind(null),
           'Event callback must not be called more than once.');
 
       chrome.test.succeed();
```
```
The name of the file: chrome/browser/extensions/api/omnibox/omnibox_api_interactive_test.cc
Insertions: 4, Deletions: 1.

@@ -779,8 +779,11 @@
                  content: 'content',
              };
              const expectedError = /Unexpected property: 'content'./;
+             // Verify `chrome.omnibox.setDefaultSuggestion` throws on invalid
+             // suggestion property.
              chrome.test.assertThrows(
-                 () => chrome.omnibox.setDefaultSuggestion(invalidSuggestion),
+                 chrome.omnibox.setDefaultSuggestion.bind(
+                     null, invalidSuggestion),
                  expectedError);
              chrome.test.succeed();
            },
```
```
The name of the file: chrome/test/data/extensions/api_test/printer_provider/request_printers/test.js
Insertions: 25, Deletions: 16.

@@ -33,33 +33,41 @@
       }
 
       if (test === 'NOT_ARRAY') {
+        // Verify `callback` throws when passed a non-array argument.
         chrome.test.assertThrows(
-            () => callback('XXX'), 'No matching signature.');
+            callback.bind(null, /* printerInfo */ 'XXX'),
+            'No matching signature.');
       } else if (test === 'INVALID_PRINTER_TYPE') {
         const expectedError =
             `Error at parameter 'printerInfo': Error at index 1: ` +
             'Invalid type: expected printerProvider.PrinterInfo, ' +
             'found string.';
+        // Verify `callback` throws when printer info list contains invalid
+        // type.
         chrome.test.assertThrows(
-            () => callback([
-              {
-                id: 'printer1',
-                name: 'Printer 1',
-                description: 'Test printer',
-              },
-              'printer2',
-            ]),
+            callback.bind(
+                null,
+                [
+                  {
+                    id: 'printer1',
+                    name: 'Printer 1',
+                    description: 'Test printer',
+                  },
+                  'printer2',
+                ]),
             expectedError);
       } else if (test === 'INVALID_PRINTER') {
         const expectedError = `Error at parameter 'printerInfo': ` +
             `Error at index 0: Unexpected property: 'unsupported'.`;
+        // Verify `callback` throws when printer info object has unsupported
+        // property.
         chrome.test.assertThrows(
-            () => callback([{
-              id: 'printer1',
-              name: 'Printer 1',
-              description: 'Test printer',
-              unsupported: 'print',
-            }]),
+            callback.bind(null, [{
+                            id: 'printer1',
+                            name: 'Printer 1',
+                            description: 'Test printer',
+                            unsupported: 'print',
+                          }]),
             expectedError);
       } else {
         chrome.test.assertEq('OK', test);
@@ -76,8 +84,9 @@
         ]);
       }
 
+      // Verify `callback` throws when called more than once.
       chrome.test.assertThrows(
-          () => callback(),
+          callback.bind(null),
           'Event callback must not be called more than once.');
 
       chrome.test.succeed();
```
```
The name of the file: chrome/test/data/extensions/api_test/extension_action/title/background.js
Insertions: 3, Deletions: 2.

@@ -27,9 +27,10 @@
 
     // Invalid titles throw and do not alter the histogram.
     chrome.test.assertThrows(
-        () => chrome.action.setTitle({title: null}), invalidTitleError);
+        chrome.action.setTitle.bind(null, {title: null}), invalidTitleError);
     chrome.test.assertThrows(
-        () => chrome.action.setTitle({title: undefined}), invalidTitleError);
+        chrome.action.setTitle.bind(null, {title: undefined}),
+        invalidTitleError);
     chrome.test.assertEq('A', await chrome.action.getTitle({}));
 
     // Increments 60-byte string bucket.
```
```
The name of the file: chrome/test/data/extensions/api_test/bindings/unavailable_bindings_never_registered/background.js
Insertions: 4, Deletions: 2.

@@ -42,9 +42,11 @@
               assertStorageIsRegistered();
 
               // Although storage should throw an error on use since it's
-              // removed.
+              // Verify `chrome.storage.local.get` throws when storage
+              // permission is removed.
               chrome.test.assertThrows(
-                  () => chrome.storage.local.get(function() {}),
+                  chrome.storage.local.get.bind(
+                      chrome.storage.local, function() {}),
                   `'storage.get' is not available in this context.`);
 
               chrome.test.succeed();
```
```
The name of the file: extensions/test/data/api_test/messaging/send_message_polyfill_unserializable/background.js
Insertions: 6, Deletions: 2.

@@ -7,13 +7,17 @@
       // JS functions are not JSON serializable. The below causes extensions v8
       // C++ logic to throw a TypeError in this context.
     case 'respond synchronously with an unserializable value':
+      // Verify `sendResponse` throws when responding synchronously with an
+      // unserializable function.
       chrome.test.assertThrows(
-          () => sendResponse(() => {}), 'Could not serialize message.');
+          sendResponse.bind(null, () => {}), 'Could not serialize message.');
       break;
     case 'respond asynchronously with an unserializable value':
       setTimeout(() => {
+        // Verify `sendResponse` throws when responding asynchronously with an
+        // unserializable function.
         chrome.test.assertThrows(
-            () => sendResponse(() => {}), 'Could not serialize message.');
+            sendResponse.bind(null, () => {}), 'Could not serialize message.');
       }, 1);
       return true;
     default:
```
```
The name of the file: chrome/browser/extensions/user_script_world_browsertest.cc
Insertions: 7, Deletions: 2.

@@ -465,14 +465,19 @@
          let errorMsg = /User scripts may not message external extensions./;
          chrome.test.runTests([
            function sendMessageToExternalExtensionThrowsError() {
+             // Verify `chrome.runtime.sendMessage` throws when messaging
+             // external extensions.
              chrome.test.assertThrows(
-                 () => chrome.runtime.sendMessage(targetId, 'test message'),
+                 chrome.runtime.sendMessage.bind(
+                     null, targetId, /* message */ 'test message'),
                  errorMsg);
              chrome.test.succeed();
            },
            function connectToExternalExtensionThrowsError() {
+             // Verify `chrome.runtime.connect` throws when connecting to
+             // external extensions.
              chrome.test.assertThrows(
-                 () => chrome.runtime.connect(targetId), errorMsg);
+                 chrome.runtime.connect.bind(null, targetId), errorMsg);
              chrome.test.succeed();
            },
          ]);
```
```
The name of the file: chrome/test/data/extensions/api_test/printer_provider/request_capability/test.js
Insertions: 5, Deletions: 2.

@@ -28,8 +28,10 @@
       }
 
       if (test === 'INVALID_VALUE') {
+        // Verify `callback` throws when passed an invalid argument type.
         chrome.test.assertThrows(
-            () => callback('XXX'), 'No matching signature.');
+            callback.bind(null, /* capabilities */ 'XXX'),
+            'No matching signature.');
       } else if (test === 'EMPTY') {
         callback({});
       } else {
@@ -37,8 +39,9 @@
         callback({capability: 'value'});
       }
 
+      // Verify `callback` throws when called more than once.
       chrome.test.assertThrows(
-          () => callback({cap: 'value'}),
+          callback.bind(null, /* capabilities */ {cap: 'value'}),
           'Event callback must not be called more than once.');
 
       chrome.test.succeed();
```
```
The name of the file: chrome/browser/extensions/api/browsing_data/browsing_data_test.cc
Insertions: 5, Deletions: 2.

@@ -420,11 +420,14 @@
           chrome.test.succeed();
       },
       async function emptyOriginsFilter() {
+          // Define the expected error regex for empty `origins` list.
           const expectedError = new RegExp(
               '.* Array must have at least 1 items; found 0.');
+          // Verify `chrome.browsingData.remove` throws when `origins` is empty.
           chrome.test.assertThrows(
-              () => chrome.browsingData.remove(
-                  {'origins': []}, {'cookies': true}),
+              chrome.browsingData.remove.bind(
+                  null, /* options */ {'origins': []},
+                  /* dataToRemove */ {'cookies': true}),
               expectedError);
           chrome.test.succeed();
       },
```
```
The name of the file: chrome/test/data/extensions/api_test/printer_provider/usb_printers/test.js
Insertions: 2, Deletions: 1.

@@ -25,8 +25,9 @@
         });
       }
 
+      // Verify `callback` throws when called more than once.
       chrome.test.assertThrows(
-          () => callback(),
+          callback.bind(null),
           'Event callback must not be called more than once.');
 
       chrome.test.succeed();
```
```
The name of the file: extensions/docs/testing_api.md
Insertions: 19, Deletions: 15.

@@ -301,36 +301,40 @@
 Examples:
 ```js
 // Assert that any error is thrown:
-chrome.test.assertThrows(() => {
-  throw new Error('some error');
-});
+chrome.test.assertThrows(myObject.myFunction.bind(myObject, arg1));
 
 // Assert that an error with a specific message is thrown:
-chrome.test.assertThrows(() => {
-  throw new Error('specific error');
-}, 'specific error');
+chrome.test.assertThrows(
+  myObject.myFunction.bind(myObject, arg1),
+  /* expectedError */ 'specific error'
+);
 
 // Assert that an error matching a `RegExp` is thrown:
-chrome.test.assertThrows(() => {
-  JSON.parse('invalid-json');
-}, /Unexpected token/);
+chrome.test.assertThrows(
+  JSON.parse.bind(null, /* text */ 'invalid-json'),
+  /* expectedError */ /Unexpected token/
+);
 
 // Assert that an error is thrown with a custom failure message:
-chrome.test.assertThrows(() => {
-  throw new Error('wrong error');
-}, 'expected error', 'My custom failure message');
+chrome.test.assertThrows(
+  myObject.myFunction.bind(myObject, arg1),
+  /* expectedError */ 'expected error',
+  /* message */ 'My custom failure message'
+);
 
-// Assert that a function called with a specific context and arguments throws:
+// Assert that a function called with a specific context and arguments throws.
+// Note: Using an arrow function wrapper can reduce stack trace clarity
+// (since an anonymous function will show up in the stack trace when doing this).
 chrome.test.assertThrows(
   () => myObject.myFunction(arg1, arg2),
-  'Expected Error Message'
+  /* expectedError */ 'Expected Error Message'
 );
 
 // Assert that a function called with a specific context and arguments throws
 // (using `bind`):
 chrome.test.assertThrows(
   myObject.myFunction.bind(myObject, arg1, arg2),
-  'Expected Error Message'
+  /* expectedError */ 'Expected Error Message'
 );
 ```
 
```
```
The name of the file: chrome/test/data/extensions/api_test/isolated_world_csp/mv3/eval.js
Insertions: 3, Deletions: 1.

@@ -6,8 +6,10 @@
   window.foo = 2;
   const expectedExceptionMessage = 'Evaluating a string as JavaScript ' +
       'violates the following Content Security Policy directive';
+  // Verify `eval` throws when violating Content Security Policy.
   chrome.test.assertThrows(
-      () => eval('window.foo = 3;'), new RegExp(expectedExceptionMessage));
+      eval.bind(null, /* script */ 'window.foo = 3;'),
+      new RegExp(expectedExceptionMessage));
   chrome.test.assertEq(2, window.foo);
   chrome.test.succeed();
 }]);
```
```
The name of the file: chrome/test/data/extensions/api_test/tabs/prerendering/test.js
Insertions: 4, Deletions: 2.

@@ -62,9 +62,11 @@
 // Checks if manifest v2 doesn't support `documentId`.
 async function testGetTitleByDocumentId() {
   await setup();
+  // Verify `executeScript` throws when using `documentId` in manifest v2.
   chrome.test.assertThrows(
-      () => chrome.tabs.executeScript(
-          tabId, {documentId: prerenderingDocumentId, code: 'document.title;'},
+      chrome.tabs.executeScript.bind(
+          null, tabId,
+          {documentId: prerenderingDocumentId, code: 'document.title;'},
           results => chrome.test.fail('should not succeed.')),
       'Error in invocation of tabs.executeScript(optional integer tabId, ' +
           'extensionTypes.InjectDetails details, optional function ' +
```
```
The name of the file: chrome/test/data/extensions/api_test/permissions/optional/background.js
Insertions: 4, Deletions: 1.

@@ -195,8 +195,11 @@
                 assertTrue(
                     typeof chrome.bookmarks === 'object' &&
                     chrome.bookmarks != null);
+                // Verify `chrome.bookmarks.getTree` throws when bookmarks
+                // permission is removed.
                 assertThrows(
-                    () => chrome.bookmarks.getTree(function() {}),
+                    chrome.bookmarks.getTree.bind(
+                        chrome.bookmarks, function() {}),
                     `'bookmarks.getTree' is not available in this context.`);
               },
               ));
```
```
The name of the file: chrome/test/data/extensions/api_test/printer_provider/request_print/test.js
Insertions: 6, Deletions: 3.

@@ -21,8 +21,9 @@
 // invocation throws an exception.
 function wrapPrintCallback(callback, returnValue) {
   callback(returnValue);
+  // Verify `callback` throws when called more than once.
   chrome.test.assertThrows(
-      () => callback('OK'),
+      callback.bind(null, /* result */ 'OK'),
       'Event callback must not be called more than once.');
 }
 
@@ -43,13 +44,15 @@
         case 'IGNORE_CALLBACK':
           break;
         case 'ASYNC_RESPONSE':
-          setTimeout(callback.bind(null, 'OK'), 0);
+          setTimeout(callback.bind(null, /* result */ 'OK'), 0);
           break;
         case 'INVALID_VALUE':
           const expectedError =
               `Error at parameter 'result': Value must be one of ` +
               'FAILED, INVALID_DATA, INVALID_TICKET, OK.';
-          chrome.test.assertThrows(() => callback('XXX'), expectedError);
+          // Verify `callback` throws when passed an invalid result value.
+          chrome.test.assertThrows(
+              callback.bind(null, /* result */ 'XXX'), expectedError);
           break;
         case 'FAILED':
         case 'INVALID_TICKET':
```
```
The name of the file: chrome/test/data/extensions/api_test/scripting/main_frame/worker.js
Insertions: 3, Deletions: 1.

@@ -432,8 +432,10 @@
         'scripting.ScriptInjection injection, optional function callback): ' +
         'Error at parameter \'injection\': Error at property \'args\': ' +
         'Error at index 0: Value is unserializable.';
+    // Verify `executeScript` throws when passed an unserializable function
+    // argument.
     chrome.test.assertThrows(
-        () => chrome.scripting.executeScript({
+        chrome.scripting.executeScript.bind(null, {
           target: {
             tabId: tab.id,
           },
```
```
The name of the file: chrome/test/data/extensions/api_test/dom/closed_shadow_root_from_content_script/content-script.js
Insertions: 4, Deletions: 1.

@@ -6,8 +6,11 @@
   const host = document.getElementById('host');
   const shadowRoot = chrome.dom.openOrClosedShadowRoot(host);
   chrome.test.assertEq(shadowRoot.childElementCount, 2);
+  // Verify `chrome.dom.openOrClosedShadowRoot` throws when passed a
+  // non-HTMLElement object.
   chrome.test.assertThrows(
-      () => chrome.dom.openOrClosedShadowRoot(new Object()),
+      chrome.dom.openOrClosedShadowRoot.bind(null, /* element */ new Object()),
+      /* expectedError */
       'Error in invocation of dom.openOrClosedShadowRoot(HTMLElement ' +
           'element): ');
   chrome.test.succeed();
```

Change information

Commit message:

[Extensions] Replace chrome.test.assertThrows with W3C signature

Previously, `chrome.test.assertThrows` used the signature `(fn, self,
args, expectedError)`.

This CL replaces that signature with one matching the W3C
`browser.test.assertThrows` proposal [1]: `(fn, expectedError,
message)`. The old signature is no longer supported, and callers must
now use arrow functions or `bind()` to pass arguments to the tested
function.

All users of the previous signature are now updated to use the new
signature.

[1]
https://github.com/w3c/webextensions/blob/main/proposals/browser_test_api.md

TAG=agy
CONV=7c2600ab-5924-4641-b811-1b1e358063f3

Fixed: 519962796

Change-Id: I91c7ec0a0f6d8e596db3ead4156c98cf7e0c8783

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7959318

Reviewed-by: Tim <tjud...@chromium.org>

Auto-Submit: Justin Lulejian <jlul...@chromium.org>

Commit-Queue: Justin Lulejian <jlul...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1653473}

Files:

M chrome/browser/extensions/api/browsing_data/browsing_data_test.cc
M chrome/browser/extensions/api/omnibox/omnibox_api_interactive_test.cc
M chrome/browser/extensions/api/test/apitest_apitest.cc
M chrome/browser/extensions/user_script_world_browsertest.cc
M chrome/test/data/extensions/api_test/bindings/unavailable_bindings_never_registered/background.js
M chrome/test/data/extensions/api_test/dom/closed_shadow_root_from_content_script/content-script.js
M chrome/test/data/extensions/api_test/extension_action/title/background.js
M chrome/test/data/extensions/api_test/file_browser/format_test/test.js
M chrome/test/data/extensions/api_test/isolated_world_csp/mv3/eval.js
M chrome/test/data/extensions/api_test/native_bindings/developer_mode_only_with_api_permission/service_worker_background.js
M chrome/test/data/extensions/api_test/pdf_viewer_private/text_info_test.js
M chrome/test/data/extensions/api_test/permissions/optional/background.js
M chrome/test/data/extensions/api_test/printer_provider/request_capability/test.js
M chrome/test/data/extensions/api_test/printer_provider/request_print/test.js
M chrome/test/data/extensions/api_test/printer_provider/request_printers/test.js
M chrome/test/data/extensions/api_test/printer_provider/request_printers_second/test.js
M chrome/test/data/extensions/api_test/printer_provider/usb_printers/test.js
M chrome/test/data/extensions/api_test/protocol_handler/test_registration.js
M chrome/test/data/extensions/api_test/scripting/main_frame/worker.js
M chrome/test/data/extensions/api_test/tabs/prerendering/test.js
M chrome/test/data/extensions/api_test/tabs/prerendering_into_new_tab/test.js
M extensions/common/api/test.json
M extensions/docs/testing_api.md
M extensions/renderer/resources/test_custom_bindings.js
M extensions/test/data/api_test/messaging/send_message_polyfill_unserializable/background.js
M third_party/closure_compiler/externs/test.js

Change size: L

Delta: 26 files changed, 590 insertions(+), 119 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Tim

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

[Extensions] Replace chrome.test.assertThrows with W3C signature [chromium/src : main]

Justin Lulejian (Gerrit)

Justin Lulejian voted and added 1 comment

Votes added by Justin Lulejian

1 comment

Related details

Tim (Gerrit)

Tim voted and added 3 comments

Votes added by Tim

3 comments

Related details

Justin Lulejian (Gerrit)

Justin Lulejian voted and added 3 comments

Votes added by Justin Lulejian

3 comments

Related details

Justin Lulejian (Gerrit)

Justin Lulejian voted Commit-Queue+2

Related details

Justin Lulejian (Gerrit)

Justin Lulejian voted Commit-Queue+1

Justin Lulejian (Gerrit)

Justin Lulejian voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information