Commit c1e7cf12 authored by Vladimir Levin's avatar Vladimir Levin Committed by Commit Bot

SubtreeVisibility: Remove activation ulocking.

This patch removes all unlocking from the activation signal code path.

This is in line with the proposed spec: auto unlocks only on selection,
focus, and visibility. Hidden-matchable issues an event but does not
unlock.

R=chrishtr@chromium.org

Change-Id: Id47e1edcf4a2042e286ce50943f5c5b4a6b5fc4a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134571Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755911}
parent 8cfee54c
...@@ -382,11 +382,6 @@ void DisplayLockContext::CommitForActivationWithSignal( ...@@ -382,11 +382,6 @@ void DisplayLockContext::CommitForActivationWithSignal(
} }
RecordActivationReason(document_, reason); RecordActivationReason(document_, reason);
// TODO(vmpstr): This should be removed after find-in-page doesn't rely on
// this.
if (reason == DisplayLockActivationReason::kFindInPage)
Unlock();
} }
void DisplayLockContext::NotifyIsIntersectingViewport() { void DisplayLockContext::NotifyIsIntersectingViewport() {
......
...@@ -301,13 +301,16 @@ TEST_F(DisplayLockContextTest, ...@@ -301,13 +301,16 @@ TEST_F(DisplayLockContextTest,
ResizeAndFocus(); ResizeAndFocus();
SetHtmlInnerHTML(R"HTML( SetHtmlInnerHTML(R"HTML(
<style> <style>
.spacer {
height: 10000px;
}
#container { #container {
width: 100px; width: 100px;
height: 100px; height: 100px;
contain: style layout paint; contain: style layout paint;
} }
</style> </style>
<body><div id="container">testing</div></body> <body><div class=spacer></div><div id="container">testing</div></body>
)HTML"); )HTML");
const String search_text = "testing"; const String search_text = "testing";
...@@ -328,7 +331,8 @@ TEST_F(DisplayLockContextTest, ...@@ -328,7 +331,8 @@ TEST_F(DisplayLockContextTest,
// Check if we can still get the same result with the same query. // Check if we can still get the same result with the same query.
Find(search_text, client); Find(search_text, client);
EXPECT_EQ(1, client.Count()); EXPECT_EQ(1, client.Count());
EXPECT_FALSE(container->GetDisplayLockContext()->IsLocked()); EXPECT_TRUE(container->GetDisplayLockContext()->IsLocked());
EXPECT_GT(GetDocument().scrollingElement()->scrollTop(), 1000);
} }
TEST_F(DisplayLockContextTest, TEST_F(DisplayLockContextTest,
...@@ -471,7 +475,7 @@ TEST_F(DisplayLockContextTest, FindInPageWithChangedContent) { ...@@ -471,7 +475,7 @@ TEST_F(DisplayLockContextTest, FindInPageWithChangedContent) {
client.SetFrame(LocalMainFrame()); client.SetFrame(LocalMainFrame());
Find("testing", client); Find("testing", client);
EXPECT_EQ(3, client.Count()); EXPECT_EQ(3, client.Count());
EXPECT_FALSE(container->GetDisplayLockContext()->IsLocked()); EXPECT_TRUE(container->GetDisplayLockContext()->IsLocked());
} }
TEST_F(DisplayLockContextTest, FindInPageWithNoMatchesWontUnlock) { TEST_F(DisplayLockContextTest, FindInPageWithNoMatchesWontUnlock) {
...@@ -548,18 +552,14 @@ TEST_F(DisplayLockContextTest, ...@@ -548,18 +552,14 @@ TEST_F(DisplayLockContextTest,
EXPECT_EQ(2, client.Count()); EXPECT_EQ(2, client.Count());
EXPECT_EQ(1, client.ActiveIndex()); EXPECT_EQ(1, client.ActiveIndex());
// #container should be unlocked, since the match is inside that EXPECT_TRUE(container->GetDisplayLockContext()->IsLocked());
// element ("testing1" inside the div).
EXPECT_FALSE(container->GetDisplayLockContext()->IsLocked());
// Since the active match isn't located within other locked elements
// they need to stay locked.
EXPECT_TRUE(activatable->GetDisplayLockContext()->IsLocked()); EXPECT_TRUE(activatable->GetDisplayLockContext()->IsLocked());
EXPECT_TRUE(non_activatable->GetDisplayLockContext()->IsLocked()); EXPECT_TRUE(non_activatable->GetDisplayLockContext()->IsLocked());
EXPECT_TRUE(nested_non_activatable->GetDisplayLockContext()->IsLocked()); EXPECT_TRUE(nested_non_activatable->GetDisplayLockContext()->IsLocked());
} }
TEST_F(DisplayLockContextTest, TEST_F(DisplayLockContextTest,
NestedActivatableLockedElementIsUnlockedByFindInPage) { NestedActivatableLockedElementIsNotUnlockedByFindInPage) {
ResizeAndFocus(); ResizeAndFocus();
SetHtmlInnerHTML(R"HTML( SetHtmlInnerHTML(R"HTML(
<body> <body>
...@@ -588,8 +588,8 @@ TEST_F(DisplayLockContextTest, ...@@ -588,8 +588,8 @@ TEST_F(DisplayLockContextTest,
EXPECT_EQ(1, client.Count()); EXPECT_EQ(1, client.Count());
EXPECT_EQ(1, client.ActiveIndex()); EXPECT_EQ(1, client.ActiveIndex());
EXPECT_FALSE(container->GetDisplayLockContext()->IsLocked()); EXPECT_TRUE(container->GetDisplayLockContext()->IsLocked());
EXPECT_FALSE(child->GetDisplayLockContext()->IsLocked()); EXPECT_TRUE(child->GetDisplayLockContext()->IsLocked());
} }
TEST_F(DisplayLockContextTest, TEST_F(DisplayLockContextTest,
...@@ -633,12 +633,16 @@ TEST_F(DisplayLockContextTest, ...@@ -633,12 +633,16 @@ TEST_F(DisplayLockContextTest,
EXPECT_EQ(2, client.Count()); EXPECT_EQ(2, client.Count());
EXPECT_EQ(1, client.ActiveIndex()); EXPECT_EQ(1, client.ActiveIndex());
EXPECT_EQ(text_rect(div_one), client.ActiveMatchRect()); EXPECT_EQ(text_rect(div_one), client.ActiveMatchRect());
// Pretend that the event unlocked the element.
CommitElement(*div_one);
// Going forward from #one would go to #three. // Going forward from #one would go to #three.
Find(search_text, client, true /* find_next */); Find(search_text, client, true /* find_next */);
EXPECT_EQ(2, client.Count()); EXPECT_EQ(2, client.Count());
EXPECT_EQ(2, client.ActiveIndex()); EXPECT_EQ(2, client.ActiveIndex());
EXPECT_EQ(text_rect(div_three), client.ActiveMatchRect()); EXPECT_EQ(text_rect(div_three), client.ActiveMatchRect());
// Pretend that the event unlocked the element.
CommitElement(*div_three);
// Going backwards from #three would go to #one. // Going backwards from #three would go to #one.
client.Reset(); client.Reset();
......
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