[DL]: Ensure to reject update/commit promises on disconnected elements.
This patch ensures that we reject update/commit when it happens on
a disconnected element.
R=chrishtr@chromium.org
Bug: 882663
Change-Id: Iaee923a4f41714b437c0a2b85bbb175359a2f32c
Reviewed-on: https://chromium-review.googlesource.com/c/1387944
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618713}diff --git a/third_party/blink/renderer/core/display_lock/display_lock_context.cc b/third_party/blink/renderer/core/display_lock/display_lock_context.cc
index 5e631a67..d58b177 100644
--- a/third_party/blink/renderer/core/display_lock/display_lock_context.cc
+++ b/third_party/blink/renderer/core/display_lock/display_lock_context.cc
@@ -139,8 +139,8 @@
}
ScriptPromise DisplayLockContext::update(ScriptState* script_state) {
- // Reject if we're unlocked.
- if (state_ == kUnlocked)
+ // Reject if we're unlocked or disconnected.
+ if (state_ == kUnlocked || !element_->isConnected())
return GetRejectedPromise(script_state);
// If we have a resolver, then we're at least updating already, just return
@@ -151,16 +151,13 @@
}
update_resolver_ = ScriptPromiseResolver::Create(script_state);
- // We only need to kick off an Update if we're in a locked state, all other
- // states are already updating.
- if (state_ == kLocked)
- StartUpdate();
+ StartUpdateIfNeeded();
return update_resolver_->Promise();
}
ScriptPromise DisplayLockContext::commit(ScriptState* script_state) {
- // Reject if we're unlocked.
- if (state_ == kUnlocked)
+ // Reject if we're unlocked or disonnected.
+ if (state_ == kUnlocked || !element_->isConnected())
return GetRejectedPromise(script_state);
// If we have a resolver, we must be committing already, just return the same
@@ -391,12 +388,19 @@
layout_invalidation_reason::kDisplayLockCommitting);
}
-void DisplayLockContext::StartUpdate() {
- DCHECK_EQ(state_, kLocked);
- state_ = kUpdating;
+void DisplayLockContext::StartUpdateIfNeeded() {
+ // We should not be calling this if we're unlocked.
+ DCHECK_NE(state_, kUnlocked);
+ // Any state other than kLocked means that we are already in the process of
+ // updating/committing, so we can piggy back on that process without kicking
+ // off any new updates.
+ if (state_ != kLocked)
+ return;
+
// We don't need to mark anything dirty since the budget will take care of
// that for us.
update_budget_ = CreateNewBudget();
+ state_ = kUpdating;
ScheduleAnimation();
}
@@ -509,6 +513,8 @@
}
void DisplayLockContext::ScheduleAnimation() {
+ DCHECK(element_->isConnected());
+
// Schedule an animation to perform the lifecycle phases.
element_->GetDocument().GetPage()->Animator().ScheduleVisualUpdate(
element_->GetDocument().GetFrame());
diff --git a/third_party/blink/renderer/core/display_lock/display_lock_context.h b/third_party/blink/renderer/core/display_lock/display_lock_context.h
index b8956f9..06c767c 100644
--- a/third_party/blink/renderer/core/display_lock/display_lock_context.h
+++ b/third_party/blink/renderer/core/display_lock/display_lock_context.h
@@ -149,7 +149,7 @@
// Initiate a commit.
void StartCommit();
// Initiate an update.
- void StartUpdate();
+ void StartUpdateIfNeeded();
// The following functions propagate dirty bits from the locked element up to
// the ancestors in order to be reached. They return true if the element or
diff --git a/third_party/blink/web_tests/display-lock/lock-before-append/commit-while-disconnected-expected.html b/third_party/blink/web_tests/display-lock/lock-before-append/commit-while-disconnected-expected.html
new file mode 100644
index 0000000..1333caeb
--- /dev/null
+++ b/third_party/blink/web_tests/display-lock/lock-before-append/commit-while-disconnected-expected.html
@@ -0,0 +1,4 @@
+<!doctype HTML>
+
+<div id="log">PASS</div>
+
diff --git a/third_party/blink/web_tests/display-lock/lock-before-append/commit-while-disconnected.html b/third_party/blink/web_tests/display-lock/lock-before-append/commit-while-disconnected.html
new file mode 100644
index 0000000..a5643b93
--- /dev/null
+++ b/third_party/blink/web_tests/display-lock/lock-before-append/commit-while-disconnected.html
@@ -0,0 +1,33 @@
+<!doctype HTML>
+
+<!--
+Runs an acquire(), then commits without connecting the element.
+-->
+
+<div id="log"></div>
+
+<script>
+// TODO(vmpstr): In WPT this needs to be replaced with reftest-wait.
+if (window.testRunner)
+ window.testRunner.waitUntilDone();
+
+function finishTest(status_string) {
+ if (document.getElementById("log").innerHTML === "")
+ document.getElementById("log").innerHTML = status_string;
+ if (window.testRunner)
+ window.testRunner.notifyDone();
+}
+
+function runTest() {
+ let container = document.createElement("div");
+ container.id = "container";
+
+ container.getDisplayLock().acquire({ timeout: Infinity }).then(() => {
+ container.getDisplayLock().commit().then(
+ () => { finishTest("FAIL"); },
+ () => { finishTest("PASS"); });
+ });
+}
+
+window.onload = runTest;
+</script>
diff --git a/third_party/blink/web_tests/display-lock/lock-before-append/update-while-disconnected-expected.html b/third_party/blink/web_tests/display-lock/lock-before-append/update-while-disconnected-expected.html
new file mode 100644
index 0000000..1333caeb
--- /dev/null
+++ b/third_party/blink/web_tests/display-lock/lock-before-append/update-while-disconnected-expected.html
@@ -0,0 +1,4 @@
+<!doctype HTML>
+
+<div id="log">PASS</div>
+
diff --git a/third_party/blink/web_tests/display-lock/lock-before-append/update-while-disconnected.html b/third_party/blink/web_tests/display-lock/lock-before-append/update-while-disconnected.html
new file mode 100644
index 0000000..9f0e4ed
--- /dev/null
+++ b/third_party/blink/web_tests/display-lock/lock-before-append/update-while-disconnected.html
@@ -0,0 +1,33 @@
+<!doctype HTML>
+
+<!--
+Runs an acquire(), then updates without connecting the element.
+-->
+
+<div id="log"></div>
+
+<script>
+// TODO(vmpstr): In WPT this needs to be replaced with reftest-wait.
+if (window.testRunner)
+ window.testRunner.waitUntilDone();
+
+function finishTest(status_string) {
+ if (document.getElementById("log").innerHTML === "")
+ document.getElementById("log").innerHTML = status_string;
+ if (window.testRunner)
+ window.testRunner.notifyDone();
+}
+
+function runTest() {
+ let container = document.createElement("div");
+ container.id = "container";
+
+ container.getDisplayLock().acquire({ timeout: Infinity }).then(() => {
+ container.getDisplayLock().update().then(
+ () => { finishTest("FAIL"); },
+ () => { finishTest("PASS"); });
+ });
+}
+
+window.onload = runTest;
+</script>
