New issue
Advanced search Search tips

Issue 828990 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 161072



Sign in to add a comment

Lock API - Aborting after lock acquired should be a noop

Project Member Reported by ralp...@google.com, Apr 4 2018

Issue description

Chrome Version: 66.0.3359.79
OS: All

Aborting a lock request after the lock is acquired should not reject the request promise.
See spec discussion: https://github.com/inexorabletash/web-locks/issues/34#issuecomment-363898166


What steps will reproduce the problem?

Paste this in the console:

const controller = new AbortController();
// We will abort after 5 seconds
setTimeout(() => controller.abort(), 5000);
// Requesting the lock
navigator.locks.request('test', {signal: controller.signal}, (lock) => {
    console.log('Acquired the lock - starting work');
    return new Promise((resolve, reject) => {
        setTimeout(resolve, 10000);
    }).then(() => {
        console.log('Finished work');
    })
}).then(() => {
  console.log('request promise resolved')
}, (e) => {
  console.log('request promise rejected');
  console.log(e.name);
});

What is the expected result?
Should print:
 - Acquired the lock - starting work
 - Finished work
 - request promise resolved

What happens instead?
It prints:
 - Acquired the lock - starting work
 - request promise rejected
 - AbortError
 - Finished work


 

Comment 1 by ralp...@google.com, Apr 4 2018

Description: Show this description
Status: Assigned (was: Untriaged)
Thanks! Will fix.

Blocking: 161072
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b57aa5ab06b239c1b074b7adbaa017e91ba82e2

commit 6b57aa5ab06b239c1b074b7adbaa017e91ba82e2
Author: Joshua Bell <jsbell@chromium.org>
Date: Fri Apr 13 20:53:20 2018

Web Locks API: Abort after lock is granted should not cause rejection

An abort signal before a lock is granted causes the promise returned
by navigator.locks.request() to reject. But if the signal comes in
after the lock is granted, the signal should not change the promise
behavior (it resolves with the result of the callback function).

The existing test was insufficient to catch a bug here; fix the bug
(no-op the abort if it comes in too late) and improve test coverage.

Bug:  828990 
Change-Id: I1b056a05ba49e284fb8eea76bd9652a904232c60
Reviewed-on: https://chromium-review.googlesource.com/999828
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550755}
[modify] https://crrev.com/6b57aa5ab06b239c1b074b7adbaa017e91ba82e2/third_party/WebKit/LayoutTests/http/tests/locks/signal.html
[modify] https://crrev.com/6b57aa5ab06b239c1b074b7adbaa017e91ba82e2/third_party/blink/renderer/modules/locks/lock_manager.cc
[modify] https://crrev.com/6b57aa5ab06b239c1b074b7adbaa017e91ba82e2/third_party/blink/renderer/modules/locks/lock_manager.h

Comment 7 by jsb...@chromium.org, Apr 13 2018

Status: Fixed (was: Started)
Missed the 67 cut, unfortunately, but should be in the next canary.
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b57aa5ab06b239c1b074b7adbaa017e91ba82e2

commit 6b57aa5ab06b239c1b074b7adbaa017e91ba82e2
Author: Joshua Bell <jsbell@chromium.org>
Date: Fri Apr 13 20:53:20 2018

Web Locks API: Abort after lock is granted should not cause rejection

An abort signal before a lock is granted causes the promise returned
by navigator.locks.request() to reject. But if the signal comes in
after the lock is granted, the signal should not change the promise
behavior (it resolves with the result of the callback function).

The existing test was insufficient to catch a bug here; fix the bug
(no-op the abort if it comes in too late) and improve test coverage.

Bug:  828990 
Change-Id: I1b056a05ba49e284fb8eea76bd9652a904232c60
Reviewed-on: https://chromium-review.googlesource.com/999828
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550755}
[modify] https://crrev.com/6b57aa5ab06b239c1b074b7adbaa017e91ba82e2/third_party/WebKit/LayoutTests/http/tests/locks/signal.html
[modify] https://crrev.com/6b57aa5ab06b239c1b074b7adbaa017e91ba82e2/third_party/blink/renderer/modules/locks/lock_manager.cc
[modify] https://crrev.com/6b57aa5ab06b239c1b074b7adbaa017e91ba82e2/third_party/blink/renderer/modules/locks/lock_manager.h

Sign in to add a comment