Skip to content

Commit

Permalink
DL: rendersubtree attribute
Browse files Browse the repository at this point in the history
Add the rendersubtree attribute (and renderSubtree property), accepting
the values "visible", "invisible", "invisible-activatable".

Will lock/unlock accordingly when the attribute is added/removed/changed
its value. Activation will set the attribute to "visible".

Note:
- acquire(), commit(), updateAndCommit() won't set/remove/change the
attribute at all. These functions (and Element.displayLock) will be
removed in another CL.
- Containment is not implied by this attribute and no DevTools error
for containment-related failures yet. Those will be
added in another CL.
- content-sizing is not currently supported. content-size CSS property
will be implemented in another CL.

See discussion:
WICG/display-locking#62 (comment)

Bug: 991095
Change-Id: I81ef6e6ff8bb5718dfb8ee09de701a08ed92062f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1735579
Reviewed-by: Chris Harrelson OOO <chrishtr@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685940}
  • Loading branch information
rakina authored and Commit Bot committed Aug 12, 2019
1 parent c1476d8 commit 7b47b5b
Show file tree
Hide file tree
Showing 53 changed files with 319 additions and 190 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -154,19 +154,13 @@ ScriptPromise DisplayLockContext::acquire(ScriptState* script_state,
? options->timeout()
: kDefaultLockTimeoutMs;

if (IsLocked()) {
// If we're locked, the activatable flag might change the activation
// blocking lock count. If we're not locked, the activation blocking lock
// count will be updated when we changed the state.
state_.UpdateActivationBlockingCount(activatable_,
options && options->activatable());
}
activatable_ = options && options->activatable();
// We always reschedule a timeout task even if we're not starting a new
// acquire. The reason for this is that the last acquire dictates the timeout
// interval. Note that the following call cancels any existing timeout tasks.
RescheduleTimeoutTask(timeout_ms);

// If we're acquiring something that isn't currently locked, we need to recalc
// the layout size. Otherwise if we're re-acquiring, we only need to recalc if
// the locked size has changed.
bool should_recalc_layout_size = !IsLocked();
// We need to recalc if the locked size has changed.
bool should_recalc_layout_size = false;
if (options && options->hasSize()) {
auto parsed_size = ParseAndVerifySize(options->size());
if (!parsed_size)
Expand All @@ -181,11 +175,6 @@ ScriptPromise DisplayLockContext::acquire(ScriptState* script_state,
locked_content_logical_size_ = LayoutSize();
}

// We always reschedule a timeout task even if we're not starting a new
// acquire. The reason for this is that the last acquire dictates the timeout
// interval. Note that the following call cancels any existing timeout tasks.
RescheduleTimeoutTask(timeout_ms);

if (should_recalc_layout_size && ConnectedToView()) {
if (auto* layout_object = element_->GetLayoutObject()) {
layout_object->SetNeedsLayoutAndPrefWidthsRecalc(
Expand All @@ -194,6 +183,8 @@ ScriptPromise DisplayLockContext::acquire(ScriptState* script_state,
ScheduleAnimation();
}

SetActivatable(options && options->activatable());

if (acquire_resolver_)
return acquire_resolver_->Promise();

Expand All @@ -209,39 +200,60 @@ ScriptPromise DisplayLockContext::acquire(ScriptState* script_state,
// property is false).
FinishCommitResolver(kResolve);

StartAcquire();

// If we're not connected, resolve immediately.
if (!ConnectedToView())
return GetResolvedPromise(script_state);

MakeResolver(script_state, &acquire_resolver_);
return acquire_resolver_->Promise();
}

void DisplayLockContext::SetActivatable(bool activatable) {
if (IsLocked()) {
// If we're locked, the activatable flag might change the activation
// blocking lock count. If we're not locked, the activation blocking lock
// count will be updated when we changed the state.
state_.UpdateActivationBlockingCount(activatable_, activatable);
}
activatable_ = activatable;
}

void DisplayLockContext::StartAcquire() {
DCHECK(!IsLocked());
update_budget_.reset();
state_ = kLocked;

// If we're already connected then we need to ensure that we update our style
// to check for containment later, layout size based on the options, and
// also clear the painted output.
if (ConnectedToView()) {
element_->SetNeedsStyleRecalc(
kLocalStyleChange,
StyleChangeReasonForTracing::Create(style_change_reason::kDisplayLock));
MakeResolver(script_state, &acquire_resolver_);
if (!ConnectedToView())
return;

element_->SetNeedsStyleRecalc(
kLocalStyleChange,
StyleChangeReasonForTracing::Create(style_change_reason::kDisplayLock));
ScheduleAnimation();

auto* layout_object = element_->GetLayoutObject();
if (!layout_object) {
is_horizontal_writing_mode_ = true;
if (auto* layout_object = element_->GetLayoutObject()) {
is_horizontal_writing_mode_ = layout_object->IsHorizontalWritingMode();
// GraphicsLayer collection would normally skip layers if paint is blocked
// by display-locking (see: CollectDrawableLayersForLayerListRecursively
// in LocalFrameView). However, if we don't trigger this collection, then
// we might use the cached result instead. In order to ensure we skip the
// newly locked layers, we need to set |need_graphics_layer_collection_|
// before marking the layer for repaint.
needs_graphics_layer_collection_ = true;
MarkPaintLayerNeedsRepaint();
}
// TODO(vmpstr): This needs to be set after invalidation above, since we
// want the object to layout once. After the changes to separate self and
// child layout, this would no longer be required and we can set the
// container as locked earlier.
state_ = kLocked;
return acquire_resolver_->Promise();
return;
}

// Otherwise (if we're not connected), resolve immediately.
return GetResolvedPromise(script_state);
layout_object->SetNeedsLayoutAndPrefWidthsRecalc(
layout_invalidation_reason::kDisplayLock);

is_horizontal_writing_mode_ = layout_object->IsHorizontalWritingMode();
// GraphicsLayer collection would normally skip layers if paint is blocked
// by display-locking (see: CollectDrawableLayersForLayerListRecursively
// in LocalFrameView). However, if we don't trigger this collection, then
// we might use the cached result instead. In order to ensure we skip the
// newly locked layers, we need to set |need_graphics_layer_collection_|
// before marking the layer for repaint.
needs_graphics_layer_collection_ = true;
MarkPaintLayerNeedsRepaint();
}

ScriptPromise DisplayLockContext::update(ScriptState* script_state) {
Expand Down Expand Up @@ -517,6 +529,10 @@ void DisplayLockContext::CommitForActivation() {
DCHECK(ConnectedToView());
DCHECK(ShouldCommitForActivation());
StartCommit();
// Since setting the attribute might trigger a commit if we are still locked,
// we set it after we start the commit.
if (element_->hasAttribute(html_names::kRendersubtreeAttr))
element_->setAttribute(html_names::kRendersubtreeAttr, "visible");
}

bool DisplayLockContext::ShouldCommitForActivation() const {
Expand Down Expand Up @@ -555,6 +571,7 @@ void DisplayLockContext::NotifyForcedUpdateScopeEnded() {
}

void DisplayLockContext::StartCommit() {
DCHECK(IsLocked());
// Since we are starting a commit, cancel the timeout task.
CancelTimeoutTask();
if (CleanupAndRejectCommitIfNotConnected())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ class CORE_EXPORT DisplayLockContext final
ScriptPromise commit(ScriptState*);
ScriptPromise updateAndCommit(ScriptState*);

void SetActivatable(bool activatable);

// Acquire the lock, should only be called when unlocked.
void StartAcquire();
// Initiate a commit.
void StartCommit();

enum LifecycleTarget { kSelf, kChildren };

// Lifecycle observation / state functions.
Expand Down Expand Up @@ -217,8 +224,6 @@ class CORE_EXPORT DisplayLockContext final
UntracedMember<DisplayLockContext> context_;
};

// Initiate a commit.
void StartCommit();
// Initiate an update.
void StartUpdateIfNeeded();

Expand Down
23 changes: 23 additions & 0 deletions third_party/blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1927,6 +1927,24 @@ void Element::AttributeChanged(const AttributeModificationParams& params) {
name == html_names::kInvisibleAttr &&
params.old_value != params.new_value) {
InvisibleAttributeChanged(params.old_value, params.new_value);
} else if (RuntimeEnabledFeatures::DisplayLockingEnabled() &&
name == html_names::kRendersubtreeAttr &&
params.old_value != params.new_value) {
const bool should_be_invisible =
EqualIgnoringASCIICase(params.new_value, "invisible");
const bool should_be_invisible_activatable =
EqualIgnoringASCIICase(params.new_value, "invisible-activatable");
if (should_be_invisible || should_be_invisible_activatable) {
// Getting locked.
EnsureDisplayLockContext().SetActivatable(
should_be_invisible_activatable);
if (!GetDisplayLockContext()->IsLocked())
GetDisplayLockContext()->StartAcquire();
} else {
// Getting unlocked.
if (EnsureDisplayLockContext().IsLocked())
GetDisplayLockContext()->StartCommit();
}
}
}

Expand Down Expand Up @@ -4129,6 +4147,11 @@ DisplayLockContext* Element::GetDisplayLockContext() const {
: nullptr;
}

DisplayLockContext& Element::EnsureDisplayLockContext() {
return *EnsureElementRareData().EnsureDisplayLockContext(
this, GetExecutionContext());
}

// Step 1 of http://domparsing.spec.whatwg.org/#insertadjacenthtml()
static Element* ContextElementForInsertion(const String& where,
Element* element,
Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/dom/element.h
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,7 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {

DisplayLockContext* getDisplayLockForBindings();
DisplayLockContext* GetDisplayLockContext() const;
DisplayLockContext& EnsureDisplayLockContext();

bool StyleRecalcBlockedByDisplayLock(
DisplayLockContext::LifecycleTarget) const;
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/dom/element.idl
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ callback ScrollStateCallback = void (ScrollState scrollState);

// Display locking. Returns a display lock context.
[RuntimeEnabled=DisplayLocking, ImplementedAs=getDisplayLockForBindings] readonly attribute DisplayLockContext displayLock;
// Declarative display locking.
[RuntimeEnabled=DisplayLocking, CEReactions, CustomElementCallbacks, Reflect, ReflectOnly=("invisible-activatable","invisible", "visible"), ReflectInvalid="visible", ReflectEmpty="visible"] attribute DOMString renderSubtree;

// Element Timing
[RuntimeEnabled=ElementTiming, Affects=Nothing, CEReactions, Reflect=elementtiming] attribute DOMString elementTiming;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@
"preload",
"pseudo",
"readonly",
"rendersubtree",
"referrerpolicy",
"rel",
"required",
Expand Down
5 changes: 3 additions & 2 deletions third_party/blink/web_tests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,12 @@ crbug.com/882663 inspector-protocol/css [ Skip ]

# Display locking failures
crbug.com/955533 wpt_internal/display-lock/sizing/overflow-auto-with-overflow.html [ Failure ]
# Skip some tests for rendersubtree
crbug.com/991095 wpt_internal/display-lock/rendersubtree/containment [ Skip ]
crbug.com/991095 wpt_internal/display-lock/rendersubtree/sizing [ Skip ]

# virtual-scroller failures
crbug.com/986574 wpt_internal/virtual-scroller/ [ Failure ]
# Skip tests for rendersubtree for now.
crbug.com/991095 wpt_internal/display-lock/rendersubtree/ [ Skip ]

# Sheriff 2018/05/25
crbug.com/846747 http/tests/navigation/navigation-interrupted-by-fragment.html [ Pass Timeout ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ namespace http://www.w3.org/1999/xhtml
property removeAttributeNode
property removeChild
property removeEventListener
property renderSubtree
property replaceChild
property replaceWith
property requestFullscreen
Expand Down Expand Up @@ -1454,6 +1455,7 @@ namespace http://www.w3.org/2000/svg
property removeAttributeNode
property removeChild
property removeEventListener
property renderSubtree
property replaceChild
property replaceWith
property requestFullscreen
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2019,6 +2019,7 @@ interface Element : Node
getter part
getter prefix
getter previousElementSibling
getter renderSubtree
getter role
getter scrollHeight
getter scrollLeft
Expand Down Expand Up @@ -2141,6 +2142,7 @@ interface Element : Node
setter onwebkitfullscreenerror
setter outerHTML
setter part
setter renderSubtree
setter role
setter scrollLeft
setter scrollTop
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<link rel="help" href="https://github.com/WICG/display-locking">
<link rel="match" href="pass-container-with-child-ref.html">
<script src="/common/reftest-wait.js"></script>
<script src="resources/utils.js"></script>

<style>
#container {
Expand Down Expand Up @@ -33,12 +34,12 @@

function runTest() {
const container = document.getElementById("container");
container.displayLock.acquire({ timeout: Infinity }).then(() => {
setInvisible(container).then(() => {
const child = document.createElement("div");
child.id = "child";
container.appendChild(child);

container.displayLock.commit().then(
setVisible(container).then(
() => { finishTest("PASS"); },
(e) => { finishTest("FAIL " + e.message); });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<link rel="help" href="https://github.com/WICG/display-locking">
<link rel="match" href="pass-container-with-child-ref.html">
<script src="/common/reftest-wait.js"></script>
<script src="resources/utils.js"></script>

<style>
#container {
Expand Down Expand Up @@ -33,13 +34,13 @@

function runTest() {
const container = document.getElementById("container");
container.displayLock.acquire({ timeout: Infinity });
setInvisible(container);

const child = document.createElement("div");
child.id = "child";
container.appendChild(child);

container.displayLock.commit().then(
setVisible(container).then(
() => { finishTest("PASS"); },
(e) => { finishTest("FAIL " + e.message); });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<link rel="help" href="https://github.com/WICG/display-locking">
<link rel="match" href="pass-container-with-child-ref.html">
<script src="/common/reftest-wait.js"></script>
<script src="resources/utils.js"></script>

<style>
#container {
Expand Down Expand Up @@ -33,7 +34,7 @@

function runTest() {
const container = document.getElementById("container");
container.displayLock.acquire({ timeout: Infinity });
setInvisible(container);

const child = document.createElement("div");
child.id = "child";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<link rel="help" href="https://github.com/WICG/display-locking">
<link rel="match" href="acquire-in-breakable-div-ref.html">
<script src="/common/reftest-wait.js"></script>
<script src="resources/utils.js"></script>

<style>
#container {
Expand All @@ -28,7 +29,7 @@
<script>
async function runTest() {
const container = document.getElementById("container");
await container.displayLock.acquire({ timeout: Infinity });
await setInvisible(container);
takeScreenshot();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<link rel="help" href="https://github.com/WICG/display-locking">
<link rel="match" href="acquire-in-iframe-ref.html">
<script src="/common/reftest-wait.js"></script>
<script src="resources/utils.js"></script>

<div id="log"></div>
<iframe id="frame" srcdoc='
Expand All @@ -28,7 +29,7 @@

function runTest() {
const container = document.getElementById("frame").contentDocument.getElementById("container");
container.displayLock.acquire({ timeout: Infinity }).then(
setInvisible(container).then(
() => { finishTest("PASS"); },
(e) => { finishTest("FAIL " + e.message); });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<link rel="help" href="https://github.com/WICG/display-locking">
<link rel="match" href="pass-ref.html">
<script src="/common/reftest-wait.js"></script>
<script src="resources/utils.js"></script>

<style>
#container {
Expand All @@ -28,7 +29,7 @@

function runTest() {
const container = document.getElementById("container");
container.displayLock.acquire({ timeout: Infinity }).then(
setInvisible(container).then(
() => { finishTest("PASS"); },
(e) => { finishTest("FAIL " + e.message); }
);
Expand Down

0 comments on commit 7b47b5b

Please sign in to comment.