Skip to content

Commit

Permalink
Merge "DeferredShaping: Do not auto-lock by selection, focus, etc." t…
Browse files Browse the repository at this point in the history
…o M103 branch

This is a reland of commit ae4e1fc

Changes from the original CL:
 * Add null element_ check to IsShapingDeferred()
 * Add a test for the above.

Original change's description:
> DeferredShaping: Do not auto-lock by selection, focus, etc.
>
> After shapign-deferred boxes are unlocked automitically by selection,
> focus, etc., they should not be locked automatically again in order to
> avoid inconsistent locking states.
>
> Bug: 1328053
> Change-Id: I7ce01361be0c88d374f80082a2ce37a1817a768c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3666576
> Auto-Submit: Kent Tamura <tkent@chromium.org>
> Reviewed-by: Koji Ishii <kojii@chromium.org>
> Commit-Queue: Koji Ishii <kojii@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1007770}

(cherry picked from commit b815a2d)

Bug: 1328053, 1330060
Change-Id: Id19254e2e2dcc23531757e99d9022242d9bc963c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3675737
Auto-Submit: Kent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1008725}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3680238
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#412}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
tkent-google authored and Chromium LUCI CQ committed May 31, 2022
1 parent 3843729 commit 806da0c
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,8 @@ void DisplayLockContext::ScheduleTopLayerCheck() {
}

bool DisplayLockContext::IsShapingDeferred() const {
if (!element_)
return false;
if (const auto* layout_object = element_->GetLayoutObject())
return layout_object->IsShapingDeferred();
return false;
Expand Down Expand Up @@ -1218,6 +1220,17 @@ void DisplayLockContext::NotifyRenderAffectingStateChanged() {
!state(RenderAffectingState::kAutoUnlockedForPrint) &&
!state(RenderAffectingState::kSubtreeHasTopLayerElement)));

// For shaping-deferred boxes, we'd like to unlock permanently.
if (IsShapingDeferred() && state_ != EContentVisibility::kVisible &&
!should_be_locked && IsLocked()) {
SetRequestedState(EContentVisibility::kVisible);
// We need to change the document lifecycle explicitly because Unlock()
// inside the above SetRequestedState() didn't change the lifecycle.
// See CanDirtyStyle() check in Unlock().
document_->Lifecycle().EnsureStateAtMost(DocumentLifecycle::kStyleClean);
return;
}

if (should_be_locked && !IsLocked())
Lock();
else if (!should_be_locked && IsLocked())
Expand Down
41 changes: 41 additions & 0 deletions third_party/blink/renderer/core/layout/deferred_shaping_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "third_party/blink/renderer/core/paint/paint_timing.h"
#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
#include "third_party/blink/renderer/platform/fonts/shaping/shape_result_inline_headers.h"
#include "third_party/blink/renderer/platform/heap/thread_state.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"

namespace blink {
Expand Down Expand Up @@ -158,6 +159,46 @@ TEST_F(DeferredShapingTest, DynamicPropertyChange) {
EXPECT_TRUE(IsLocked("target"));
}

TEST_F(DeferredShapingTest, NoAutoLock) {
SetBodyInnerHTML(R"HTML(
<div style="height:1800px"></div>
<div id="target">IFC</div>
)HTML");
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(IsDefer("target"));
EXPECT_TRUE(IsLocked("target"));

auto* context = GetElementById("target")->GetDisplayLockContext();
context->NotifySubtreeGainedSelection();
EXPECT_FALSE(IsLocked("target"));

context->NotifySubtreeLostSelection();
EXPECT_FALSE(IsLocked("target"));
}

// crbug.com/1330060
TEST_F(DeferredShapingTest, NoAutoLockWithoutElement) {
SetBodyInnerHTML(R"HTML(
<div style="height:1800px"></div>
<div id="target">IFC</div>
)HTML");
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(IsDefer("target"));
EXPECT_TRUE(IsLocked("target"));

Persistent<DisplayLockContext> context =
GetElementById("target")->GetDisplayLockContext();
context->NotifySubtreeGainedSelection();
EXPECT_FALSE(IsLocked("target"));

GetElementById("target")->remove();
UpdateAllLifecyclePhasesForTest();
// Clear a WeakMember of DisplayLockContext.
ThreadState::Current()->CollectAllGarbageForTesting();
context->NotifySubtreeLostSelection();
// Pass if no crashes.
}

TEST_F(DeferredShapingTest, ListMarkerCrash) {
SetBodyInnerHTML(R"HTML(
<div style="height:1800px"></div>
Expand Down

0 comments on commit 806da0c

Please sign in to comment.