Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

WakeLock.request() returns a promise that never resolves #226

Closed
tomayac opened this issue Aug 7, 2019 · 33 comments 路 Fixed by #237

Comments

@tomayac
Copy link
Collaborator

commented Aug 7, 2019

When playing with the new API, implemented by @rakuco, I intuitively started with this:

/* 馃毇 Code sample 1 */
(async () => {
  const controller = new AbortController();
  const signal = controller.signal;
  try {
    await WakeLock.request('screen', {signal});
    // This will never be reached
    console.log('Wake Lock is active');
  } catch (e) {
    // This will never be reached
    if (e.name === 'AbortError') {
      console.log('All fine, Wake Lock aborted');
    } else {
      console.error(e.name, e.message);
    }
  }
  // This will never be reached
  window.setTimeout(() => {
    controller.abort();
  }, 5000);
})();

I soon realized that I can't await a never resolving promise, so I switched to the below, which works as intended, but results in an ugly uncaught promise error:
Uncaught (in promise) DOMException:

/* 馃毇 Code sample 2 */
(async () => {
  const controller = new AbortController();
  const signal = controller.signal;
  try {
    // Don't `await`
    /* await */ WakeLock.request('screen', {signal});
    // This will be reached now
    console.log('Wake Lock is active');
  } catch (e) {
    // This will never be reached
    if (e.name === 'AbortError') {
      console.log('All fine, Wake Lock aborted');
    } else {
      console.error(e.name, e.message);
    }
  }
  // This will be reached now
  window.setTimeout(() => {
    controller.abort();
  }, 5000);
})();

In order to avoid an uncaught promise, the final working code I settled upon is the following, with the catch() chained to the WakeLock.request(), which is a little ugly as well:

/* 鉁 Code sample 3 */
(async () => {
  const controller = new AbortController();
  const signal = controller.signal;
  // Don't `await`
  WakeLock.request('screen', {signal})
  // Chain a `catch()`
  .catch((e) => {
    if (e.name === 'AbortError') {
      console.log('All fine, Wake Lock aborted');
    } else {
      console.error(e.name, e.message);
    }
  });
  // This will be reached now
  console.log('Wake Lock is active');
  // This will be reached now
  window.setTimeout(() => {
    controller.abort();
  }, 5000);
})();

Unless I am completely wrong (which is easily possible), the intuitive (by my intuition at least) Code sample 1 could be made work if the promise returned as a result of running the steps in 搂聽6.3 would resolve if (and only if) active, i.e., the result of 搂聽7.4, is true. Thoughts?

@reillyeon

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2019

I agree that the API is potentially confusing. The Promise returned by WakeLock.request() never resolves. It only rejects when the wake lock has been released. This is the result of an extensive conversation that resulted in #201.

The intuition is that the "lock acquired" state is uninteresting. If we fail to acquire a wake lock it will happen quickly and the Promise will be rejected. If we successfully acquire a lock the site only cares if that lock is taken away from it.

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

It only rejects when the wake lock has been released.

That seems wrong, fwiw - and @tomayac鈥檚 code seems to illustrates that. It鈥檚 not 鈥渆xceptional鈥 that the lock is released (something is not right) - though a user aborting the acquisition operation might be. The lock being acquired is a natural point at which to settle the promise.

I鈥檇 suggest maybe looking at how the full screen API works. This is quite similar.

@reillyeon

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2019

The problem with resolving the Promise when the lock is acquired is that it removes the only way we have to signal that the lock has been revoked by the UA. It sounds like you want to revert back to a more complex WakeLock object which dispatches an event when the lock state changes.

@tomayac

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 8, 2019

It sounds like you want to revert back to a more complex WakeLock object which dispatches an event when the lock state changes.

Could the promise resolve with some sort of result object, so the following could work (reduced to just the relevant bits):

try {
  const wakeLock = await WakeLock.request('screen', {signal});
  console.log(`馃檶 Yay, Wake Lock of type ${wakeLock.type} is active!`);

  // The event name `"wakelockintervention"` is made up
  wakeLock.addEventListener('wakelockintervention', (e) => {
    console.error(`馃槺 The UA terminated Wake Lock ${e.target.type}.`);
  });
} catch (e) {
  // Regular abort 
  if (e.name === 'AbortError') {
    console.log('鉁 All fine, Wake Lock aborted');
  } else {
    // Something went wrong during acquiring
    console.error('馃毇', e.name, e.message);
   }
}
@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

No, because then we get back to the situation of having to manage a bunch of objects.

Let鈥檚 just attach .requestLock() to Window, and the just fire events at the Window object (or let鈥檚 just add .wakelock. to Navigator. The .requestLock() can just async give you back some key, and then we can just have .removeLock(key); when done.

That gets rid of the whole having to manage objects, and can just use simple events. If the lock is held, we can just keep returning the same key.

@tomayac

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 8, 2019

Sounds great to me, I had a similar idea in mind where like with window.setInterval you鈥檇 get an ID back, but I thought it was too audacious to suggest attaching the request method 鈥渢oo high鈥.

One could then listen for window.onwakelockintervention(reusing my made-up name) and be notified of UA interventions.

The other point I raised was with the never-resolving promise. Can this be converted in a resolving promise when the lock is acquired successfully?

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

Can this be converted in a resolving promise when the lock is acquired successfully?

Yes. The (async) result of acquiring the lock would be the key.

@reillyeon

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2019

Proposed new WebIDL from TPAC 2019 F2F:

partial interface Navigator {
  [SameObject] readonly attribute WakeLock wakeLock;
};

partial interface WorkerNavigator {
  [SameObject] readonly attribute WakeLock wakeLock;
};

[Exposed=(Window,DedicatedWorker)]
interface WakeLock {
  Promise<unsigned long long> request(WakeLockType type);
  Promise<void> release(unsigned long long wakeLockID);
};

dictionary WakeLockEventInit {
  required unsigned long long wakeLockID;
};

[Exposed=(Window,DedicatedWorker)]
interface WakeLockEvent : Event {
  constructor(DOMString type, WakeLockEventInit init);
  readonly attribute unsigned long long wakeLockID;
};

A new "wakelockreleased" event is fired with a WakeLockEvent with wakeLockID set to the ID of the lock that has been released either voluntarily or involuntarily.

@rakuco

This comment has been minimized.

Copy link
Collaborator

commented Sep 19, 2019

cc @kenchris for fun and fyi :-)

@kenchris

This comment has been minimized.

Copy link
Collaborator

commented Sep 19, 2019

Can you link to the discussion minutes later (had a conflict)

@rakuco

This comment has been minimized.

Copy link
Collaborator

commented Sep 19, 2019

@marcoscaceres will talk to you about that (hopefully someone will also post an answer here :-)

@rakuco

This comment has been minimized.

Copy link
Collaborator

commented Sep 19, 2019

A new "wakelockreleased" event is fired with a WakeLockEvent with lock set to the ID of the lock that has been released either voluntarily or involuntarily.

What would be the event target for these events?

@reillyeon

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2019

A new "wakelockreleased" event is fired with a WakeLockEvent with lock set to the ID of the lock that has been released either voluntarily or involuntarily.

What would be the event target for these events?

document for frames and self for workers.

@css-meeting-bot

This comment has been minimized.

Copy link
Member

commented Sep 20, 2019

The Devices and Sensors Working Group just discussed WakeLock.request() returns a promise that never resolves.

The full IRC log of that discussion <xfq> Topic: WakeLock.request() returns a promise that never resolves
<xfq> github: https://github.com//issues/226
<anssik> https://github.com//issues/226
<anssik> scribeNick: anssik
<anssik> reillyg: #锘226 is a reaction to earlier API changes to remove WK instances and use single instance of a WakeLock object, minimal API that takes wake lock type and AbortSignal
<anssik> tom: awaiting a promise that never resolves is an uncommon design pattern
<anssik> reillyg: API was unintuitive, the issue #锘226 contains a number of examples of code developers would write and would expect to works based on other promise-using APIs
<anssik> tom: async/await pattern does not work too
<anssik> RRSAgent, draft minutes v2
<RRSAgent> I have made the request to generate https://www.w3.org/2019/09/20-dap-minutes.html anssik
<anssik> reillyg: the proposed fix was to build a mode intuitive API for developers and it fixes the ergonomics issues enumerated in #锘226
<anssik> ... had marcos in the room to speak how to use AbortController the right way
<anssik> ... AbortController was not the right tool for this task
<anssik> ... we'd instead use a pattern of an id, similarly to setInterval/setTimeout()
<anssik> tom: should rename "lockId "into "wakeLockID"
<anssik> reillyg: updated the proposal in #锘226
<anssik> kenneth: it's not constructible?
<anssik> reillyg: yes, since it's an attribute on navigator
@domenic

This comment has been minimized.

Copy link

commented Sep 21, 2019

Although I am significantly disappointed about the movement away from the unified cancelation primitive of AbortSignal, I strongly urge the working group to avoid numeric IDs for cancelation. setTimeout's use of those is a very bad pattern from the old days of the web, and not to be emulated. Objects should be preferred to numbers.

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Sep 22, 2019

AbortSignal wasn鈥檛 the right thing to use here. I don鈥檛 mind returning an object as the sentinel.

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Sep 23, 2019

AbortSignal wasn鈥檛 the right thing to use here.

Now that I'm back on firm soil, let me clarify... the AbortController usage was ambiguous because it was not clear if it applied to:

  1. the asynchronous acquisition of the lock - requires IPC, but presumedly fairly quick.
  2. the actual lock itself - ongoing operation after.
  3. both... which is complicated to implement IMO.

I don鈥檛 mind returning an object as the sentinel.

If we go down the object route again, which I think more clear, we can have a WakeLockSentinel returned that just has release() method, a type attribute, and an event handler attribute to monitor state change, and a way to check current state.

partial interface Navigator {
  [SameObject] readonly attribute WakeLock wakeLock;
};

partial interface WorkerNavigator {
  [SameObject] readonly attribute WakeLock wakeLock;
};

[Exposed=(Window,DedicatedWorker)]
interface WakeLock {
  Promise<WakeLockSentinel> request(WakeLockType type);
};

dictionary WakeLockEventInit {
  required unsigned WakeLockSentinel lock;
};

[Exposed=(Window,DedicatedWorker)]
interface WakeLockEvent : Event {
  constructor(DOMString type, WakeLockEventInit init);
  readonly attribute WakeLockSentinel lock;
};

enum WakeLockState {
  "active",
  "released",
}

interface WakeLockSentinel : EventTarget {
  // can only be called once
  Promise<void> release();
  readonly attribute WakeLockType type;
  attribute EventHandler onstatechange;
  readonly attribute WakeLockState state;
}

So roughly:

let lock = await navigator.wakeLock.request("screen");
lock.onstatechange = async ev => {
   // request a new lock, maybe...
   lock = await navigator.wakeLock.request(ev.lock.type); 
}

// ... stuff happens.... then...

if (lock.state === "released") return; 
try {
   await lock.release();
} catch (err) {
  // any unexpected errors
}

// Lock was released, so it's consumed/dead; this would reject:
await lock.release(); // InvalidStateError

I'd be happy with the above API.

@tomayac

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 23, 2019

I tried to rewrite my example from #226 (comment) with the new new new API proposal:

(async () => {
  let lock = null;
  try {
    lock = await navigator.wakeLock.request('screen');
    console.log('Wake Lock is active');

    // This would fire on visibility changes
    lock.addEventListener('statechange', async (e) => {
      // Re-request a new lock after a visibility change
      lock = await navigator.wakeLock.request('screen'); // (鈾)
    });

    // Schedule a regular release
    window.setTimeout(() => {
      lock.release();
      lock = null; // (鈫)
    }, 5000);
  } catch (e) {
    // This would catch browser interventions
    // like battery saver force-releasing the wake lock
    console.error(e.name, e.message);
    lock = null; // (鈫)
  }
})();

Two and a half questions:

  • Would onstatechange fire in case of a browser intervention (that would be catch'ed (ugh, well, caught))?
  • As the only state change that's possible is 'active' to 'released', maybe onstatechange could be renamed to onrelease?
  • Not entirely sure about garbage collection here. If I null-ify the lock variable (above marked with // (鈫)), would it's event listeners be GC'ed? And if I recycle the variable to re-acquire a new lock (above marked with // (鈾)), would a previously created event listener be still there?
@rakuco

This comment has been minimized.

Copy link
Collaborator

commented Sep 23, 2019

For future reference, Reilly is also asking for more clarification on AbortController and AbortSignal in w3ctag/design-principles#138

@kenchris

This comment has been minimized.

Copy link
Collaborator

commented Sep 24, 2019

let lock = await navigator.wakeLock.request("screen");
lock.onstatechange = async ev => {
   // request a new lock, maybe...
   lock = await navigator.wakeLock.request(ev.lock.type); 
}

So when the promise resolves it will have either failed or be active, but before you add the onstatechanged callback. That makes onstatechanged a bit useless, and we might as well just have a onlockreleased instead.

Also, can you try writing your example in a way where it integrates with visibilitychange?

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

That makes onstatechanged a bit useless, and we might as well just have a onlockreleased instead.

I'd be ok with that... maybe just "release" tho, as shorter :)

@kenchris

This comment has been minimized.

Copy link
Collaborator

commented Sep 26, 2019

I'd be ok with that... maybe just "release" tho, as shorter :)

Yeah, sure :-)

partial interface Navigator {
  [SameObject] readonly attribute WakeLock wakeLock;
};

partial interface WorkerNavigator {
  [SameObject] readonly attribute WakeLock wakeLock;
};

[Exposed=(Window,DedicatedWorker)]
interface WakeLock {
  Promise<WakeLockSentinel> request(WakeLockType type);
};

dictionary WakeLockEventInit {
  required unsigned WakeLockSentinel lock;
};

[Exposed=(Window,DedicatedWorker)]
interface WakeLockEvent : Event {
  constructor(DOMString type, WakeLockEventInit init);
  readonly attribute WakeLockSentinel lock;
};

interface WakeLockSentinel : EventTarget {
  // can only be called once
  Promise<void> release();
  readonly attribute WakeLockType type;
  attribute EventHandler onrelease;
}

What is the point of the event giving you the WakeLockSentinel? Maybe it would be better to give the reason why it was released? But then again, if I call release() myself, I am not very interested in that event, so maybe a systemrelease event would make more sense?

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

What is the point of the event giving you the WakeLockSentinel?

So you can check the type.

Maybe it would be better to give the reason why it was released?

The reason why it was released none of the app's business.

But then again, if I call release() myself, I am not very interested in that event,

Sure, but other code might be.

so maybe a systemrelease event would make more sense?

IMO no, because you can get the type from the sentinel. Then we don't need to add new event names. We just use "release".

@kenchris

This comment has been minimized.

Copy link
Collaborator

commented Sep 26, 2019

Why it was released none of the apps business.

True, but I do want to differ whether I released it or it was released by the system, because I might want to reacquire

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

If you are releasing it, then you could remove the event listener before calling release(), no?

@kenchris

This comment has been minimized.

Copy link
Collaborator

commented Sep 26, 2019

You could, but it feels weird and I wonder how many would actually think about that

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

Not sure I follow? It's completely normal to remove a listener? We are not doing anything special.

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Oct 2, 2019

Here is an example of removing the listener... as you can see, it's not any more weird than adding an even listener:

(async () => {
  let lock = null;
  try {
    lock = await navigator.wakeLock.request('screen');
    console.log('Wake Lock is active');

    const listener = async listener (e) {
      // Re-request a new lock after a visibility change
      // This could also throw... but whatever... 
      lock = await navigator.wakeLock.request('screen'); // (鈾)
    }
    // This would fire on visibility changes, for example
    lock.addEventListener('statechange', listener);

    // Schedule a regular release
    window.setTimeout(() => {
      // "cool and normal"鈩笍
      lock.removeEventListener("statechange", listener);
      lock.release();
      lock = null; // (鈫)
    }, 5000);

  } catch (e) {
    // This would catch browser interventions
    // like battery saver rejecting the wake lock request
    console.error(e.name, e.message);
    lock = null; // (鈫)
  }
})();
@rakuco

This comment has been minimized.

Copy link
Collaborator

commented Oct 3, 2019

How is WakeLockSentinel.release() supposed to behave when an event such as a page visibility change happens? Is it supposed to do nothing, return a rejected promise or none of the above?

@reillyeon

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2019

How is WakeLockSentinel.release() supposed to behave when an event such as a page visibility change happens? Is it supposed to do nothing, return a rejected promise or none of the above?

My vote is for it to do nothing. Returning a rejected Promise doesn't tell the page anything useful.

@reillyeon

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2019

  • Not entirely sure about garbage collection here. If I null-ify the lock variable (above marked with // (鈫)), would it's event listeners be GC'ed? And if I recycle the variable to re-acquire a new lock (above marked with // (鈾)), would a previously created event listener be still there?

I don't know how this works in spec-language but in Blink there is the concept of "has pending activity." An object that doesn't have any references but does have an event listener that could still be fired can be marked as having pending activity and therefore not be garbage collected. As an example from another API, a Sensor object that has been started does this for "reading" and "error" events.

@rakuco

This comment has been minimized.

Copy link
Collaborator

commented Oct 4, 2019

How is WakeLockSentinel.release() supposed to behave when an event such as a page visibility change happens? Is it supposed to do nothing, return a rejected promise or none of the above?

My vote is for it to do nothing. Returning a rejected Promise doesn't tell the page anything useful.

OK, so to sum it up:

  • navigator.wakeLock.request() creates a WakeLockSentinel that can be release()d and which can receive up to 1 "release" event.
  • An event like a page visibility change causes existing WakeLockSentinels' release() methods to silently do nothing, and all these objects receive up to 1 "release" event
  • Otherwise, calling WakeLockSentinel.release() works once and delivers up to 1 "release" event to the same object.

If that's the case:

  • A WakeLockEvent's only attribute is lock, which is a WakeLockSentinel. If we've reached a "release" event handler with a WakeLockEvent, calling release() on it will either do nothing or return a rejected promise based on the above. In other words lock is essentially useless in a WakeLockEvent except for its type attribute. In this case, wouldn't it suffice to have a type attribute in WakeLockEvent to do something like
    lock.addEventListener('release', ev => { lock = navigator.wakeLock.request(ev.type); })
  • Is the "release" event supposed to indicate that a particular WakeLockSentinel has been released or that the platform wake lock has been released? In other words, if I create two screen wake locks and call release() on one, should it instantly receive a "release" event or does that happen only when both locks are released and we thus no longer hold a platform wake lock?
@reillyeon

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2019

  • A WakeLockEvent's only attribute is lock, which is a WakeLockSentinel. If we've reached a "release" event handler with a WakeLockEvent, calling release() on it will either do nothing or return a rejected promise based on the above. In other words lock is essentially useless in a WakeLockEvent except for its type attribute. In this case, wouldn't it suffice to have a type attribute in WakeLockEvent to do something like
    lock.addEventListener('release', ev => { lock = navigator.wakeLock.request(ev.type); })

Providing the WakeLockSentinel rather than the WakeLockType prevents duplication of attributes between WakeLockSentinel and WakeLockEvent. It also allows script to know exactly which sentinel has been invalidated. At the moment that can be trivially deduced from the WakeLockType but this avoids potential ambiguity going forward.

  • Is the "release" event supposed to indicate that a particular WakeLockSentinel has been released or that the platform wake lock has been released? In other words, if I create two screen wake locks and call release() on one, should it instantly receive a "release" event or does that happen only when both locks are released and we thus no longer hold a platform wake lock?

It indicates a particular WakeLockSentinel has been released. If you create two wake locks and release() one then only that one will receive a "release" event. If the wake lock is released by the platform then both will receive a "release" event.

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 9, 2019
This is another big update to the implementation, which now follows the
API proposed in w3c/wake-lock#226.

From a Blink/Chromium internals perspective, things have not changed much,
and we still perform the same permission checks and connect to the wake lock
service via WakeLockStateRecord. The biggest change is that WakeLockController
was merged into WakeLock itself.

From an end-user perspective, though, the API now looks like a mix of the
old, navigator-based API and the one we implemented a few months ago:

* The main idea is that WakeLock.request() no longer returns a promise that
  does not resolve while the lock is held, as that is not very intuitive.
  Instead, it returns a new WakeLockSentinel object.
* WakeLockSentinel is a new interface that can release() a lock and receive
  "release" events.
* We do go back to a Navigator-based API, but we also support
  WorkerNavigator.
* There is no WakeLockRequest interface or state tracking in the WakeLock
  interface.
* WakeLock.requestPermission() was removed.
* Consequently, we now have a WakeLockEvent interface.

The changes to the spec still need to land, but there is some rough
consensus on what the API should look like, and we do not want to miss the
M79 branch cut.

Bug: 257511, 1010162
Change-Id: If1baed914ccd9eb7103a1bb71a40529976777ec2
chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 10, 2019
This is another big update to the implementation, which now follows the
API proposed in w3c/wake-lock#226.

From a Blink/Chromium internals perspective, things have not changed much,
and we still perform the same permission checks and connect to the wake lock
service via WakeLockStateRecord. The biggest change is that WakeLockController
was merged into WakeLock itself.

From an end-user perspective, though, the API now looks like a mix of the
old, navigator-based API and the one we implemented a few months ago:

* The main idea is that WakeLock.request() no longer returns a promise that
  does not resolve while the lock is held, as that is not very intuitive.
  Instead, it returns a new WakeLockSentinel object.
* WakeLockSentinel is a new interface that can release() a lock and receive
  "release" events.
* We do go back to a Navigator-based API, but we also support
  WorkerNavigator.
* There is no WakeLockRequest interface or state tracking in the WakeLock
  interface.
* WakeLock.requestPermission() was removed.
* Consequently, we now have a WakeLockEvent interface.

The changes to the spec still need to land, but there is some rough
consensus on what the API should look like, and we do not want to miss the
M79 branch cut.

Bug: 257511, 1010162
Change-Id: If1baed914ccd9eb7103a1bb71a40529976777ec2
chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 10, 2019
This is another big update to the implementation, which now follows the
API proposed in w3c/wake-lock#226.

From a Blink/Chromium internals perspective, things have not changed much,
and we still perform the same permission checks and connect to the wake lock
service via WakeLockStateRecord. The biggest change is that WakeLockController
was merged into WakeLock itself.

From an end-user perspective, though, the API now looks like a mix of the
old, navigator-based API and the one we implemented a few months ago:

* The main idea is that WakeLock.request() no longer returns a promise that
  does not resolve while the lock is held, as that is not very intuitive.
  Instead, it returns a new WakeLockSentinel object.
* WakeLockSentinel is a new interface that can release() a lock and receive
  "release" events.
* We do go back to a Navigator-based API, but we also support
  WorkerNavigator.
* There is no WakeLockRequest interface or state tracking in the WakeLock
  interface.
* WakeLock.requestPermission() was removed.
* Consequently, we now have a WakeLockEvent interface.

The changes to the spec still need to land, but there is some rough
consensus on what the API should look like, and we do not want to miss the
M79 branch cut.

Bug: 257511, 1010162
Change-Id: If1baed914ccd9eb7103a1bb71a40529976777ec2
chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 10, 2019
This is another big update to the implementation, which now follows the
API proposed in w3c/wake-lock#226.

From a Blink/Chromium internals perspective, things have not changed much,
and we still perform the same permission checks and connect to the wake lock
service via WakeLockStateRecord. The biggest change is that WakeLockController
was merged into WakeLock itself.

From an end-user perspective, though, the API now looks like a mix of the
old, navigator-based API and the one we implemented a few months ago:

* The main idea is that WakeLock.request() no longer returns a promise that
  does not resolve while the lock is held, as that is not very intuitive.
  Instead, it returns a new WakeLockSentinel object.
* WakeLockSentinel is a new interface that can release() a lock and receive
  "release" events.
* We do go back to a Navigator-based API, but we also support
  WorkerNavigator.
* There is no WakeLockRequest interface or state tracking in the WakeLock
  interface.
* WakeLock.requestPermission() was removed.
* Consequently, we now have a WakeLockEvent interface.

The changes to the spec still need to land, but there is some rough
consensus on what the API should look like, and we do not want to miss the
M79 branch cut.

Bug: 257511, 1010162
Change-Id: If1baed914ccd9eb7103a1bb71a40529976777ec2
chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 11, 2019
This is another big update to the implementation, which now follows the
API proposed in w3c/wake-lock#226.

From a Blink/Chromium internals perspective, things have not changed much,
and we still perform the same permission checks and connect to the wake lock
service via WakeLockStateRecord. The biggest change is that WakeLockController
was merged into WakeLock itself.

From an end-user perspective, though, the API now looks like a mix of the
old, navigator-based API and the one we implemented a few months ago:

* The main idea is that WakeLock.request() no longer returns a promise that
  does not resolve while the lock is held, as that is not very intuitive.
  Instead, it returns a new WakeLockSentinel object.
* WakeLockSentinel is a new interface that can release() a lock and receive
  "release" events.
* We do go back to a Navigator-based API, but we also support
  WorkerNavigator.
* There is no WakeLockRequest interface or state tracking in the WakeLock
  interface.
* WakeLock.requestPermission() was removed.
* Consequently, we now have a WakeLockEvent interface.

The changes to the spec still need to land, but there is some rough
consensus on what the API should look like, and we do not want to miss the
M79 branch cut.

Bug: 257511, 1010162
Change-Id: If1baed914ccd9eb7103a1bb71a40529976777ec2
chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 14, 2019
This is another big update to the implementation, which now follows the
API proposed in w3c/wake-lock#226.

From a Blink/Chromium internals perspective, things have not changed much,
and we still perform the same permission checks and connect to the wake lock
service via WakeLockStateRecord. The biggest change is that WakeLockController
was merged into WakeLock itself.

From an end-user perspective, though, the API now looks like a mix of the
old, navigator-based API and the one we implemented a few months ago:

* The main idea is that WakeLock.request() no longer returns a promise that
  does not resolve while the lock is held, as that is not very intuitive.
  Instead, it returns a new WakeLockSentinel object.
* WakeLockSentinel is a new interface that can release() a lock and receive
  "release" events.
* We do go back to a Navigator-based API, but we also support
  WorkerNavigator.
* There is no WakeLockRequest interface or state tracking in the WakeLock
  interface.
* WakeLock.requestPermission() was removed.
* Consequently, we now have a WakeLockEvent interface.

The changes to the spec still need to land, but there is some rough
consensus on what the API should look like, and we do not want to miss the
M79 branch cut.

Bug: 257511, 1010162
Change-Id: If1baed914ccd9eb7103a1bb71a40529976777ec2
chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 14, 2019
This is another big update to the implementation, which now follows the
API proposed in w3c/wake-lock#226.

From a Blink/Chromium internals perspective, things have not changed much,
and we still perform the same permission checks and connect to the wake lock
service via WakeLockStateRecord. The biggest change is that WakeLockController
was merged into WakeLock itself.

From an end-user perspective, though, the API now looks like a mix of the
old, navigator-based API and the one we implemented a few months ago:

* The main idea is that WakeLock.request() no longer returns a promise that
  does not resolve while the lock is held, as that is not very intuitive.
  Instead, it returns a new WakeLockSentinel object.
* WakeLockSentinel is a new interface that can release() a lock and receive
  "release" events.
* We do go back to a Navigator-based API, but we also support
  WorkerNavigator.
* There is no WakeLockRequest interface or state tracking in the WakeLock
  interface.
* WakeLock.requestPermission() was removed.
* Consequently, we now have a WakeLockEvent interface.

The changes to the spec still need to land, but there is some rough
consensus on what the API should look like, and we do not want to miss the
M79 branch cut.

Bug: 257511, 1010162
Change-Id: If1baed914ccd9eb7103a1bb71a40529976777ec2
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 14, 2019
This is another big update to the implementation, which now follows the
API proposed in w3c/wake-lock#226.

From a Blink/Chromium internals perspective, things have not changed much,
and we still perform the same permission checks and connect to the wake lock
service via WakeLockStateRecord. The biggest change is that WakeLockController
was merged into WakeLock itself.

From an end-user perspective, though, the API now looks like a mix of the
old, navigator-based API and the one we implemented a few months ago:

* The main idea is that WakeLock.request() no longer returns a promise that
  does not resolve while the lock is held, as that is not very intuitive.
  Instead, it returns a new WakeLockSentinel object.
* WakeLockSentinel is a new interface that can release() a lock and receive
  "release" events.
* We do go back to a Navigator-based API, but we also support
  WorkerNavigator.
* There is no WakeLockRequest interface or state tracking in the WakeLock
  interface.
* WakeLock.requestPermission() was removed.
* Consequently, we now have a WakeLockEvent interface.

The changes to the spec still need to land, but there is some rough
consensus on what the API should look like, and we do not want to miss the
M79 branch cut.

Bug: 257511, 1010162
Change-Id: If1baed914ccd9eb7103a1bb71a40529976777ec2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1849683
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#705632}
chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 14, 2019
This is another big update to the implementation, which now follows the
API proposed in w3c/wake-lock#226.

From a Blink/Chromium internals perspective, things have not changed much,
and we still perform the same permission checks and connect to the wake lock
service via WakeLockStateRecord. The biggest change is that WakeLockController
was merged into WakeLock itself.

From an end-user perspective, though, the API now looks like a mix of the
old, navigator-based API and the one we implemented a few months ago:

* The main idea is that WakeLock.request() no longer returns a promise that
  does not resolve while the lock is held, as that is not very intuitive.
  Instead, it returns a new WakeLockSentinel object.
* WakeLockSentinel is a new interface that can release() a lock and receive
  "release" events.
* We do go back to a Navigator-based API, but we also support
  WorkerNavigator.
* There is no WakeLockRequest interface or state tracking in the WakeLock
  interface.
* WakeLock.requestPermission() was removed.
* Consequently, we now have a WakeLockEvent interface.

The changes to the spec still need to land, but there is some rough
consensus on what the API should look like, and we do not want to miss the
M79 branch cut.

Bug: 257511, 1010162
Change-Id: If1baed914ccd9eb7103a1bb71a40529976777ec2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1849683
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#705632}
chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 14, 2019
This is another big update to the implementation, which now follows the
API proposed in w3c/wake-lock#226.

From a Blink/Chromium internals perspective, things have not changed much,
and we still perform the same permission checks and connect to the wake lock
service via WakeLockStateRecord. The biggest change is that WakeLockController
was merged into WakeLock itself.

From an end-user perspective, though, the API now looks like a mix of the
old, navigator-based API and the one we implemented a few months ago:

* The main idea is that WakeLock.request() no longer returns a promise that
  does not resolve while the lock is held, as that is not very intuitive.
  Instead, it returns a new WakeLockSentinel object.
* WakeLockSentinel is a new interface that can release() a lock and receive
  "release" events.
* We do go back to a Navigator-based API, but we also support
  WorkerNavigator.
* There is no WakeLockRequest interface or state tracking in the WakeLock
  interface.
* WakeLock.requestPermission() was removed.
* Consequently, we now have a WakeLockEvent interface.

The changes to the spec still need to land, but there is some rough
consensus on what the API should look like, and we do not want to miss the
M79 branch cut.

Bug: 257511, 1010162
Change-Id: If1baed914ccd9eb7103a1bb71a40529976777ec2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1849683
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#705632}
rakuco added a commit to rakuco/wake-lock that referenced this issue Oct 15, 2019
As discussed in w3c#226 and w3c#198, there are no implementations that currently
implement any UI that would be displayed when `requestPermission()` is
called, and we are looking at making its functionality available through
other means (see the discussion in w3c#198 for more details).
rakuco added a commit to rakuco/wake-lock that referenced this issue Oct 15, 2019
In preparation for the new API in w3c#226, we will get rid of all uses of
AbortSignal and have a separate WakeLockSentinel object instead.
rakuco added a commit to rakuco/wake-lock that referenced this issue Oct 15, 2019
rakuco added a commit to rakuco/wake-lock that referenced this issue Oct 16, 2019
Address w3c#226 by redefining the way script authors are supposed to interact
with the Wake Lock API.

The biggest issue with the previous iteration of the API is that it was
_too_ simple and ended up using the promise returned by `WakeLock.request()`
as a way to indicate whether a lock was being held or not. This is not
intuitive at all, as the promise would never resolve while the lock wasn't
released.

The new API is a compromise between the 2018 version of the API and the one
we had until now:

* We go back to adding a partial interface to `Navigator`, but we add a
  partial interface to `WorkerNavigator` as well.

* The `WakeLock` interface's only remaining method is no longer static, and
  instead of returning a `Promise<void>` it now returns a
  `Promise<WakeLockSentinel>`.

* `WakeLockSentinel` is a new interface that represents a handle to a
  platform wake lock returned by `WakeLock.request()`. It has a `release()`
  method and an `onrelease` event handler. `release()` replaces the previous
  `AbortController`-based infrastructure for releasing a lock, and
  `onrelease` lets users be notified when a lock is released (since they can
  also be released by the UA under circumstances such as loss of full
  activity).
  When all `WakeLockSentinel`s of a given type are released, the platform
  lock is released too.

* `WakeLockEvent` and its related interface `WakeLockEventInit` are
  delivered to `WakeLockSentinel`'s `onrelease` event handler, and allow
  users to also re-request a lock if necessary.
kenchris added a commit that referenced this issue Oct 16, 2019
As discussed in #226 and #198, there are no implementations that currently
implement any UI that would be displayed when `requestPermission()` is
called, and we are looking at making its functionality available through
other means (see the discussion in #198 for more details).
kenchris added a commit that referenced this issue Oct 16, 2019
Address #226 by redefining the way script authors are supposed to interact
with the Wake Lock API.

The biggest issue with the previous iteration of the API is that it was
_too_ simple and ended up using the promise returned by `WakeLock.request()`
as a way to indicate whether a lock was being held or not. This is not
intuitive at all, as the promise would never resolve while the lock wasn't
released.

The new API is a compromise between the 2018 version of the API and the one
we had until now:

* We go back to adding a partial interface to `Navigator`, but we add a
  partial interface to `WorkerNavigator` as well.

* The `WakeLock` interface's only remaining method is no longer static, and
  instead of returning a `Promise<void>` it now returns a
  `Promise<WakeLockSentinel>`.

* `WakeLockSentinel` is a new interface that represents a handle to a
  platform wake lock returned by `WakeLock.request()`. It has a `release()`
  method and an `onrelease` event handler. `release()` replaces the previous
  `AbortController`-based infrastructure for releasing a lock, and
  `onrelease` lets users be notified when a lock is released (since they can
  also be released by the UA under circumstances such as loss of full
  activity).
  When all `WakeLockSentinel`s of a given type are released, the platform
  lock is released too.

* `WakeLockEvent` and its related interface `WakeLockEventInit` are
  delivered to `WakeLockSentinel`'s `onrelease` event handler, and allow
  users to also re-request a lock if necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can鈥檛 perform that action at this time.