Skip to content

Commit

Permalink
VT: Don't make cv auto with vt descendants relevant.
Browse files Browse the repository at this point in the history
This patch aligns with latest resolution and removes the code that
makes content-visibility: auto elements with view transition descendants
relevant to the user.

R=khushalsagar@chromium.org

Fixed: 1409132
Change-Id: Ic2f03c7e5a8c33c83a8769baba7b0094327e72d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4198788
Reviewed-by: Khushal Sagar <khushalsagar@chromium.org>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1098635}
  • Loading branch information
vmpstr authored and Chromium LUCI CQ committed Jan 30, 2023
1 parent 559733e commit a7b8baf
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ DisplayLockContext::DisplayLockContext(Element* element)
DetermineIfSubtreeHasFocus();
DetermineIfSubtreeHasSelection();
DetermineIfSubtreeHasTopLayerElement();
DetermineIfInViewTransitionElementChain();
DetermineIfDescendantIsViewTransitionElement();
}

void DisplayLockContext::SetRequestedState(EContentVisibility state,
Expand Down Expand Up @@ -887,7 +887,7 @@ void DisplayLockContext::DidMoveToNewDocument(Document& old_document) {
DetermineIfSubtreeHasFocus();
DetermineIfSubtreeHasSelection();
DetermineIfSubtreeHasTopLayerElement();
DetermineIfInViewTransitionElementChain();
DetermineIfDescendantIsViewTransitionElement();
}

void DisplayLockContext::WillStartLifecycleUpdate(const LocalFrameView& view) {
Expand Down Expand Up @@ -1162,55 +1162,22 @@ void DisplayLockContext::DetermineIfSubtreeHasTopLayerElement() {
}
}

void DisplayLockContext::DetermineIfInViewTransitionElementChain() {
ResetAndDetermineIfAncestorIsViewTransitionElement();
void DisplayLockContext::DetermineIfDescendantIsViewTransitionElement() {
ResetDescendantIsViewTransitionElement();
if (ConnectedToView()) {
document_->GetDisplayLockDocumentState()
.UpdateViewTransitionElementAncestorLocks();
}
}

void DisplayLockContext::ResetInViewTransitionElementChain() {
SetRenderAffectingState(RenderAffectingState::kViewTransitionElementChain,
false);
}

void DisplayLockContext::SetInViewTransitionElementChain() {
SetRenderAffectingState(RenderAffectingState::kViewTransitionElementChain,
true);
}

bool DisplayLockContext::IsInViewTransitionElementAncestorChain() const {
return render_affecting_state_[static_cast<int>(
RenderAffectingState::kViewTransitionElementChain)];
void DisplayLockContext::ResetDescendantIsViewTransitionElement() {
SetRenderAffectingState(
RenderAffectingState::kDescendantIsViewTransitionElement, false);
}

void DisplayLockContext::ResetAndDetermineIfAncestorIsViewTransitionElement() {
ResetInViewTransitionElementChain();
if (!ConnectedToView())
return;

auto* transition = ViewTransitionUtils::GetActiveTransition(*document_);
if (!transition)
return;

bool has_view_transition_element_ancestor = false;
for (auto* candidate = element_.Get(); candidate;
candidate = FlatTreeTraversal::ParentElement(*candidate)) {
// We don't care about document element as the ancestor, since it's common
// to have one and it will be clipped by viewport anyway.
if (candidate->IsDocumentElement())
continue;

if (auto* layout_object = candidate->GetLayoutObject();
layout_object &&
transition->IsRepresentedViaPseudoElements(*layout_object)) {
has_view_transition_element_ancestor = true;
break;
}
}
SetRenderAffectingState(RenderAffectingState::kViewTransitionElementChain,
has_view_transition_element_ancestor);
void DisplayLockContext::SetDescendantIsViewTransitionElement() {
SetRenderAffectingState(
RenderAffectingState::kDescendantIsViewTransitionElement, true);
}

void DisplayLockContext::ClearHasTopLayerElement() {
Expand Down Expand Up @@ -1333,7 +1300,7 @@ void DisplayLockContext::NotifyRenderAffectingStateChanged() {
!state(RenderAffectingState::kAutoStateUnlockedUntilLifecycle) &&
!state(RenderAffectingState::kAutoUnlockedForPrint) &&
!state(RenderAffectingState::kSubtreeHasTopLayerElement) &&
!state(RenderAffectingState::kViewTransitionElementChain)));
!state(RenderAffectingState::kDescendantIsViewTransitionElement)));

if (should_be_locked && !IsLocked())
Lock();
Expand Down Expand Up @@ -1367,8 +1334,8 @@ const char* DisplayLockContext::RenderAffectingStateName(int state) const {
return "AutoUnlockedForPrint";
case RenderAffectingState::kSubtreeHasTopLayerElement:
return "SubtreeHasTopLayerElement";
case RenderAffectingState::kViewTransitionElementChain:
return "ViewTransitionElementChain";
case RenderAffectingState::kDescendantIsViewTransitionElement:
return "DescendantIsViewTransitionElement";
case RenderAffectingState::kNumRenderAffectingStates:
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,30 +249,9 @@ class CORE_EXPORT DisplayLockContext final

void ScheduleTopLayerCheck();

// This updates the rendering state to account for the fact that one of the
// ancestor may be a non-root view transition element, which should cause the
// content-visibility: auto locks to be unlocked.
// This function is called anytime a descendant or ancestor view transition
// element may change. Note that to determine the descendants, this function
// uses a document level function to mark all ancestors of view transition
// elements. This updates all display locks on such ancestor chains, but it
// should be a no-op for any lock except this one. This is the most optimal
// way to do this and not a necessary component of the function.
// Note that this function also does not consider the root as a view
// transition element (even though it might be). The reason for this is that
// root is treated different in View Transitions: it is clipped by a viewport
// or some margin around, and it's captured by default. This means that it
// will frequently be in the chain of all display locks, and we want to avoid
// unnecessary unlocks.
void DetermineIfInViewTransitionElementChain();
// Note that the following only checks the ancestor chain, and does not
// consider view transition descendants. This is an optimization to be used by
// the document state.
void ResetAndDetermineIfAncestorIsViewTransitionElement();
// State control for view transition element render affecting state.
void ResetInViewTransitionElementChain();
void SetInViewTransitionElementChain();
bool IsInViewTransitionElementAncestorChain() const;
void ResetDescendantIsViewTransitionElement();
void SetDescendantIsViewTransitionElement();

private:
// Give access to |NotifyForcedUpdateScopeStarted()| and
Expand Down Expand Up @@ -378,6 +357,10 @@ class CORE_EXPORT DisplayLockContext final
// top layer node up the ancestor chain looking for `element_`.
void DetermineIfSubtreeHasTopLayerElement();

// Determines if there are view transition elements in the subtree of this
// element.
void DetermineIfDescendantIsViewTransitionElement();

// Detaching the layout tree from the top layers nested under this lock.
void DetachDescendantTopLayerElements();

Expand Down Expand Up @@ -521,7 +504,7 @@ class CORE_EXPORT DisplayLockContext final
kAutoStateUnlockedUntilLifecycle,
kAutoUnlockedForPrint,
kSubtreeHasTopLayerElement,
kViewTransitionElementChain,
kDescendantIsViewTransitionElement,
kNumRenderAffectingStates
};
void SetRenderAffectingState(RenderAffectingState state, bool flag);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,21 +252,14 @@ bool DisplayLockDocumentState::MarkAncestorContextsHaveTopLayerElement(
}

void DisplayLockDocumentState::NotifyViewTransitionPseudoTreeChanged() {
// Note that this function doesn't use
// DisplayLockContext::DetermineIfInViewTransitionElementChain, since that
// would mean we have to call UpdateViewTransitionElementAncestorLocks for
// each lock. This function only calls it once by hoisting it out of the
// context calls.
// TODO(crbug.com/1409132): This needs to be updated to match the new desired
// behavior for content-visibility: auto in a snapshot descendent.

// Reset the flag and determine if the ancestor is a view transition element.
// Reset the view transition element flag.
// TODO(vmpstr): This should be optimized to keep track of elements that
// actually have this flag set.
for (auto context : display_lock_contexts_)
context->ResetAndDetermineIfAncestorIsViewTransitionElement();
context->ResetDescendantIsViewTransitionElement();

// Also process the view transition elements to check if their ancestors are
// locks. These two parts give us the full chain (either locks are ancestors
// of transition element or transition elements are ancestors of locks).
// Process the view transition elements to check if their ancestors are
// locks that need to be made relevant.
UpdateViewTransitionElementAncestorLocks();
}

Expand All @@ -279,16 +272,10 @@ void DisplayLockDocumentState::UpdateViewTransitionElementAncestorLocks() {
for (auto element : transitioning_elements) {
auto* ancestor = element.Get();
// When the element which has c-v:auto is itself a view transition element,
// marking it as such could go in either walk (from the function naming) but
// it happens in the ancestor chain check and skipped here. This DCHECK
// verifies this.
DCHECK(!element->GetDisplayLockContext() ||
element->GetDisplayLockContext()
->IsInViewTransitionElementAncestorChain());

// we keep it locked. So start with the parent.
while ((ancestor = FlatTreeTraversal::ParentElement(*ancestor))) {
if (auto* context = ancestor->GetDisplayLockContext())
context->SetInViewTransitionElementChain();
context->SetDescendantIsViewTransitionElement();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@
border: 1px solid black;
box-sizing: border-box;
}
.hidden {
.vis-hidden {
visibility: hidden;
}
.cv-hidden {
content-visibility: hidden;
}
</style>

<div class=flex>
<div class="box hidden">ancestor c-v</div>
<div class=box>self c-v</div>
<div class=box>descendant c-v</div>
<div class="box vis-hidden">ancestor c-v</div>
<div class="box cv-hidden">self c-v</div>
<div class="box cv-hidden">descendant c-v</div>
</div>

0 comments on commit a7b8baf

Please sign in to comment.