Convert to purely screen wake lock #255

marcoscaceres · 2020-03-11T05:58:47Z

Closes #254
Closes #253
Closes #232

BREAKING CHANGES:

powerful feature name becomes "screen-wake-lock"
Feature policy becomes "screen-wake-lock".
Dropped WakeLockPermissionDescriptor, just use "screen-wake-lock".

The following tasks have been completed:

Modified Web platform tests (link to pull request)

Implementation commitment:

Chromium (https://bugs.chromium.org/p/chromium/issues/detail?id=1064685)
Gecko - Waiting on standards position before committing.
WebKit - Not participating in this working group.

Preview | Diff

index.html

marcoscaceres · 2020-03-16T12:43:13Z

Breaking changers remain:

powerful feature name becomes "screen-wake-lock"
Feature policy becomes "screen-wake-lock".
Dropped WakeLockPermissionDescriptor, just use "screen-wake-lock".

@reillyeon, I guess I could revert those too? The WakeLockPermissionDescriptor felt like a bit of overkill, but I guess it does provide some extensibility.

anssiko

Much thanks!

LGTM overall, punting the review of the remaining breaking changes https://github.com/w3c/wake-lock/pull/255#issuecomment-599514748 to @reillyeon.

reillyeon

This looks good except for a couple places where we should still be using WakeLockType.

index.html

marcoscaceres · 2020-03-17T06:47:02Z

Thanks @reillyeon for chatting all those. Need a day before I jump back into this.

marcoscaceres · 2020-03-25T04:49:48Z

Ok, sorry for taking a little longer than expected. Hope you are all healthy and well.

How should we handle the testing?

Honry · 2020-03-25T06:01:41Z

How should we handle the testing?

I'd suggest to separate tests into corresponding directories, as WPT's test directories are typically named after the shortname of the spec. And we may reuse some common JS from screen wake lock for system wake lock.

marcoscaceres · 2020-03-25T07:07:14Z

Thanks @Honry! I guess we need to then split up:
https://github.com/web-platform-tests/wpt/tree/master/wake-lock

@Honry will you be our testing lead for this? It would be great to start matching tests to spec text.

@anssiko, @reillyeon, should we request a new shortname for the spec?

marcoscaceres · 2020-03-25T07:14:49Z

@Honry, lol, sorry, I see you are already listed as our QA lead! 🙌🎉🎊 Can't ask for a better QA lead on this!

ReSpec supports data-tests as a way of linking tests to spec text. And we also support checking if tests exist in WPT:
https://github.com/w3c/respec/wiki/wpt-tests-exist

Honry · 2020-03-25T07:15:40Z

@Honry will you be our testing lead for this? It would be great to start matching tests to spec text.

@marcoscaceres, sure! :)

Honry · 2020-03-25T07:17:45Z

Actually the original test links were added by myself. I will update them once tests relocated.

marcoscaceres · 2020-03-25T07:18:51Z

Ok cool! Let's merge this, and do the test linking independently.

reillyeon · 2020-03-25T19:49:10Z

I've filed a Chromium issue to track the breaking changes.

anssiko · 2020-03-25T20:18:39Z

@anssiko, @reillyeon, should we request a new shortname for the spec?

Can do this when we publish to /TR.

This CL renames "wake-lock" to "screen-wake-lock" in policy-controlled feature specific for screen wake lock, and leaves system wake lock's policy control as allowed by default because its associated feature policy is not defined yet. Spec changes at: https://github.com/w3c/wake-lock/pull/255 Bug: 1064685 Change-Id: I7701a532a7688d708a75aedafc8713d671da6e2b

Make some small corrections after the big changes from that pull request.

…on names. This adapts the implementation to some of the breaking spec changes introduced in w3c/screen-wake-lock#255. Namely, WakeLockPermissionDescriptor has been removed in favor of using a regular PermissionDescriptor object with |name| set to "screen-wake-lock". That spec change also removed system wake locks (and consequently the additions to the WorkerNavigator interface) altogether, but that will be done in a separate CL, so for now we also support a PermissionDescriptor object with |name| set to "system-wake-lock". Bug: 257511, 1064685 Change-Id: I096f3fae24444da5ca6b1eeb3889a813a9e5f270

…on names. This adapts the implementation to some of the breaking spec changes introduced in w3c/screen-wake-lock#255. Namely, WakeLockPermissionDescriptor has been removed in favor of using a regular PermissionDescriptor object with |name| set to "screen-wake-lock". That spec change also removed system wake locks (and consequently the additions to the WorkerNavigator interface) altogether, but that will be done in a separate CL, so for now we also support a PermissionDescriptor object with |name| set to "system-wake-lock". Bug: 257511, 1064685 Change-Id: I096f3fae24444da5ca6b1eeb3889a813a9e5f270 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134228 Reviewed-by: Kentaro Hara <[email protected]> Reviewed-by: Kent Tamura <[email protected]> Reviewed-by: Mounir Lamouri <[email protected]> Reviewed-by: Kinuko Yasuda <[email protected]> Reviewed-by: Reilly Grant <[email protected]> Auto-Submit: Raphael Kubo da Costa <[email protected]> Commit-Queue: Raphael Kubo da Costa <[email protected]> Cr-Commit-Position: refs/heads/master@{#756753}

…on names. This adapts the implementation to some of the breaking spec changes introduced in w3c/screen-wake-lock#255. Namely, WakeLockPermissionDescriptor has been removed in favor of using a regular PermissionDescriptor object with |name| set to "screen-wake-lock". That spec change also removed system wake locks (and consequently the additions to the WorkerNavigator interface) altogether, but that will be done in a separate CL, so for now we also support a PermissionDescriptor object with |name| set to "system-wake-lock". Bug: 257511, 1064685 Change-Id: I096f3fae24444da5ca6b1eeb3889a813a9e5f270 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134228 Reviewed-by: Kentaro Hara <[email protected]> Reviewed-by: Kent Tamura <[email protected]> Reviewed-by: Mounir Lamouri <[email protected]> Reviewed-by: Kinuko Yasuda <[email protected]> Reviewed-by: Reilly Grant <[email protected]> Auto-Submit: Raphael Kubo da Costa <[email protected]> Commit-Queue: Raphael Kubo da Costa <[email protected]> Cr-Commit-Position: refs/heads/master@{#756753}

…riptor, use separate permission names., a=testonly Automatic update from web-platform-tests wake lock: Remove WakeLockPermissionDescriptor, use separate permission names. This adapts the implementation to some of the breaking spec changes introduced in w3c/screen-wake-lock#255. Namely, WakeLockPermissionDescriptor has been removed in favor of using a regular PermissionDescriptor object with |name| set to "screen-wake-lock". That spec change also removed system wake locks (and consequently the additions to the WorkerNavigator interface) altogether, but that will be done in a separate CL, so for now we also support a PermissionDescriptor object with |name| set to "system-wake-lock". Bug: 257511, 1064685 Change-Id: I096f3fae24444da5ca6b1eeb3889a813a9e5f270 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134228 Reviewed-by: Kentaro Hara <[email protected]> Reviewed-by: Kent Tamura <[email protected]> Reviewed-by: Mounir Lamouri <[email protected]> Reviewed-by: Kinuko Yasuda <[email protected]> Reviewed-by: Reilly Grant <[email protected]> Auto-Submit: Raphael Kubo da Costa <[email protected]> Commit-Queue: Raphael Kubo da Costa <[email protected]> Cr-Commit-Position: refs/heads/master@{#756753} -- wpt-commits: 7ab34f7e16b668b470397262b66db4ba915beedd wpt-pr: 22658

And make it default to "screen". Existing code continues to work, while new code can also call `navigator.wakeLock.request()` to request a screen wake lock. Issues w3c#253, w3c#255 and w3c#288 indicate that there is a preference for making `type` an optional parameter that just got overlooked.

And make it default to "screen". Existing code continues to work, while new code can also call `navigator.wakeLock.request()` to request a screen wake lock. Issues #253, #255 and #288 indicate that there is a preference for making `type` an optional parameter that just got overlooked.

BREAKING CHANGE: Convert to purely screen wake lock

1f8e78d

reillyeon requested changes Mar 11, 2020

View reviewed changes

index.html Outdated Show resolved Hide resolved

anssiko reviewed Mar 11, 2020

View reviewed changes

index.html Outdated Show resolved Hide resolved

reillyeon requested changes Mar 12, 2020

View reviewed changes

index.html Outdated Show resolved Hide resolved

Marcos Cáceres added 2 commits March 16, 2020 23:19

Review feeback

d8880bf

use WakeLockType on request()

3a8a33e

marcoscaceres changed the title ~~BREAKING CHANGE: Convert to purely screen wake lock~~ Convert to purely screen wake lock Mar 16, 2020

Add Raphael, remove imp status

1ab289d

marcoscaceres commented Mar 16, 2020

View reviewed changes

index.html Show resolved Hide resolved

Marcos Cáceres added 2 commits March 16, 2020 23:34

add @rakuco's id

5a52832

Keep current shortname for now

805f603

anssiko approved these changes Mar 16, 2020

View reviewed changes

reillyeon approved these changes Mar 16, 2020

View reviewed changes

index.html Outdated Show resolved Hide resolved

index.html Outdated Show resolved Hide resolved

index.html Outdated Show resolved Hide resolved

index.html Outdated Show resolved Hide resolved

index.html Outdated Show resolved Hide resolved

Marcos Cáceres added 2 commits March 25, 2020 15:43

Fixup! thanks @reillyeon

bf80bdc

Fix another wrong type

7f0f261

marcoscaceres merged commit 59d8d39 into w3c:gh-pages Mar 25, 2020

marcoscaceres deleted the screen-wake-lock branch March 25, 2020 07:20

chromium-wpt-export-bot mentioned this pull request Mar 26, 2020

[wakelock] Rename feature policy web-platform-tests/wpt#22472

Merged

rakuco pushed a commit to rakuco/wake-lock that referenced this pull request Apr 1, 2020

Proof-read after w3c#255.

504da6a

Make some small corrections after the big changes from that pull request.

marcoscaceres pushed a commit that referenced this pull request Apr 2, 2020

Editorial: Proof-read after #255. (#260)

e9ff478

Make some small corrections after the big changes from that pull request.

chromium-wpt-export-bot mentioned this pull request Apr 2, 2020

wake lock: Remove WakeLockPermissionDescriptor, use separate permission names. web-platform-tests/wpt#22658

Merged

rakuco mentioned this pull request Sep 30, 2020

Make "type" an optional parameter for WakeLock.request(). #291

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert to purely screen wake lock #255

Convert to purely screen wake lock #255

marcoscaceres commented Mar 11, 2020 •

edited by reillyeon

Loading

marcoscaceres commented Mar 16, 2020

anssiko left a comment

reillyeon left a comment

marcoscaceres commented Mar 17, 2020

marcoscaceres commented Mar 25, 2020

Honry commented Mar 25, 2020

marcoscaceres commented Mar 25, 2020

marcoscaceres commented Mar 25, 2020

Honry commented Mar 25, 2020

Honry commented Mar 25, 2020

marcoscaceres commented Mar 25, 2020

reillyeon commented Mar 25, 2020

anssiko commented Mar 25, 2020

Convert to purely screen wake lock #255

Convert to purely screen wake lock #255

Conversation

marcoscaceres commented Mar 11, 2020 • edited by reillyeon Loading

marcoscaceres commented Mar 16, 2020

anssiko left a comment

Choose a reason for hiding this comment

reillyeon left a comment

Choose a reason for hiding this comment

marcoscaceres commented Mar 17, 2020

marcoscaceres commented Mar 25, 2020

Honry commented Mar 25, 2020

marcoscaceres commented Mar 25, 2020

marcoscaceres commented Mar 25, 2020

Honry commented Mar 25, 2020

Honry commented Mar 25, 2020

marcoscaceres commented Mar 25, 2020

reillyeon commented Mar 25, 2020

anssiko commented Mar 25, 2020

marcoscaceres commented Mar 11, 2020 •

edited by reillyeon

Loading