Skip to content

GitLab

T
tangled
Commit c499dbdf authored by Rakina Zata Amni's avatar Rakina Zata Amni Committed by Commit Bot
Browse files
Options

[DL]: Handle commit() calls that happen on the same frame as activation 

It's possible to be in a committing state without having a commit
promise resolver, if the commit is caused by activation. This causes
problems with commit() calls that happen on the same frame as
activation-induced commit, as it expects the resolver to exist.

This CL handles that case and adds a test.

Bug: 953614
Change-Id: I83ba0b6e450672588ff960389fc045be4896fe32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1573425Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Reviewed-by: default avatarFergal Daly <fergal@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652473}
parent 12505b8e
Hide whitespace changes
Inline Side-by-side
Showing with 159 additions and 16 deletions
third_party/blink/renderer/core/display_lock/display_lock_context.cc
View file @ c499dbdf
...@@ -246,27 +246,24 @@ ScriptPromise DisplayLockContext::commit(ScriptState* script_state) { ...@@ -246,27 +246,24 @@ ScriptPromise DisplayLockContext::commit(ScriptState* script_state) {
if (state_ == kUnlocked) if (state_ == kUnlocked)
return GetResolvedPromise(script_state); return GetResolvedPromise(script_state);
// If we're already committing then return the promise. // If we're already committing and it's not because of activation,
if (state_ == kCommitting) // we would already have the commit promise and should just return it.
if (commit_resolver_) {
DCHECK(state_ == kUpdating || state_ == kCommitting);
return commit_resolver_->Promise(); return commit_resolver_->Promise();
}
// Now that we've explicitly been requested to commit, we have cancel the
// timeout task.
CancelTimeoutTask();
// Note that we don't resolve the update promise here, since it should still // Note that we don't resolve the update promise here, since it should still
// finish updating before resolution. That is, calling update() and commit() // finish updating before resolution. That is, calling update() and commit()
// together will still wait until the lifecycle is clean before resolving any // together will still wait until the lifecycle is clean before resolving any
// of the promises. // of the promises.
DCHECK_NE(state_, kCommitting);
// We might already have a resolver if we called updateAndCommit() before commit_resolver_ = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
// this.
if (!commit_resolver_) {
commit_resolver_ =
MakeGarbageCollected<ScriptPromiseResolver>(script_state);
}
auto promise = commit_resolver_->Promise(); auto promise = commit_resolver_->Promise();
StartCommit(); // It's possible we are already committing due to activation. If not, we
// should start the commit.
if (state_ != kCommitting)
StartCommit();
return promise; return promise;
} }
...@@ -496,12 +493,13 @@ void DisplayLockContext::NotifyForcedUpdateScopeEnded() { ...@@ -496,12 +493,13 @@ void DisplayLockContext::NotifyForcedUpdateScopeEnded() {
} }
void DisplayLockContext::StartCommit() { void DisplayLockContext::StartCommit() {
// Since we are starting a commit, cancel the timeout task.
CancelTimeoutTask();
// If we don't have an element or we're not connected, then the process of // If we don't have an element or we're not connected, then the process of
// committing is the same as just unlocking the element. // committing is the same as just unlocking the element.
if (!element_ || !ConnectedToView()) { if (!element_ || !ConnectedToView()) {
state_ = kUnlocked; state_ = kUnlocked;
update_budget_.reset(); update_budget_.reset();
CancelTimeoutTask();
// Note that we reject the update, but resolve the commit. // Note that we reject the update, but resolve the commit.
FinishUpdateResolver(kReject, rejection_names::kElementIsDisconnected); FinishUpdateResolver(kReject, rejection_names::kElementIsDisconnected);
FinishCommitResolver(kResolve); FinishCommitResolver(kResolve);
...@@ -515,7 +513,6 @@ void DisplayLockContext::StartCommit() { ...@@ -515,7 +513,6 @@ void DisplayLockContext::StartCommit() {
if (acquire_resolver_) { if (acquire_resolver_) {
FinishAcquireResolver(kReject, rejection_names::kLockCommitted); FinishAcquireResolver(kReject, rejection_names::kLockCommitted);
FinishCommitResolver(kResolve); FinishCommitResolver(kResolve);
CancelTimeoutTask();
state_ = kUnlocked; state_ = kUnlocked;
} else if (state_ != kUpdating) { } else if (state_ != kUpdating) {
ScheduleAnimation(); ScheduleAnimation();
......
third_party/blink/web_tests/display-lock/activation/activate-commit-same-frame.html 0 → 100644
View file @ c499dbdf
<!doctype HTML>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<style>
.spacer {
width: 150px;
height: 3000px;
background: lightblue;
}
#container {
contain: style layout;
width: 150px;
height: 150px;
background: lightgreen;
}
#target {
width: 100px;
height: 100px;
background: green;
}
</style>
<div class="spacer"></div>
<div id="container"><div id="target"></div></div>
<script>
async_test((t) => {
async function runTest() {
let container = document.getElementById("container");
let acquire_promise = container.displayLock.acquire({ timeout: Infinity, activatable: true });
await acquire_promise;
target.scrollIntoView();
let commit_promise = container.displayLock.commit();
await commit_promise;
t.step(() => assert_false(container.displayLock.locked, "context after commit & activation is unlocked"));
t.done();
}
window.onload = function() {
requestAnimationFrame(() => requestAnimationFrame(runTest));
};
}, "scrollIntoView and committing on the same frame should work");
</script>
third_party/blink/web_tests/display-lock/activation/activate-update-and-commit-same-frame.html 0 → 100644
View file @ c499dbdf
<!doctype HTML>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<style>
.spacer {
width: 150px;
height: 3000px;
background: lightblue;
}
#container {
contain: style layout;
width: 150px;
height: 150px;
background: lightgreen;
}
#target {
width: 100px;
height: 100px;
background: green;
}
</style>
<div class="spacer"></div>
<div id="container"><div id="target"></div></div>
<script>
async_test((t) => {
async function runTest() {
let container = document.getElementById("container");
let acquire_promise = container.displayLock.acquire({ timeout: Infinity, activatable: true });
await acquire_promise;
target.scrollIntoView();
let update_and_commit_promise = container.displayLock.updateAndCommit();
await update_and_commit_promise;
t.step(() => assert_false(container.displayLock.locked, "context after update & activation is unlocked"));
t.done();
}
window.onload = function() {
requestAnimationFrame(() => requestAnimationFrame(runTest));
};
}, "scrollIntoView and calling updateAndCommit on the same frame should work");
</script>
third_party/blink/web_tests/display-lock/activation/activate-update-same-frame.html 0 → 100644
View file @ c499dbdf
<!doctype HTML>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<style>
.spacer {
width: 150px;
height: 3000px;
background: lightblue;
}
#container {
contain: style layout;
width: 150px;
height: 150px;
background: lightgreen;
}
#target {
width: 100px;
height: 100px;
background: green;
}
</style>
<div class="spacer"></div>
<div id="container"><div id="target"></div></div>
<script>
async_test((t) => {
async function runTest() {
let container = document.getElementById("container");
let acquire_promise = container.displayLock.acquire({ timeout: Infinity, activatable: true });
await acquire_promise;
target.scrollIntoView();
let update_promise = container.displayLock.update();
await update_promise;
t.step(() => assert_false(container.displayLock.locked, "context after update & activation is unlocked"));
t.done();
}
window.onload = function() {
requestAnimationFrame(() => requestAnimationFrame(runTest));
};
}, "scrollIntoView and updating on the same frame should work");
</script>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment