Commit d542339d authored by Gayane Petrosyan's avatar Gayane Petrosyan Committed by Commit Bot

Reland "[SH-Blink] Create TextFragmentSelectorGenerator with page."

This is a reland of d43884e1

Fix memory leak issue by cleaning selection_range member in
TextFragmentSelectorGenerator on DocumentDetached.
Reverted version is in patchset 1.

Original change's description:
> [SH-Blink] Create TextFragmentSelectorGenerator with page.
>
> Create TextFragmentSelectorGenerator with page and set selection
> information when context menu is shown.
> This is done so that Java code can just call the GenerateSelector
> function without any arguments and generation can happen for the latest
> selected text.
>
> Follow-up CL
> https://chromium-review.googlesource.com/c/chromium/src/+/2341662
> will connect to browser-side component.
>
> Bug: 1102382
> Change-Id: I5574f5b40e5a74bef56698e2de5a9b1fea17e0d0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2340734
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Reviewed-by: David Bokan <bokan@chromium.org>
> Commit-Queue: Gayane Petrosyan <gayane@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#797256}

Bug: 1102382
Change-Id: If05b8e643627f4851dced45ce02cd78b7fa87fde
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354304Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Commit-Queue: Gayane Petrosyan <gayane@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798643}
parent 59919f45
...@@ -41,6 +41,7 @@ ...@@ -41,6 +41,7 @@
#include "third_party/blink/renderer/core/dom/node.h" #include "third_party/blink/renderer/core/dom/node.h"
#include "third_party/blink/renderer/core/editing/editing_tri_state.h" #include "third_party/blink/renderer/core/editing/editing_tri_state.h"
#include "third_party/blink/renderer/core/editing/editor.h" #include "third_party/blink/renderer/core/editing/editor.h"
#include "third_party/blink/renderer/core/editing/ephemeral_range.h"
#include "third_party/blink/renderer/core/editing/frame_selection.h" #include "third_party/blink/renderer/core/editing/frame_selection.h"
#include "third_party/blink/renderer/core/editing/ime/input_method_controller.h" #include "third_party/blink/renderer/core/editing/ime/input_method_controller.h"
#include "third_party/blink/renderer/core/editing/selection_controller.h" #include "third_party/blink/renderer/core/editing/selection_controller.h"
...@@ -69,6 +70,7 @@ ...@@ -69,6 +70,7 @@
#include "third_party/blink/renderer/core/page/context_menu_provider.h" #include "third_party/blink/renderer/core/page/context_menu_provider.h"
#include "third_party/blink/renderer/core/page/focus_controller.h" #include "third_party/blink/renderer/core/page/focus_controller.h"
#include "third_party/blink/renderer/core/page/page.h" #include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h"
#include "third_party/blink/renderer/platform/exported/wrapped_resource_response.h" #include "third_party/blink/renderer/platform/exported/wrapped_resource_response.h"
namespace blink { namespace blink {
...@@ -246,6 +248,17 @@ bool ContextMenuController::ShowContextMenu(LocalFrame* frame, ...@@ -246,6 +248,17 @@ bool ContextMenuController::ShowContextMenu(LocalFrame* frame,
PhysicalOffset(FlooredIntPoint(point))); PhysicalOffset(FlooredIntPoint(point)));
} }
// Store text selection when it happens as it might be cleared when the
// browser will request |TextFragmentSelectorGenerator| to generator selector.
if (!selected_frame->Selection().SelectedText().IsEmpty()) {
VisibleSelectionInFlatTree selection =
selected_frame->Selection().ComputeVisibleSelectionInFlatTree();
EphemeralRangeInFlatTree selection_range(selection.Start(),
selection.End());
page_->GetTextFragmentSelectorGenerator().UpdateSelection(selected_frame,
selection_range);
}
WebContextMenuData data; WebContextMenuData data;
data.mouse_position = selected_frame->View()->FrameToViewport( data.mouse_position = selected_frame->View()->FrameToViewport(
result.RoundedPointInInnerNodeFrame()); result.RoundedPointInInnerNodeFrame());
......
...@@ -75,6 +75,7 @@ ...@@ -75,6 +75,7 @@
#include "third_party/blink/renderer/core/page/scoped_page_pauser.h" #include "third_party/blink/renderer/core/page/scoped_page_pauser.h"
#include "third_party/blink/renderer/core/page/scrolling/overscroll_controller.h" #include "third_party/blink/renderer/core/page/scrolling/overscroll_controller.h"
#include "third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h" #include "third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h"
#include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h"
#include "third_party/blink/renderer/core/page/scrolling/top_document_root_scroller_controller.h" #include "third_party/blink/renderer/core/page/scrolling/top_document_root_scroller_controller.h"
#include "third_party/blink/renderer/core/page/spatial_navigation_controller.h" #include "third_party/blink/renderer/core/page/spatial_navigation_controller.h"
#include "third_party/blink/renderer/core/page/validation_message_client_impl.h" #include "third_party/blink/renderer/core/page/validation_message_client_impl.h"
...@@ -225,7 +226,9 @@ Page::Page(PageClients& page_clients) ...@@ -225,7 +226,9 @@ Page::Page(PageClients& page_clients)
next_related_page_(this), next_related_page_(this),
prev_related_page_(this), prev_related_page_(this),
autoplay_flags_(0), autoplay_flags_(0),
web_text_autosizer_page_info_({0, 0, 1.f}) { web_text_autosizer_page_info_({0, 0, 1.f}),
text_fragment_selector_generator_(
MakeGarbageCollected<TextFragmentSelectorGenerator>()) {
DCHECK(!AllPages().Contains(this)); DCHECK(!AllPages().Contains(this));
AllPages().insert(this); AllPages().insert(this);
...@@ -340,6 +343,7 @@ LocalFrame* Page::DeprecatedLocalMainFrame() const { ...@@ -340,6 +343,7 @@ LocalFrame* Page::DeprecatedLocalMainFrame() const {
void Page::DocumentDetached(Document* document) { void Page::DocumentDetached(Document* document) {
pointer_lock_controller_->DocumentDetached(document); pointer_lock_controller_->DocumentDetached(document);
text_fragment_selector_generator_->DocumentDetached(document);
context_menu_controller_->DocumentDetached(document); context_menu_controller_->DocumentDetached(document);
if (validation_message_client_) if (validation_message_client_)
validation_message_client_->DocumentDetached(*document); validation_message_client_->DocumentDetached(*document);
...@@ -914,6 +918,7 @@ void Page::Trace(Visitor* visitor) const { ...@@ -914,6 +918,7 @@ void Page::Trace(Visitor* visitor) const {
visitor->Trace(plugins_changed_observers_); visitor->Trace(plugins_changed_observers_);
visitor->Trace(next_related_page_); visitor->Trace(next_related_page_);
visitor->Trace(prev_related_page_); visitor->Trace(prev_related_page_);
visitor->Trace(text_fragment_selector_generator_);
Supplementable<Page>::Trace(visitor); Supplementable<Page>::Trace(visitor);
} }
......
...@@ -75,6 +75,7 @@ class PageScaleConstraintsSet; ...@@ -75,6 +75,7 @@ class PageScaleConstraintsSet;
class PluginData; class PluginData;
class PluginsChangedObserver; class PluginsChangedObserver;
class PointerLockController; class PointerLockController;
class TextFragmentSelectorGenerator;
class ScopedPagePauser; class ScopedPagePauser;
class ScrollingCoordinator; class ScrollingCoordinator;
class ScrollbarTheme; class ScrollbarTheme;
...@@ -356,6 +357,10 @@ class CORE_EXPORT Page final : public GarbageCollected<Page>, ...@@ -356,6 +357,10 @@ class CORE_EXPORT Page final : public GarbageCollected<Page>,
static void PrepareForLeakDetection(); static void PrepareForLeakDetection();
TextFragmentSelectorGenerator& GetTextFragmentSelectorGenerator() const {
return *text_fragment_selector_generator_;
}
private: private:
friend class ScopedPagePauser; friend class ScopedPagePauser;
...@@ -477,6 +482,8 @@ class CORE_EXPORT Page final : public GarbageCollected<Page>, ...@@ -477,6 +482,8 @@ class CORE_EXPORT Page final : public GarbageCollected<Page>,
WebScopedVirtualTimePauser history_navigation_virtual_time_pauser_; WebScopedVirtualTimePauser history_navigation_virtual_time_pauser_;
const Member<TextFragmentSelectorGenerator> text_fragment_selector_generator_;
DISALLOW_COPY_AND_ASSIGN(Page); DISALLOW_COPY_AND_ASSIGN(Page);
}; };
......
...@@ -15,27 +15,31 @@ namespace blink { ...@@ -15,27 +15,31 @@ namespace blink {
constexpr int kExactTextMaxChars = 300; constexpr int kExactTextMaxChars = 300;
constexpr int kNoContextMinChars = 20; constexpr int kNoContextMinChars = 20;
TextFragmentSelectorGenerator::TextFragmentSelectorGenerator( void TextFragmentSelectorGenerator::UpdateSelection(
LocalFrame* frame, LocalFrame* selection_frame,
base::OnceCallback<void(const TextFragmentSelector&)> callback)
: frame_(frame), callback_(std::move(callback)) {}
// TextFragmentSelectorGenerator is responsible for generating text fragment
// selectors that have a unique match.
void TextFragmentSelectorGenerator::GenerateSelector(
const EphemeralRangeInFlatTree& selection_range) { const EphemeralRangeInFlatTree& selection_range) {
selection_frame_ = selection_frame;
selection_range_ = MakeGarbageCollected<Range>(
selection_range.GetDocument(),
ToPositionInDOMTree(selection_range.StartPosition()),
ToPositionInDOMTree(selection_range.EndPosition()));
}
void TextFragmentSelectorGenerator::GenerateSelector() {
EphemeralRangeInFlatTree ephemeral_range(selection_range_);
const TextFragmentSelector kInvalidSelector( const TextFragmentSelector kInvalidSelector(
TextFragmentSelector::SelectorType::kInvalid); TextFragmentSelector::SelectorType::kInvalid);
Node& start_first_block_ancestor = Node& start_first_block_ancestor =
FindBuffer::GetFirstBlockLevelAncestorInclusive( FindBuffer::GetFirstBlockLevelAncestorInclusive(
*selection_range.StartPosition().AnchorNode()); *ephemeral_range.StartPosition().AnchorNode());
Node& end_first_block_ancestor = Node& end_first_block_ancestor =
FindBuffer::GetFirstBlockLevelAncestorInclusive( FindBuffer::GetFirstBlockLevelAncestorInclusive(
*selection_range.EndPosition().AnchorNode()); *ephemeral_range.EndPosition().AnchorNode());
if (!start_first_block_ancestor.isSameNode(&end_first_block_ancestor)) if (!start_first_block_ancestor.isSameNode(&end_first_block_ancestor))
std::move(callback_).Run(kInvalidSelector); NotifySelectorReady(kInvalidSelector);
// TODO(gayane): If same node, need to check if start and end are interrupted // TODO(gayane): If same node, need to check if start and end are interrupted
// by a block. Example: <div>start of the selection <div> sub block </div>end // by a block. Example: <div>start of the selection <div> sub block </div>end
...@@ -43,16 +47,16 @@ void TextFragmentSelectorGenerator::GenerateSelector( ...@@ -43,16 +47,16 @@ void TextFragmentSelectorGenerator::GenerateSelector(
// TODO(gayane): Move selection start and end to contain full words. // TODO(gayane): Move selection start and end to contain full words.
String selected_text = PlainText(selection_range); String selected_text = PlainText(ephemeral_range);
if (selected_text.length() < kNoContextMinChars || if (selected_text.length() < kNoContextMinChars ||
selected_text.length() > kExactTextMaxChars) selected_text.length() > kExactTextMaxChars)
std::move(callback_).Run(kInvalidSelector); NotifySelectorReady(kInvalidSelector);
selector_ = std::make_unique<TextFragmentSelector>( selector_ = std::make_unique<TextFragmentSelector>(
TextFragmentSelector::SelectorType::kExact, selected_text, "", "", ""); TextFragmentSelector::SelectorType::kExact, selected_text, "", "", "");
TextFragmentFinder finder(*this, *selector_); TextFragmentFinder finder(*this, *selector_);
finder.FindMatch(*frame_->GetDocument()); finder.FindMatch(*selection_frame_->GetDocument());
} }
void TextFragmentSelectorGenerator::DidFindMatch( void TextFragmentSelectorGenerator::DidFindMatch(
...@@ -60,16 +64,36 @@ void TextFragmentSelectorGenerator::DidFindMatch( ...@@ -60,16 +64,36 @@ void TextFragmentSelectorGenerator::DidFindMatch(
const TextFragmentAnchorMetrics::Match match_metrics, const TextFragmentAnchorMetrics::Match match_metrics,
bool is_unique) { bool is_unique) {
if (is_unique) { if (is_unique) {
std::move(callback_).Run(*selector_); NotifySelectorReady(*selector_);
} else { } else {
// TODO(gayane): Should add more range and/or context. // TODO(gayane): Should add more range and/or context.
std::move(callback_).Run( NotifySelectorReady(
TextFragmentSelector(TextFragmentSelector::SelectorType::kInvalid)); TextFragmentSelector(TextFragmentSelector::SelectorType::kInvalid));
} }
} }
void TextFragmentSelectorGenerator::SetCallbackForTesting(
base::OnceCallback<void(const TextFragmentSelector&)> callback) {
callback_for_tests_ = std::move(callback);
}
void TextFragmentSelectorGenerator::NotifySelectorReady(
const TextFragmentSelector& selector) {
if (!callback_for_tests_.is_null())
std::move(callback_for_tests_).Run(selector);
}
void TextFragmentSelectorGenerator::DocumentDetached(Document* document) {
if (selection_range_ && selection_range_->OwnerDocument() == *document) {
selection_range_->Dispose();
selection_range_ = nullptr;
selection_frame_ = nullptr;
}
}
void TextFragmentSelectorGenerator::Trace(Visitor* visitor) const { void TextFragmentSelectorGenerator::Trace(Visitor* visitor) const {
visitor->Trace(frame_); visitor->Trace(selection_frame_);
visitor->Trace(selection_range_);
} }
} // namespace blink } // namespace blink
...@@ -13,28 +13,48 @@ namespace blink { ...@@ -13,28 +13,48 @@ namespace blink {
class LocalFrame; class LocalFrame;
// TextFragmentSelectorGenerator is responsible for generating text fragment
// selectors for the user selected text according to spec in
// https://github.com/WICG/scroll-to-text-fragment#proposed-solution.
// Generated selectors would be later used to highlight the same
// text if successfully parsed by |TextFragmentAnchor |. Generation will be
// triggered when users request "link to text" for the selected text.
class CORE_EXPORT TextFragmentSelectorGenerator final class CORE_EXPORT TextFragmentSelectorGenerator final
: public GarbageCollected<TextFragmentSelectorGenerator>, : public GarbageCollected<TextFragmentSelectorGenerator>,
public TextFragmentFinder::Client { public TextFragmentFinder::Client {
public: public:
TextFragmentSelectorGenerator( explicit TextFragmentSelectorGenerator() = default;
LocalFrame* frame,
base::OnceCallback<void(const TextFragmentSelector&)> callback); // Sets the frame and range of the current selection.
void UpdateSelection(LocalFrame* selection_frame,
const EphemeralRangeInFlatTree& selection_range);
void GenerateSelector(const EphemeralRangeInFlatTree& selection_range); // Generates selector for current selection.
void GenerateSelector();
// TextFragmentFinder::Client interface // TextFragmentFinder::Client interface
void DidFindMatch(const EphemeralRangeInFlatTree& match, void DidFindMatch(const EphemeralRangeInFlatTree& match,
const TextFragmentAnchorMetrics::Match match_metrics, const TextFragmentAnchorMetrics::Match match_metrics,
bool is_unique) override; bool is_unique) override;
// Sets the callback used for notifying test results of |GenerateSelector|.
void SetCallbackForTesting(
base::OnceCallback<void(const TextFragmentSelector&)> callback);
// Notifies the results of |GenerateSelector|.
void NotifySelectorReady(const TextFragmentSelector& selector);
void DocumentDetached(Document* document);
void Trace(Visitor*) const; void Trace(Visitor*) const;
private: private:
Member<LocalFrame> frame_; Member<LocalFrame> selection_frame_;
base::OnceCallback<void(const TextFragmentSelector&)> callback_; Member<Range> selection_range_;
std::unique_ptr<TextFragmentSelector> selector_; std::unique_ptr<TextFragmentSelector> selector_;
base::OnceCallback<void(const TextFragmentSelector&)> callback_for_tests_;
DISALLOW_COPY_AND_ASSIGN(TextFragmentSelectorGenerator); DISALLOW_COPY_AND_ASSIGN(TextFragmentSelectorGenerator);
}; };
......
...@@ -41,10 +41,13 @@ TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector) { ...@@ -41,10 +41,13 @@ TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector) {
callback_called = true; callback_called = true;
}); });
TextFragmentSelectorGenerator generator(GetDocument().GetFrame(), TextFragmentSelectorGenerator generator;
std::move(callback)); generator.UpdateSelection(
generator.GenerateSelector( GetDocument().GetFrame(),
ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end))); ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end)));
generator.SetCallbackForTesting(std::move(callback));
generator.GenerateSelector();
EXPECT_TRUE(callback_called); EXPECT_TRUE(callback_called);
} }
...@@ -69,10 +72,13 @@ TEST_F(TextFragmentSelectorGeneratorTest, ExactTextWithNestedTextNodes) { ...@@ -69,10 +72,13 @@ TEST_F(TextFragmentSelectorGeneratorTest, ExactTextWithNestedTextNodes) {
callback_called = true; callback_called = true;
}); });
TextFragmentSelectorGenerator generator(GetDocument().GetFrame(), TextFragmentSelectorGenerator generator;
std::move(callback)); generator.UpdateSelection(
generator.GenerateSelector( GetDocument().GetFrame(),
ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end))); ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end)));
generator.SetCallbackForTesting(std::move(callback));
generator.GenerateSelector();
EXPECT_TRUE(callback_called); EXPECT_TRUE(callback_called);
} }
...@@ -96,10 +102,13 @@ TEST_F(TextFragmentSelectorGeneratorTest, ExactTextWithExtraSpace) { ...@@ -96,10 +102,13 @@ TEST_F(TextFragmentSelectorGeneratorTest, ExactTextWithExtraSpace) {
callback_called = true; callback_called = true;
}); });
TextFragmentSelectorGenerator generator(GetDocument().GetFrame(), TextFragmentSelectorGenerator generator;
std::move(callback)); generator.UpdateSelection(
generator.GenerateSelector( GetDocument().GetFrame(),
ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end))); ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end)));
generator.SetCallbackForTesting(std::move(callback));
generator.GenerateSelector();
EXPECT_TRUE(callback_called); EXPECT_TRUE(callback_called);
} }
...@@ -125,10 +134,13 @@ TEST_F(TextFragmentSelectorGeneratorTest, MultiblockSelection) { ...@@ -125,10 +134,13 @@ TEST_F(TextFragmentSelectorGeneratorTest, MultiblockSelection) {
callback_called = true; callback_called = true;
}); });
TextFragmentSelectorGenerator generator(GetDocument().GetFrame(), TextFragmentSelectorGenerator generator;
std::move(callback)); generator.UpdateSelection(
generator.GenerateSelector( GetDocument().GetFrame(),
ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end))); ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end)));
generator.SetCallbackForTesting(std::move(callback));
generator.GenerateSelector();
EXPECT_TRUE(callback_called); EXPECT_TRUE(callback_called);
} }
......
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