[Extensions] Implement AlarmCreateInfo.name [chromium/src : main]
Anton Bershanskyi (Gerrit)
Anton Bershanskyi added 1 comment![]( "Open in Gerrit")
Hi Devlin,
this CL implements support for passing extension alarm name via `AlarmCreateInfo.name`. This is purely syntactic sugar to make the `alarms.create()` API a bit more ergonomic. After this CL, the following two calls are equivalent:
```js
alarms.create("kettle", {delayInMinutes: 1.5});
alarms.create({name: "kettle", delayInMinutes: 1.5});
```
This does not expose any new capabilities or behaviors, it merely scratches a long-standing itch I had.
We shortly discussed this at WECG and everyone expressed lukewarm interest in it:
https://github.com/w3c/webextensions/issues/999
https://github.com/w3c/webextensions/blob/main/_minutes/2026-05-21-wecg.md
Thanks!
Related details
- Devlin Cronin
- Oliver Dunk
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Devlin Cronin (Gerrit)
Devlin Cronin added 4 comments![]( "Open in Gerrit")
Thanks, Anton!
I'm not opposed to this, but I would like Oliver to weigh in from a devrel side. I agree this is the cleaner signature, but I could see an argument that having two supported signatures for the same effect is more confusing than having a single, well-documented, slightly-worse signature.
void ExtensionAlarmsTestGetNamedAlarmCallback(ExtensionAlarmsTest* test,nit1: Passing the test object here is a little unconventional. Two other options would be:
1) Using an instance method on the test itself, or
2) Using a lambda in the test.
That said, see below for what IMO is an even better solution.
base::BindOnce(ExtensionAlarmsTestGetNamedAlarmCallback, this));This won't work well in all cases -- tests don't run all pending callbacks, so when we reach the end of the test body, the test is done. This means that if this callback runs async, the test will finish, and the validation will never happen -- worse, we might potentially get a UAF, crashing the test suite.
I think we can make this much cleaner by using a base::test::TestFuture:
```
base::test::TestFuture<Alarm*> alarm_future;
alarm_manager_->GetAlarm(extension()->id(), "Alarm Name", alarm_future.GetCallback());
Alarm* alarm = alarm_future.Get();
// Verify alarm...
```
Under the hood, TestFuture::Get() will wait until the callback is invoked and return its resolved value.
"[\"Invalid\", {\"name\": \"Invalid\", \"delayInMinutes\": 0}]"));nit: cleaner to use a R"(String Literal)" here to avoid \\-escapes
Related details
- Anton Bershanskyi
- Oliver Dunk
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. |
Anton Bershanskyi (Gerrit)
Anton Bershanskyi added 4 comments![]( "Open in Gerrit")
Hi Oliver, I would be glad to hear your thoughts regarding extend `alarms.create()` to also accept alarm name in the object. I believe it makes the API a bit more ergonomic. At the same time as Devlin noted, having two ways of doing something might be worse than just one not ergonomic way.
If Chromium and WECG/WEWG agrees that this is a valuable addition, I can prepare CLs/PRs implementing this for Chromium and Firefox and polyfil libraries of your choice. However, I can not say anything about the timelines for creating patches for multiple projects. Developing for Safari might be harder for me due to lack of a suitable compilation environment.
Hi Devlin, thanks for detailed feedback, I updated tests.
void ExtensionAlarmsTestGetNamedAlarmCallback(ExtensionAlarmsTest* test,nit1: Passing the test object here is a little unconventional. Two other options would be:
1) Using an instance method on the test itself, or
2) Using a lambda in the test.That said, see below for what IMO is an even better solution.
Done
base::BindOnce(ExtensionAlarmsTestGetNamedAlarmCallback, this));This won't work well in all cases -- tests don't run all pending callbacks, so when we reach the end of the test body, the test is done. This means that if this callback runs async, the test will finish, and the validation will never happen -- worse, we might potentially get a UAF, crashing the test suite.
I think we can make this much cleaner by using a base::test::TestFuture:
```
base::test::TestFuture<Alarm*> alarm_future;
alarm_manager_->GetAlarm(extension()->id(), "Alarm Name", alarm_future.GetCallback());
Alarm* alarm = alarm_future.Get();
// Verify alarm...
```Under the hood, TestFuture::Get() will wait until the callback is invoked and return its resolved value.
Done
"[\"Invalid\", {\"name\": \"Invalid\", \"delayInMinutes\": 0}]"));nit: cleaner to use a R"(String Literal)" here to avoid \\-escapes
- Oliver Dunk
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bershanskyi (Gerrit)
Anton Bershanskyi added 1 comment![]( "Open in Gerrit")
Hi Devlin,
this CL extends `alarms.create()` so that alarm name can be passed either as a separate (optional) first string parameter or within the object. For example, the following two calls will become equivalent:
```js
alarms.create("kettle", {delayInMinutes: 1.5});
alarms.create({name: "kettle", delayInMinutes: 1.5});
```
WECG discussed this proposal in detail and all three engines expressed support.[1]
What do you think of this CL?
Related details
- Devlin Cronin
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Devlin Cronin (Gerrit)
Devlin Cronin voted and added 1 comment![]( "Open in Gerrit")
Votes added by Devlin Cronin
| Code-Review | +1 |
1 comment
- Anton Bershanskyi
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bershanskyi (Gerrit)
Anton Bershanskyi added 1 comment![]( "Open in Gerrit")
Hi Justin,
this CL extends `alarms.create()` API so that the following two calls are equivalent:
```js
alarms.create("kettle", {delayInMinutes: 1.5});
alarms.create({name: "kettle", delayInMinutes: 1.5});
```
WECG/WEWG supports this API proposal (see links in the commit message).
Would you be interested in reviewing this? Thanks!
Related details
- Justin Lulejian
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Justin Lulejian (Gerrit)
Justin Lulejian added 2 comments![]( "Open in Gerrit")
TEST_F(ExtensionAlarmsTest, CreateNameInObject) {Could you also create a simple WPT test for this new API surface? The existing alarm test suite it at: [//third_party/blink/web_tests/external/wpt/web-extensions/resources/alarms/background.js](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/web-extensions/resources/alarms/background.js).
I'd suggest a new test case called `testAlarmsCreateNames` and test:
1) the alarm name as a parameter
2) the alarm name in AlarmCreateInfo
3) not passing either (no alarm name parameter, but also not set in AlarmCreateInfo)
Use alarms.get() to verify that the alarm was created as expected, but we don't need to test that it actually fired (that's already tested in `testAlarmsOnAlarm`).
And then add to `testAlarmsErrorCases` a check for:
4) passing both, which throws an error
// MessageLoop when that happens.```suggestion
// `MessageLoop` when that happens.
```
Related details
- Anton Bershanskyi
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. |
Anton Bershanskyi (Gerrit)
Anton Bershanskyi added 3 comments![]( "Open in Gerrit")
Hi Justin, please take another look. Thanks!
Could you also create a simple WPT test for this new API surface? The existing alarm test suite it at: [//third_party/blink/web_tests/external/wpt/web-extensions/resources/alarms/background.js](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/web-extensions/resources/alarms/background.js).
I'd suggest a new test case called `testAlarmsCreateNames` and test:
1) the alarm name as a parameter
2) the alarm name in AlarmCreateInfo
3) not passing either (no alarm name parameter, but also not set in AlarmCreateInfo)Use alarms.get() to verify that the alarm was created as expected, but we don't need to test that it actually fired (that's already tested in `testAlarmsOnAlarm`).
And then add to `testAlarmsErrorCases` a check for:
4) passing both, which throws an error
Thank you for clear request, I added these tests.
```suggestion
// `MessageLoop` when that happens.
```
- Justin Lulejian
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Justin Lulejian (Gerrit)
Justin Lulejian voted and added 1 comment![]( "Open in Gerrit")
Votes added by Justin Lulejian
| Code-Review | +1 |
1 comment
lgtm, thanks Anton! Extra thanks for writing the WPT tests and fixing the existing .create() calls.
Related details
- Anton Bershanskyi
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bershanskyi (Gerrit)
Anton Bershanskyi added 1 comment![]( "Open in Gerrit")
Hi Devlin, would you be interested in another round of reviews? Thanks!
Related details
- Devlin Cronin
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Devlin Cronin (Gerrit)
Devlin Cronin voted and added 1 comment![]( "Open in Gerrit")
Votes added by Devlin Cronin
| Code-Review | +1 |
1 comment
- Anton Bershanskyi
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/61508.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
Related details
- Anton Bershanskyi
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bershanskyi (Gerrit)
Anton Bershanskyi voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
21 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
[Extensions] Implement AlarmCreateInfo.name
Add support for specifying alarm name for alarms.create() in the second
object argument called AlarmCreateInfo.name.
Background:
- https://github.com/w3c/webextensions/issues/999
- https://github.com/w3c/webextensions/blob/main/_minutes/2026-05-21-wecg.md
- M extensions/browser/api/alarms/alarm_manager.cc
- M extensions/browser/api/alarms/alarm_manager.h
- M extensions/browser/api/alarms/alarms_api.cc
- M extensions/browser/api/alarms/alarms_api_unittest.cc
- M extensions/common/api/alarms.webidl
- M third_party/blink/web_tests/external/wpt/web-extensions/resources/alarms/background.js
Code-Review: +1 by Devlin Cronin, +1 by Justin Lulejian
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/61508
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |