Skip to content

GitLab

T
tangled
Commit 2b583ddb authored by Rakina Zata Amni's avatar Rakina Zata Amni Committed by Commit Bot
Browse files
Options

[Display Locking]: Make activatable-locked nodes count in total find-in-page matches 

Previously, locked subtrees are skipped when doing find-in-page.
With this change, we force updates to activatable locked subtrees
that affect the block we're currently doing find-in-page in so that
we can do find-in-page on activatable-locked nodes with guaranteed
updated style & layout values.

This change makes the total match count include matches that are
in activatable-locked subtrees, but this change doesn't make
the locked matches navigable (e.g. highlighted and can be scrolled
to with find next/prev arrow buttons). That functionality will be
added in a different CL.


Bug: 882663
Change-Id: Icf7cec63959f7b429bbdbcaa671bf9ef5caa661e
Reviewed-on: https://chromium-review.googlesource.com/c/1477466
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: default avatarvmpstr <vmpstr@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635881}
parent adebe396
Show whitespace changes
Inline Side-by-side
Showing with 208 additions and 2 deletions
third_party/blink/renderer/core/display_lock/display_lock_context.h
View file @ 2b583ddb
...@@ -67,7 +67,7 @@ class CORE_EXPORT DisplayLockContext final ...@@ -67,7 +67,7 @@ class CORE_EXPORT DisplayLockContext final
}; };
// See GetScopedForcedUpdate() for description. // See GetScopedForcedUpdate() for description.
class ScopedForcedUpdate { class CORE_EXPORT ScopedForcedUpdate {
DISALLOW_NEW(); DISALLOW_NEW();
public: public:
......
third_party/blink/renderer/core/display_lock/display_lock_context_test.cc
View file @ 2b583ddb
...@@ -279,6 +279,151 @@ TEST_F(DisplayLockContextTest, LockedElementIsNotSearchableViaFindInPage) { ...@@ -279,6 +279,151 @@ TEST_F(DisplayLockContextTest, LockedElementIsNotSearchableViaFindInPage) {
client.Reset(); client.Reset();
} }
TEST_F(DisplayLockContextTest,
ActivatableLockedElementIsSearchableViaFindInPage) {
ResizeAndFocus();
SetHtmlInnerHTML(R"HTML(
<style>
#container {
width: 100px;
height: 100px;
contain: content;
}
</style>
<body><div id="container">testing</div></body>
)HTML");
WebString search_text(String("testing"));
auto* find_in_page = GetFindInPage();
ASSERT_TRUE(find_in_page);
DisplayLockTestFindInPageClient client;
client.SetFrame(LocalMainFrame());
auto find_options = mojom::blink::FindOptions::New();
find_options->run_synchronously_for_testing = true;
find_options->find_next = false;
find_options->forward = true;
int current_id = 123;
find_in_page->Find(current_id++, "testing", find_options->Clone());
EXPECT_FALSE(client.FindResultsAreReady());
test::RunPendingTasks();
EXPECT_TRUE(client.FindResultsAreReady());
EXPECT_EQ(1, client.Count());
client.Reset();
EXPECT_EQ(GetDocument().LockedDisplayLockCount(), 0);
EXPECT_EQ(GetDocument().ActivationBlockingDisplayLockCount(), 0);
DisplayLockOptions options;
options.setActivatable(true);
auto* element = GetDocument().getElementById("container");
auto* script_state = ToScriptStateForMainWorld(GetDocument().GetFrame());
{
ScriptState::Scope scope(script_state);
element->getDisplayLockForBindings()->acquire(script_state, &options);
}
UpdateAllLifecyclePhasesForTest();
// Sanity checks to ensure the element is locked.
EXPECT_FALSE(element->GetDisplayLockContext()->ShouldStyle());
EXPECT_FALSE(element->GetDisplayLockContext()->ShouldLayout());
EXPECT_FALSE(element->GetDisplayLockContext()->ShouldPaint());
EXPECT_EQ(GetDocument().LockedDisplayLockCount(), 1);
EXPECT_EQ(GetDocument().ActivationBlockingDisplayLockCount(), 0);
EXPECT_TRUE(element->GetDisplayLockContext()->IsActivatable());
// Check if we can still get the same result with the same query.
find_in_page->Find(current_id++, "testing", find_options->Clone());
EXPECT_FALSE(client.FindResultsAreReady());
test::RunPendingTasks();
EXPECT_TRUE(client.FindResultsAreReady());
EXPECT_EQ(1, client.Count());
client.Reset();
// Check if the result is correct if we update the contents.
element->SetInnerHTMLFromString(
"<div>tes</div>ting"
"<div style='display:none;'>testing</div>");
find_in_page->Find(current_id++, "testing", find_options->Clone());
EXPECT_FALSE(client.FindResultsAreReady());
test::RunPendingTasks();
EXPECT_TRUE(client.FindResultsAreReady());
EXPECT_EQ(0, client.Count());
client.Reset();
// Check if the result is correct if we have non-activatable lock.
element->SetInnerHTMLFromString(
"<div>testing1</div>"
"<div id='activatable' style='contain: style layout;'>"
" testing2"
" <div id='nestedNonActivatable' style='contain: style layout;'>"
" testing3"
" </div>"
"</div>"
"<div id='nonActivatable' style='contain: style layout;'>testing4</div>");
auto* activatable = GetDocument().getElementById("activatable");
auto* non_activatable = GetDocument().getElementById("nonActivatable");
auto* nested_non_activatable =
GetDocument().getElementById("nestedNonActivatable");
{
ScriptState::Scope scope(script_state);
activatable->getDisplayLockForBindings()->acquire(script_state, &options);
nested_non_activatable->getDisplayLockForBindings()->acquire(script_state,
nullptr);
non_activatable->getDisplayLockForBindings()->acquire(script_state,
nullptr);
}
UpdateAllLifecyclePhasesForTest();
EXPECT_EQ(GetDocument().LockedDisplayLockCount(), 4);
EXPECT_EQ(GetDocument().ActivationBlockingDisplayLockCount(), 2);
find_in_page->Find(current_id++, "testing", find_options->Clone());
EXPECT_FALSE(client.FindResultsAreReady());
test::RunPendingTasks();
EXPECT_TRUE(client.FindResultsAreReady());
EXPECT_EQ(2, client.Count());
client.Reset();
// Check if the result is correct if we update style.
activatable->setAttribute("style", "contain: style layout; display: none;");
EXPECT_EQ(GetDocument().LockedDisplayLockCount(), 4);
EXPECT_EQ(GetDocument().ActivationBlockingDisplayLockCount(), 2);
find_in_page->Find(current_id++, "testing", find_options->Clone());
EXPECT_FALSE(client.FindResultsAreReady());
test::RunPendingTasks();
EXPECT_TRUE(client.FindResultsAreReady());
EXPECT_EQ(1, client.Count());
client.Reset();
// Now commit all the locks and ensure we can find.
{
ScriptState::Scope scope(script_state);
element->getDisplayLockForBindings()->commit(script_state);
activatable->getDisplayLockForBindings()->commit(script_state);
nested_non_activatable->getDisplayLockForBindings()->commit(script_state);
non_activatable->getDisplayLockForBindings()->commit(script_state);
}
UpdateAllLifecyclePhasesForTest();
EXPECT_EQ(GetDocument().LockedDisplayLockCount(), 0);
EXPECT_EQ(GetDocument().ActivationBlockingDisplayLockCount(), 0);
find_in_page->Find(current_id++, "testing", find_options->Clone());
EXPECT_FALSE(client.FindResultsAreReady());
test::RunPendingTasks();
EXPECT_TRUE(client.FindResultsAreReady());
EXPECT_EQ(2, client.Count());
client.Reset();
}
TEST_F(DisplayLockContextTest, CallUpdateStyleAndLayoutAfterChange) { TEST_F(DisplayLockContextTest, CallUpdateStyleAndLayoutAfterChange) {
ResizeAndFocus(); ResizeAndFocus();
SetHtmlInnerHTML(R"HTML( SetHtmlInnerHTML(R"HTML(
......
third_party/blink/renderer/core/editing/finder/find_buffer.cc
View file @ 2b583ddb
...@@ -145,7 +145,7 @@ Node* GetNonSearchableAncestor(const Node& node) { ...@@ -145,7 +145,7 @@ Node* GetNonSearchableAncestor(const Node& node) {
for (Node& ancestor : FlatTreeTraversal::InclusiveAncestorsOf(node)) { for (Node& ancestor : FlatTreeTraversal::InclusiveAncestorsOf(node)) {
const ComputedStyle* style = ancestor.EnsureComputedStyle(); const ComputedStyle* style = ancestor.EnsureComputedStyle();
if ((style && style->Display() == EDisplay::kNone) || if ((style && style->Display() == EDisplay::kNone) ||
ShouldIgnoreContents(node)) ShouldIgnoreContents(ancestor))
return &ancestor; return &ancestor;
if (ancestor.IsDocumentNode()) if (ancestor.IsDocumentNode())
return nullptr; return nullptr;
...@@ -260,6 +260,45 @@ std::unique_ptr<FindBuffer::Results> FindBuffer::FindMatches( ...@@ -260,6 +260,45 @@ std::unique_ptr<FindBuffer::Results> FindBuffer::FindMatches(
return std::make_unique<Results>(buffer_, search_text_16_bit, options); return std::make_unique<Results>(buffer_, search_text_16_bit, options);
} }
bool FindBuffer::PushScopedForcedUpdateIfNeeded(const Element& element) {
if (auto* context = element.GetDisplayLockContext()) {
DCHECK(context->IsActivatable());
scoped_forced_update_list_.push_back(context->GetScopedForcedUpdate());
return true;
}
return false;
}
void FindBuffer::CollectScopedForcedUpdates(Node& start_node,
const Node* search_range_end_node,
const Node* node_after_block) {
if (!RuntimeEnabledFeatures::DisplayLockingEnabled())
return;
if (start_node.GetDocument().LockedDisplayLockCount() ==
start_node.GetDocument().ActivationBlockingDisplayLockCount())
return;
Node* node = &start_node;
// We assume |start_node| is always visible/activatable if locked, so we don't
// need to check activatability of ancestors here.
for (Node& ancestor : FlatTreeTraversal::InclusiveAncestorsOf(*node)) {
if (!ancestor.IsElementNode())
continue;
PushScopedForcedUpdateIfNeeded(ToElement(ancestor));
}
while (node && node != node_after_block && node != search_range_end_node) {
if (ShouldIgnoreContents(*node)) {
// Will skip display:none/non-activatable locked subtrees/etc.
node = FlatTreeTraversal::NextSkippingChildren(*node);
continue;
}
if (node->IsElementNode())
PushScopedForcedUpdateIfNeeded(ToElement(*node));
node = FlatTreeTraversal::Next(*node);
}
}
// Collects text until block boundary located at or after |start_node| // Collects text until block boundary located at or after |start_node|
// to |buffer_|. Saves the next starting node after the block to // to |buffer_|. Saves the next starting node after the block to
// |node_after_block_|. // |node_after_block_|.
...@@ -291,6 +330,13 @@ void FindBuffer::CollectTextUntilBlockBoundary( ...@@ -291,6 +330,13 @@ void FindBuffer::CollectTextUntilBlockBoundary(
Node* const first_traversed_node = node; Node* const first_traversed_node = node;
// We will also stop if we encountered/passed |end_node|. // We will also stop if we encountered/passed |end_node|.
Node* end_node = range.EndPosition().NodeAsRangeLastNode(); Node* end_node = range.EndPosition().NodeAsRangeLastNode();
if (node) {
CollectScopedForcedUpdates(*node, end_node, just_after_block);
if (!scoped_forced_update_list_.IsEmpty())
node->GetDocument().UpdateStyleAndLayoutIgnorePendingStylesheets();
}
while (node && node != just_after_block) { while (node && node != just_after_block) {
if (ShouldIgnoreContents(*node)) { if (ShouldIgnoreContents(*node)) {
if (end_node && (end_node == node || if (end_node && (end_node == node ||
......
third_party/blink/renderer/core/editing/finder/find_buffer.h
View file @ 2b583ddb
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_EDITING_FINDER_FIND_BUFFER_H_ #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_EDITING_FINDER_FIND_BUFFER_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_EDITING_FINDER_FIND_BUFFER_H_ #define THIRD_PARTY_BLINK_RENDERER_CORE_EDITING_FINDER_FIND_BUFFER_H_
#include "third_party/blink/renderer/core/display_lock/display_lock_context.h"
#include "third_party/blink/renderer/core/editing/finder/find_options.h" #include "third_party/blink/renderer/core/editing/finder/find_options.h"
#include "third_party/blink/renderer/core/editing/iterators/text_searcher_icu.h" #include "third_party/blink/renderer/core/editing/iterators/text_searcher_icu.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.h" #include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.h"
...@@ -119,6 +120,19 @@ class CORE_EXPORT FindBuffer { ...@@ -119,6 +120,19 @@ class CORE_EXPORT FindBuffer {
// another LayoutBlockFlow or after |end_position|) to |node_after_block_|. // another LayoutBlockFlow or after |end_position|) to |node_after_block_|.
void CollectTextUntilBlockBoundary(const EphemeralRangeInFlatTree& range); void CollectTextUntilBlockBoundary(const EphemeralRangeInFlatTree& range);
// Adds the ScopedForcedUpdate of |element|'s DisplayLockContext (if it's
// there) to |scoped_forced_update_list_|. Returns true if we added a
// ScopedForceUpdate.
bool PushScopedForcedUpdateIfNeeded(const Element& element);
// Collects all ScopedForceUpdates of any activatable-locked element
// within the range of [start_node, search_range_end_node] or
// [start_node, node_after_block) whichever is smaller, to
// |scoped_forced_update_list_|.
void CollectScopedForcedUpdates(Node& start_node,
const Node* search_range_end_node,
const Node* node_after_block);
class CORE_EXPORT InvisibleLayoutScope { class CORE_EXPORT InvisibleLayoutScope {
STACK_ALLOCATED(); STACK_ALLOCATED();
...@@ -179,6 +193,7 @@ class CORE_EXPORT FindBuffer { ...@@ -179,6 +193,7 @@ class CORE_EXPORT FindBuffer {
Member<Node> node_after_block_; Member<Node> node_after_block_;
Vector<UChar> buffer_; Vector<UChar> buffer_;
Vector<BufferNodeMapping> buffer_node_mappings_; Vector<BufferNodeMapping> buffer_node_mappings_;
Vector<DisplayLockContext::ScopedForcedUpdate> scoped_forced_update_list_;
// For legacy layout, we need to save a unique_ptr of the NGOffsetMapping // For legacy layout, we need to save a unique_ptr of the NGOffsetMapping
// because nobody owns it. In LayoutNG, the NGOffsetMapping is owned by // because nobody owns it. In LayoutNG, the NGOffsetMapping is owned by
......
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