diff options
Diffstat (limited to 'chromium/third_party/blink/renderer/core/page/scrolling')
22 files changed, 1419 insertions, 953 deletions
diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/element_fragment_anchor.h b/chromium/third_party/blink/renderer/core/page/scrolling/element_fragment_anchor.h index 3c3847061a7..48f0cab609e 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/element_fragment_anchor.h +++ b/chromium/third_party/blink/renderer/core/page/scrolling/element_fragment_anchor.h @@ -35,6 +35,8 @@ class CORE_EXPORT ElementFragmentAnchor final : public FragmentAnchor { bool should_scroll); ElementFragmentAnchor(Node& anchor_node, LocalFrame& frame); + ElementFragmentAnchor(const ElementFragmentAnchor&) = delete; + ElementFragmentAnchor& operator=(const ElementFragmentAnchor&) = delete; ~ElementFragmentAnchor() override = default; // Will attempt to scroll the anchor into view. @@ -75,8 +77,6 @@ class CORE_EXPORT ElementFragmentAnchor final : public FragmentAnchor { // Invoke has no effect and the fragment can be disposed (unless focus is // still needed). bool needs_invoke_ = false; - - DISALLOW_COPY_AND_ASSIGN(ElementFragmentAnchor); }; } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/main_thread_scrolling_reasons_test.cc b/chromium/third_party/blink/renderer/core/page/scrolling/main_thread_scrolling_reasons_test.cc index 0825e6b80be..c4ae15eebe4 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/main_thread_scrolling_reasons_test.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/main_thread_scrolling_reasons_test.cc @@ -22,6 +22,8 @@ #include "third_party/blink/renderer/core/testing/sim/sim_request.h" #include "third_party/blink/renderer/core/testing/sim/sim_test.h" #include "third_party/blink/renderer/platform/graphics/graphics_layer.h" +#include "third_party/blink/renderer/platform/testing/find_cc_layer.h" +#include "third_party/blink/renderer/platform/testing/paint_test_configurations.h" #include "third_party/blink/renderer/platform/testing/unit_test_helpers.h" #include "third_party/blink/renderer/platform/testing/url_test_helpers.h" @@ -35,7 +37,8 @@ namespace blink { #define EXPECT_NO_MAIN_THREAD_SCROLLING_REASON(actual) \ EXPECT_EQ(cc::MainThreadScrollingReason::kNotScrollingOnMain, actual) -class MainThreadScrollingReasonsTest : public testing::Test { +class MainThreadScrollingReasonsTest : public PaintTestConfigurations, + public testing::Test { public: MainThreadScrollingReasonsTest() : base_url_("http://www.test.com/") { helper_.InitializeWithSettings(&ConfigureSettings); @@ -112,9 +115,11 @@ class MainThreadScrollingReasonsTest : public testing::Test { frame_test_helpers::WebViewHelper helper_; }; +INSTANTIATE_PAINT_TEST_SUITE_P(MainThreadScrollingReasonsTest); + // More cases are tested in LocalFrameViewTest // .RequiresMainThreadScrollingForBackgroundFixedAttachment. -TEST_F(MainThreadScrollingReasonsTest, +TEST_P(MainThreadScrollingReasonsTest, BackgroundAttachmentFixedShouldTriggerMainThreadScroll) { RegisterMockedHttpURLLoad("iframe-background-attachment-fixed.html"); RegisterMockedHttpURLLoad("iframe-background-attachment-fixed-inner.html"); @@ -122,52 +127,76 @@ TEST_F(MainThreadScrollingReasonsTest, NavigateTo(base_url_ + "iframe-background-attachment-fixed.html"); ForceFullCompositingUpdate(); + auto* root_layer = GetFrame()->View()->RootCcLayer(); + auto* outer_layout_view = GetFrame()->View()->GetLayoutView(); Element* iframe = GetFrame()->GetDocument()->getElementById("iframe"); ASSERT_TRUE(iframe); - LayoutObject* layout_object = iframe->GetLayoutObject(); - ASSERT_TRUE(layout_object); - ASSERT_TRUE(layout_object->IsLayoutEmbeddedContent()); - - auto* layout_embedded_content = To<LayoutEmbeddedContent>(layout_object); - ASSERT_TRUE(layout_embedded_content); - - LocalFrameView* inner_frame_view = - To<LocalFrameView>(layout_embedded_content->ChildFrameView()); + LocalFrameView* inner_frame_view = To<LocalFrameView>( + To<LayoutEmbeddedContent>(iframe->GetLayoutObject())->ChildFrameView()); ASSERT_TRUE(inner_frame_view); - auto* inner_layout_view = inner_frame_view->GetLayoutView(); ASSERT_TRUE(inner_layout_view); - PaintLayerCompositor* inner_compositor = inner_layout_view->Compositor(); - ASSERT_TRUE(inner_compositor->InCompositingMode()); - - cc::Layer* cc_scroll_layer = - inner_frame_view->LayoutViewport()->LayerForScrolling(); - ASSERT_TRUE(cc_scroll_layer); - ASSERT_TRUE(IsScrollable(cc_scroll_layer)); + auto* inner_scroll_node = + inner_layout_view->FirstFragment().PaintProperties()->Scroll(); + ASSERT_TRUE(inner_scroll_node); + EXPECT_MAIN_THREAD_SCROLLING_REASON( + cc::MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects, + inner_scroll_node->GetMainThreadScrollingReasons()); + const cc::Layer* inner_scroll_layer = CcLayerByCcElementId( + root_layer, inner_scroll_node->GetCompositorElementId()); + ASSERT_TRUE(inner_scroll_layer); + ASSERT_TRUE(IsScrollable(inner_scroll_layer)); EXPECT_MAIN_THREAD_SCROLLING_REASON( cc::MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects, - GetMainThreadScrollingReasons(cc_scroll_layer)); + GetMainThreadScrollingReasons(inner_scroll_layer)); + + // Main thread scrolling of the inner layer doesn't affect the outer layer. + auto* outer_scroll_node = GetFrame() + ->View() + ->GetLayoutView() + ->FirstFragment() + .PaintProperties() + ->Scroll(); + ASSERT_TRUE(outer_scroll_node); + EXPECT_NO_MAIN_THREAD_SCROLLING_REASON( + outer_scroll_node->GetMainThreadScrollingReasons()); + const cc::Layer* outer_scroll_layer = CcLayerByCcElementId( + root_layer, outer_scroll_node->GetCompositorElementId()); + ASSERT_TRUE(outer_scroll_layer); + ASSERT_TRUE(IsScrollable(outer_scroll_layer)); + EXPECT_NO_MAIN_THREAD_SCROLLING_REASON( + GetMainThreadScrollingReasons(outer_scroll_layer)); - // Remove fixed background-attachment should make the iframe - // scroll on cc. - auto* iframe_doc = To<HTMLIFrameElement>(iframe)->contentDocument(); - iframe = iframe_doc->getElementById("scrollable"); - ASSERT_TRUE(iframe); + // Remove fixed background-attachment should make the iframe scroll on cc. + auto* content = inner_layout_view->GetDocument().getElementById("content"); + ASSERT_TRUE(content); + content->removeAttribute("class"); - iframe->removeAttribute("class"); ForceFullCompositingUpdate(); - layout_object = iframe->GetLayoutObject(); - ASSERT_TRUE(layout_object); + ASSERT_EQ(inner_scroll_node, + inner_layout_view->FirstFragment().PaintProperties()->Scroll()); + EXPECT_NO_MAIN_THREAD_SCROLLING_REASON( + inner_scroll_node->GetMainThreadScrollingReasons()); + ASSERT_EQ(inner_scroll_layer, + CcLayerByCcElementId(root_layer, + inner_scroll_node->GetCompositorElementId())); + ASSERT_TRUE(IsScrollable(inner_scroll_layer)); + EXPECT_NO_MAIN_THREAD_SCROLLING_REASON( + GetMainThreadScrollingReasons(inner_scroll_layer)); - cc_scroll_layer = - layout_object->GetFrameView()->LayoutViewport()->LayerForScrolling(); - ASSERT_TRUE(cc_scroll_layer); - ASSERT_TRUE(IsScrollable(cc_scroll_layer)); + ASSERT_EQ(outer_scroll_node, + outer_layout_view->FirstFragment().PaintProperties()->Scroll()); + EXPECT_NO_MAIN_THREAD_SCROLLING_REASON( + outer_scroll_node->GetMainThreadScrollingReasons()); + ASSERT_EQ(outer_scroll_layer, + CcLayerByCcElementId(root_layer, + outer_scroll_node->GetCompositorElementId())); + ASSERT_TRUE(IsScrollable(outer_scroll_layer)); EXPECT_NO_MAIN_THREAD_SCROLLING_REASON( - GetMainThreadScrollingReasons(cc_scroll_layer)); + GetMainThreadScrollingReasons(outer_scroll_layer)); // Force main frame to scroll on main thread. All its descendants // should scroll on main thread as well. @@ -179,21 +208,37 @@ TEST_F(MainThreadScrollingReasonsTest, ForceFullCompositingUpdate(); - layout_object = iframe->GetLayoutObject(); - ASSERT_TRUE(layout_object); + // Main thread scrolling of the outer layer affects the inner layer. + ASSERT_EQ(inner_scroll_node, + inner_layout_view->FirstFragment().PaintProperties()->Scroll()); + EXPECT_MAIN_THREAD_SCROLLING_REASON( + cc::MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects, + inner_scroll_node->GetMainThreadScrollingReasons()); + ASSERT_EQ(inner_scroll_layer, + CcLayerByCcElementId(root_layer, + inner_scroll_node->GetCompositorElementId())); + ASSERT_TRUE(IsScrollable(inner_scroll_layer)); + EXPECT_MAIN_THREAD_SCROLLING_REASON( + cc::MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects, + GetMainThreadScrollingReasons(inner_scroll_layer)); - cc_scroll_layer = - layout_object->GetFrameView()->LayoutViewport()->LayerForScrolling(); - ASSERT_TRUE(cc_scroll_layer); - ASSERT_TRUE(IsScrollable(cc_scroll_layer)); + ASSERT_EQ(outer_scroll_node, + outer_layout_view->FirstFragment().PaintProperties()->Scroll()); + EXPECT_MAIN_THREAD_SCROLLING_REASON( + cc::MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects, + outer_scroll_node->GetMainThreadScrollingReasons()); + ASSERT_EQ(outer_scroll_layer, + CcLayerByCcElementId(root_layer, + outer_scroll_node->GetCompositorElementId())); + ASSERT_TRUE(IsScrollable(outer_scroll_layer)); EXPECT_MAIN_THREAD_SCROLLING_REASON( cc::MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects, - GetMainThreadScrollingReasons(cc_scroll_layer)); + GetMainThreadScrollingReasons(outer_scroll_layer)); } // Upon resizing the content size, the main thread scrolling reason // kHasBackgroundAttachmentFixedObjects should be updated on all frames -TEST_F(MainThreadScrollingReasonsTest, +TEST_P(MainThreadScrollingReasonsTest, RecalculateMainThreadScrollingReasonsUponResize) { GetWebView()->GetSettings()->SetPreferCompositingToLCDTextEnabled(false); RegisterMockedHttpURLLoad("has-non-layer-viewport-constrained-objects.html"); @@ -224,7 +269,7 @@ TEST_F(MainThreadScrollingReasonsTest, EXPECT_FALSE(GetViewMainThreadScrollingReasons()); } -TEST_F(MainThreadScrollingReasonsTest, FastScrollingCanBeDisabledWithSetting) { +TEST_P(MainThreadScrollingReasonsTest, FastScrollingCanBeDisabledWithSetting) { GetWebView()->MainFrameViewWidget()->Resize(gfx::Size(800, 600)); LoadHTML("<div id='spacer' style='height: 1000px'></div>"); GetWebView()->GetSettings()->SetThreadedScrollingEnabled(false); @@ -241,7 +286,7 @@ TEST_F(MainThreadScrollingReasonsTest, FastScrollingCanBeDisabledWithSetting) { EXPECT_TRUE(GetMainThreadScrollingReasons(visual_viewport_scroll_layer)); } -TEST_F(MainThreadScrollingReasonsTest, FastScrollingForFixedPosition) { +TEST_P(MainThreadScrollingReasonsTest, FastScrollingForFixedPosition) { RegisterMockedHttpURLLoad("fixed-position.html"); NavigateTo(base_url_ + "fixed-position.html"); ForceFullCompositingUpdate(); @@ -250,7 +295,7 @@ TEST_F(MainThreadScrollingReasonsTest, FastScrollingForFixedPosition) { EXPECT_FALSE(GetViewMainThreadScrollingReasons()); } -TEST_F(MainThreadScrollingReasonsTest, FastScrollingForStickyPosition) { +TEST_P(MainThreadScrollingReasonsTest, FastScrollingForStickyPosition) { RegisterMockedHttpURLLoad("sticky-position.html"); NavigateTo(base_url_ + "sticky-position.html"); ForceFullCompositingUpdate(); @@ -259,7 +304,7 @@ TEST_F(MainThreadScrollingReasonsTest, FastScrollingForStickyPosition) { EXPECT_FALSE(GetViewMainThreadScrollingReasons()); } -TEST_F(MainThreadScrollingReasonsTest, FastScrollingByDefault) { +TEST_P(MainThreadScrollingReasonsTest, FastScrollingByDefault) { GetWebView()->MainFrameViewWidget()->Resize(gfx::Size(800, 600)); LoadHTML("<div id='spacer' style='height: 1000px'></div>"); ForceFullCompositingUpdate(); @@ -273,31 +318,6 @@ TEST_F(MainThreadScrollingReasonsTest, FastScrollingByDefault) { EXPECT_FALSE(GetMainThreadScrollingReasons(visual_viewport_scroll_layer)); } -TEST_F(MainThreadScrollingReasonsTest, - ScrollbarsForceMainThreadOrHaveCompositorScrollbarLayer) { - RegisterMockedHttpURLLoad("trivial-scroller.html"); - NavigateTo(base_url_ + "trivial-scroller.html"); - ForceFullCompositingUpdate(); - - Document* document = GetFrame()->GetDocument(); - Element* scrollable_element = document->getElementById("scroller"); - DCHECK(scrollable_element); - - LayoutObject* layout_object = scrollable_element->GetLayoutObject(); - auto* box = To<LayoutBox>(layout_object); - ASSERT_TRUE(box->UsesCompositedScrolling()); - CompositedLayerMapping* composited_layer_mapping = - box->Layer()->GetCompositedLayerMapping(); - GraphicsLayer* scrollbar_graphics_layer = - composited_layer_mapping->LayerForVerticalScrollbar(); - ASSERT_TRUE(scrollbar_graphics_layer); - - bool has_cc_scrollbar_layer = !scrollbar_graphics_layer->DrawsContent(); - EXPECT_TRUE( - has_cc_scrollbar_layer || - GetMainThreadScrollingReasons(scrollbar_graphics_layer->ContentsLayer())); -} - class NonCompositedMainThreadScrollingReasonsTest : public MainThreadScrollingReasonsTest { static const uint32_t kLCDTextRelatedReasons = @@ -371,61 +391,63 @@ class NonCompositedMainThreadScrollingReasonsTest } }; -TEST_F(NonCompositedMainThreadScrollingReasonsTest, TransparentTest) { +INSTANTIATE_PAINT_TEST_SUITE_P(NonCompositedMainThreadScrollingReasonsTest); + +TEST_P(NonCompositedMainThreadScrollingReasonsTest, TransparentTest) { TestNonCompositedReasons("transparent", cc::MainThreadScrollingReason::kNotScrollingOnMain); } -TEST_F(NonCompositedMainThreadScrollingReasonsTest, TransformTest) { +TEST_P(NonCompositedMainThreadScrollingReasonsTest, TransformTest) { TestNonCompositedReasons("transform", cc::MainThreadScrollingReason::kNotScrollingOnMain); } -TEST_F(NonCompositedMainThreadScrollingReasonsTest, BackgroundNotOpaqueTest) { +TEST_P(NonCompositedMainThreadScrollingReasonsTest, BackgroundNotOpaqueTest) { TestNonCompositedReasons( "background-not-opaque", cc::MainThreadScrollingReason::kNotOpaqueForTextAndLCDText); } -TEST_F(NonCompositedMainThreadScrollingReasonsTest, +TEST_P(NonCompositedMainThreadScrollingReasonsTest, CantPaintScrollingBackgroundTest) { TestNonCompositedReasons( "cant-paint-scrolling-background", cc::MainThreadScrollingReason::kCantPaintScrollingBackgroundAndLCDText); } -TEST_F(NonCompositedMainThreadScrollingReasonsTest, ClipTest) { +TEST_P(NonCompositedMainThreadScrollingReasonsTest, ClipTest) { TestNonCompositedReasons("clip", cc::MainThreadScrollingReason::kNotScrollingOnMain); } -TEST_F(NonCompositedMainThreadScrollingReasonsTest, ClipPathTest) { +TEST_P(NonCompositedMainThreadScrollingReasonsTest, ClipPathTest) { TestNonCompositedReasons("clip-path", cc::MainThreadScrollingReason::kNotScrollingOnMain); } -TEST_F(NonCompositedMainThreadScrollingReasonsTest, BoxShadowTest) { +TEST_P(NonCompositedMainThreadScrollingReasonsTest, BoxShadowTest) { TestNonCompositedReasons("box-shadow", cc::MainThreadScrollingReason::kNotScrollingOnMain); } -TEST_F(NonCompositedMainThreadScrollingReasonsTest, InsetBoxShadowTest) { +TEST_P(NonCompositedMainThreadScrollingReasonsTest, InsetBoxShadowTest) { TestNonCompositedReasons( "inset-box-shadow", cc::MainThreadScrollingReason::kCantPaintScrollingBackgroundAndLCDText); } -TEST_F(NonCompositedMainThreadScrollingReasonsTest, StackingContextTest) { +TEST_P(NonCompositedMainThreadScrollingReasonsTest, StackingContextTest) { TestNonCompositedReasons("non-stacking-context", cc::MainThreadScrollingReason::kNotScrollingOnMain); } -TEST_F(NonCompositedMainThreadScrollingReasonsTest, BorderRadiusTest) { +TEST_P(NonCompositedMainThreadScrollingReasonsTest, BorderRadiusTest) { TestNonCompositedReasons("border-radius", cc::MainThreadScrollingReason::kNotScrollingOnMain); } -TEST_F(NonCompositedMainThreadScrollingReasonsTest, +TEST_P(NonCompositedMainThreadScrollingReasonsTest, ForcedComositingWithLCDRelatedReasons) { // With "will-change:transform" we composite elements with // LCDTextRelatedReasons only. For elements with other diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc b/chromium/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc index b9b2d608815..15c6c3b6d56 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc @@ -11,8 +11,8 @@ #include "third_party/blink/public/web/web_remote_frame.h" #include "third_party/blink/public/web/web_script_source.h" #include "third_party/blink/public/web/web_settings.h" -#include "third_party/blink/renderer/bindings/core/v8/node_or_string_or_trusted_script.h" #include "third_party/blink/renderer/bindings/core/v8/v8_union_node_string_trustedscript.h" +#include "third_party/blink/renderer/core/css/css_style_declaration.h" #include "third_party/blink/renderer/core/frame/browser_controls.h" #include "third_party/blink/renderer/core/frame/dom_visual_viewport.h" #include "third_party/blink/renderer/core/frame/frame_test_helpers.h" @@ -218,14 +218,9 @@ TEST_F(RootScrollerTest, defaultEffectiveRootScrollerIsDocumentNode) { // Replace the documentElement with the iframe. The effectiveRootScroller // should remain the same. -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) HeapVector<Member<V8UnionNodeOrStringOrTrustedScript>> nodes; nodes.push_back( MakeGarbageCollected<V8UnionNodeOrStringOrTrustedScript>(iframe)); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - HeapVector<NodeOrStringOrTrustedScript> nodes; - nodes.push_back(NodeOrStringOrTrustedScript::FromNode(iframe)); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) document->documentElement()->ReplaceWith(nodes, ASSERT_NO_EXCEPTION); UpdateAllLifecyclePhases(MainFrameView()); @@ -2854,7 +2849,7 @@ class RootScrollerHitTest : public ImplicitRootScrollerSimTest { .GlobalRootScroller(); ScrollableArea* scrollable_area = To<LayoutBox>(scroller->GetLayoutObject())->GetScrollableArea(); - scrollable_area->DidScroll(FloatPoint(0, 100000)); + scrollable_area->DidCompositorScroll(FloatPoint(0, 100000)); WebView().ResizeWithBrowserControls(gfx::Size(400, 450), 50, 50, false); diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/scroll_customization_callbacks.h b/chromium/third_party/blink/renderer/core/page/scrolling/scroll_customization_callbacks.h index 0cc04b45ee8..f234b179664 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/scroll_customization_callbacks.h +++ b/chromium/third_party/blink/renderer/core/page/scrolling/scroll_customization_callbacks.h @@ -5,7 +5,6 @@ #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_SCROLLING_SCROLL_CUSTOMIZATION_CALLBACKS_H_ #define THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_SCROLLING_SCROLL_CUSTOMIZATION_CALLBACKS_H_ -#include "base/macros.h" #include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/platform/heap/handle.h" #include "third_party/blink/renderer/platform/wtf/hash_map.h" @@ -19,6 +18,10 @@ class CORE_EXPORT ScrollCustomizationCallbacks : public GarbageCollected<ScrollCustomizationCallbacks> { public: ScrollCustomizationCallbacks() = default; + ScrollCustomizationCallbacks(const ScrollCustomizationCallbacks&) = delete; + ScrollCustomizationCallbacks& operator=(const ScrollCustomizationCallbacks&) = + delete; + void SetDistributeScroll(Node*, ScrollStateCallback*); ScrollStateCallback* GetDistributeScroll(Node*); void SetApplyScroll(Node*, ScrollStateCallback*); @@ -39,8 +42,6 @@ class CORE_EXPORT ScrollCustomizationCallbacks ScrollStateCallbackList apply_scroll_callbacks_; ScrollStateCallbackList distribute_scroll_callbacks_; HeapHashMap<WeakMember<Node>, bool> in_scrolling_phase_; - - DISALLOW_COPY_AND_ASSIGN(ScrollCustomizationCallbacks); }; } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc b/chromium/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc index a7815a5217b..d544809d3a2 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc @@ -6,7 +6,6 @@ #include "third_party/blink/public/mojom/frame/find_in_page.mojom-blink.h" #include "third_party/blink/public/mojom/scroll/scroll_into_view_params.mojom-blink.h" #include "third_party/blink/public/web/web_script_source.h" -#include "third_party/blink/renderer/bindings/core/v8/scroll_into_view_options_or_boolean.h" #include "third_party/blink/renderer/bindings/core/v8/v8_scroll_into_view_options.h" #include "third_party/blink/renderer/bindings/core/v8/v8_scroll_to_options.h" #include "third_party/blink/renderer/bindings/core/v8/v8_union_boolean_scrollintoviewoptions.h" @@ -45,14 +44,8 @@ TEST_F(ScrollIntoViewTest, InstantScroll) { Element* content = GetDocument().getElementById("content"); ScrollIntoViewOptions* options = ScrollIntoViewOptions::Create(); options->setBlock("start"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) content->scrollIntoView( MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options)); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - ScrollIntoViewOptionsOrBoolean arg; - arg.SetScrollIntoViewOptions(options); - content->scrollIntoView(arg); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) ASSERT_EQ(Window().scrollY(), content->OffsetTop()); } @@ -171,13 +164,8 @@ TEST_F(ScrollIntoViewTest, SmoothScroll) { ScrollIntoViewOptions* options = ScrollIntoViewOptions::Create(); options->setBlock("start"); options->setBehavior("smooth"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - ScrollIntoViewOptionsOrBoolean arg; - arg.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) Compositor().BeginFrame(); ASSERT_EQ(Window().scrollY(), 0); @@ -215,13 +203,8 @@ TEST_F(ScrollIntoViewTest, NestedContainer) { ScrollIntoViewOptions* options = ScrollIntoViewOptions::Create(); options->setBlock("start"); options->setBehavior("smooth"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - ScrollIntoViewOptionsOrBoolean arg; - arg.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) Compositor().BeginFrame(); ASSERT_EQ(Window().scrollY(), 0); ASSERT_EQ(container->scrollTop(), 0); @@ -281,13 +264,8 @@ TEST_F(ScrollIntoViewTest, NewScrollIntoViewAbortsCurrentAnimation) { ScrollIntoViewOptions* options = ScrollIntoViewOptions::Create(); options->setBlock("start"); options->setBehavior("smooth"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - ScrollIntoViewOptionsOrBoolean arg; - arg.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) Compositor().BeginFrame(); ASSERT_EQ(Window().scrollY(), 0); @@ -358,13 +336,8 @@ TEST_F(ScrollIntoViewTest, ScrollWindowAbortsCurrentAnimation) { ScrollIntoViewOptions* options = ScrollIntoViewOptions::Create(); options->setBlock("start"); options->setBehavior("smooth"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - ScrollIntoViewOptionsOrBoolean arg; - arg.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) Compositor().BeginFrame(); ASSERT_EQ(Window().scrollY(), 0); ASSERT_EQ(container->scrollTop(), 0); @@ -418,20 +391,13 @@ TEST_F(ScrollIntoViewTest, BlockAndInlineSettings) { int window_width = 800; Element* content = GetDocument().getElementById("content"); -#if !defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - ScrollIntoViewOptionsOrBoolean arg1, arg2, arg3, arg4; -#endif // !defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) ScrollIntoViewOptions* options = ScrollIntoViewOptions::Create(); ASSERT_EQ(Window().scrollY(), 0); options->setBlock("nearest"); options->setInlinePosition("nearest"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg1 = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - arg1.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) content->scrollIntoView(arg1); ASSERT_EQ(Window().scrollX(), content->OffsetLeft() + content_width - window_width); @@ -440,24 +406,16 @@ TEST_F(ScrollIntoViewTest, BlockAndInlineSettings) { options->setBlock("start"); options->setInlinePosition("start"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg2 = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - arg2.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) content->scrollIntoView(arg2); ASSERT_EQ(Window().scrollX(), content->OffsetLeft()); ASSERT_EQ(Window().scrollY(), content->OffsetTop()); options->setBlock("center"); options->setInlinePosition("center"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg3 = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - arg3.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) content->scrollIntoView(arg3); ASSERT_EQ(Window().scrollX(), content->OffsetLeft() + (content_width - window_width) / 2); @@ -466,12 +424,8 @@ TEST_F(ScrollIntoViewTest, BlockAndInlineSettings) { options->setBlock("end"); options->setInlinePosition("end"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg4 = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - arg4.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) content->scrollIntoView(arg4); ASSERT_EQ(Window().scrollX(), content->OffsetLeft() + content_width - window_width); @@ -501,13 +455,8 @@ TEST_F(ScrollIntoViewTest, SmoothAndInstantInChain) { Element* content = GetDocument().getElementById("content"); ScrollIntoViewOptions* options = ScrollIntoViewOptions::Create(); options->setBlock("start"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - ScrollIntoViewOptionsOrBoolean arg; - arg.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) Compositor().BeginFrame(); ASSERT_EQ(Window().scrollY(), 0); ASSERT_EQ(container->scrollTop(), 0); @@ -608,13 +557,8 @@ TEST_F(ScrollIntoViewTest, ApplyRootElementScrollBehaviorToViewport) { Element* content = GetDocument().getElementById("content"); ScrollIntoViewOptions* options = ScrollIntoViewOptions::Create(); options->setBlock("start"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - ScrollIntoViewOptionsOrBoolean arg; - arg.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) Compositor().BeginFrame(); ASSERT_EQ(Window().scrollY(), 0); @@ -829,13 +773,8 @@ TEST_F(ScrollIntoViewTest, LongDistanceSmoothScrollFinishedInThreeSeconds) { ScrollIntoViewOptions* options = ScrollIntoViewOptions::Create(); options->setBlock("start"); options->setBehavior("smooth"); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* arg = MakeGarbageCollected<V8UnionBooleanOrScrollIntoViewOptions>(options); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - ScrollIntoViewOptionsOrBoolean arg; - arg.SetScrollIntoViewOptions(options); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) target->scrollIntoView(arg); // Scrolling the window diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc b/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc index ecab9a4aca1..a466b7a5117 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc @@ -90,20 +90,21 @@ ScrollingCoordinator::ScrollableAreaWithElementIdInAllLocalFrames( return nullptr; } -void ScrollingCoordinator::DidScroll( +void ScrollingCoordinator::DidCompositorScroll( CompositorElementId element_id, const gfx::ScrollOffset& offset, const absl::optional<cc::TargetSnapAreaElementIds>& snap_target_ids) { // Find the associated scrollable area using the element id and notify it of // the compositor-side scroll. We explicitly do not check the VisualViewport - // which handles scroll offset differently (see: VisualViewport::didScroll). - // Remote frames will receive DidScroll callbacks from their own compositor. + // which handles scroll offset differently (see: + // VisualViewport::DidCompositorScroll). Remote frames will receive + // DidCompositorScroll callbacks from their own compositor. // The ScrollableArea with matching ElementId may have been deleted and we can - // safely ignore the DidScroll callback. + // safely ignore the DidCompositorScroll callback. auto* scrollable = ScrollableAreaWithElementIdInAllLocalFrames(element_id); if (!scrollable) return; - scrollable->DidScroll(FloatPoint(offset.x(), offset.y())); + scrollable->DidCompositorScroll(FloatPoint(offset.x(), offset.y())); if (snap_target_ids) scrollable->SetTargetSnapAreaElementIds(snap_target_ids.value()); } @@ -179,7 +180,7 @@ cc::ScrollbarLayerBase* ScrollingCoordinator::GetScrollbarLayer( ScrollbarMap& scrollbars = orientation == kHorizontalScrollbar ? horizontal_scrollbars_ : vertical_scrollbars_; - return scrollbars.at(scrollable_area); + return scrollbars.DeprecatedAtOrEmptyValue(scrollable_area); } void ScrollingCoordinator::ScrollableAreaScrollbarLayerDidChange( diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h b/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h index cdae7058097..a8fb445489c 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h +++ b/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h @@ -28,7 +28,6 @@ #include <memory> -#include "base/macros.h" #include "base/memory/weak_ptr.h" #include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/core/scroll/scroll_types.h" @@ -64,6 +63,8 @@ class CORE_EXPORT ScrollingCoordinator final public CompositorScrollCallbacks { public: explicit ScrollingCoordinator(Page*); + ScrollingCoordinator(const ScrollingCoordinator&) = delete; + ScrollingCoordinator& operator=(const ScrollingCoordinator&) = delete; ~ScrollingCoordinator() override; void Trace(Visitor*) const; @@ -112,9 +113,10 @@ class CORE_EXPORT ScrollingCoordinator final const CompositorElementId&); // ScrollCallbacks implementation - void DidScroll(CompositorElementId, - const gfx::ScrollOffset&, - const absl::optional<cc::TargetSnapAreaElementIds>&) override; + void DidCompositorScroll( + CompositorElementId, + const gfx::ScrollOffset&, + const absl::optional<cc::TargetSnapAreaElementIds>&) override; void DidChangeScrollbarsHidden(CompositorElementId, bool hidden) override; base::WeakPtr<ScrollingCoordinator> GetWeakPtr() { @@ -149,8 +151,6 @@ class CORE_EXPORT ScrollingCoordinator final ScrollbarMap vertical_scrollbars_; base::WeakPtrFactory<ScrollingCoordinator> weak_ptr_factory_{this}; - - DISALLOW_COPY_AND_ASSIGN(ScrollingCoordinator); }; } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_context.h b/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_context.h index a4b1b926f3d..149d332a412 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_context.h +++ b/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_context.h @@ -22,8 +22,11 @@ class CORE_EXPORT ScrollingCoordinatorContext final { USING_FAST_MALLOC(ScrollingCoordinatorContext); public: - ScrollingCoordinatorContext() {} - virtual ~ScrollingCoordinatorContext() {} + ScrollingCoordinatorContext() = default; + ScrollingCoordinatorContext(const ScrollingCoordinatorContext&) = delete; + ScrollingCoordinatorContext& operator=(const ScrollingCoordinatorContext&) = + delete; + virtual ~ScrollingCoordinatorContext() = default; void SetAnimationTimeline( std::unique_ptr<CompositorAnimationTimeline> timeline) { @@ -39,8 +42,6 @@ class CORE_EXPORT ScrollingCoordinatorContext final { private: std::unique_ptr<CompositorAnimationTimeline> animation_timeline_; cc::AnimationHost* animation_host_ = nullptr; - - DISALLOW_COPY_AND_ASSIGN(ScrollingCoordinatorContext); }; } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_test.cc b/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_test.cc index 667cc41a941..c6a498fb53d 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_test.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/scrolling_test.cc @@ -33,6 +33,7 @@ #include "cc/trees/sticky_position_constraint.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/blink/public/platform/scheduler/test/renderer_scheduler_test_support.h" +#include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h" #include "third_party/blink/public/platform/web_cache.h" #include "third_party/blink/public/platform/web_url_loader_mock_factory.h" #include "third_party/blink/public/web/web_settings.h" @@ -1319,9 +1320,9 @@ TEST_P(ScrollingTest, PluginBecomesLayoutInline) { ForceFullCompositingUpdate(); } -// Ensure NonFastScrollableRegions are correctly generated for both fixed and -// in-flow plugins that need them. -TEST_P(ScrollingTest, NonFastScrollableRegionsForPlugins) { +// Ensure blocking wheel event regions are correctly generated for both fixed +// and in-flow plugins that need them. +TEST_P(ScrollingTest, WheelEventRegionsForPlugins) { LoadHTML(R"HTML( <style> body { @@ -1351,23 +1352,23 @@ TEST_P(ScrollingTest, NonFastScrollableRegionsForPlugins) { GetFrame()->GetDocument()->getElementById("plugin")); auto* plugin_fixed = To<HTMLObjectElement>( GetFrame()->GetDocument()->getElementById("pluginfixed")); - // NonFastScrollableRegions are generated for plugins that require wheel + // Wheel event regions are generated for plugins that require wheel // events. plugin->OwnedPlugin()->SetWantsWheelEvents(true); plugin_fixed->OwnedPlugin()->SetWantsWheelEvents(true); ForceFullCompositingUpdate(); - // The non-fixed plugin should create a non-fast scrollable region in the + // The non-fixed plugin should create a wheel event region in the // scrolling contents layer of the LayoutView. auto* viewport_non_fast_layer = MainFrameScrollingContentsLayer(); - EXPECT_EQ(viewport_non_fast_layer->non_fast_scrollable_region().bounds(), + EXPECT_EQ(viewport_non_fast_layer->wheel_event_region().bounds(), gfx::Rect(0, 0, 300, 300)); - // The fixed plugin should create a non-fast scrollable region in a fixed + // The fixed plugin should create a wheel event region in a fixed // cc::Layer. auto* fixed_layer = LayerByDOMElementId("fixed"); - EXPECT_EQ(fixed_layer->non_fast_scrollable_region().bounds(), + EXPECT_EQ(fixed_layer->wheel_event_region().bounds(), gfx::Rect(0, 0, 200, 200)); } @@ -2039,7 +2040,7 @@ class UnifiedScrollingSimTest : public SimTest, public PaintTestConfigurations { void RunIdleTasks() { auto* scheduler = - ThreadScheduler::Current()->GetWebMainThreadSchedulerForTest(); + blink::scheduler::WebThreadScheduler::MainThreadScheduler(); blink::scheduler::RunIdleTasksForTesting(scheduler, base::BindOnce([]() {})); test::RunPendingTasks(); diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/snap_coordinator.h b/chromium/third_party/blink/renderer/core/page/scrolling/snap_coordinator.h index 98f72b36490..4aed25c2356 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/snap_coordinator.h +++ b/chromium/third_party/blink/renderer/core/page/scrolling/snap_coordinator.h @@ -5,7 +5,6 @@ #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_SCROLLING_SNAP_COORDINATOR_H_ #define THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_SCROLLING_SNAP_COORDINATOR_H_ -#include "base/macros.h" #include "cc/input/scroll_snap_data.h" #include "cc/input/snap_selection_strategy.h" #include "third_party/blink/renderer/core/core_export.h" @@ -35,6 +34,8 @@ class CORE_EXPORT SnapCoordinator final : public GarbageCollected<SnapCoordinator> { public: explicit SnapCoordinator(); + SnapCoordinator(const SnapCoordinator&) = delete; + SnapCoordinator& operator=(const SnapCoordinator&) = delete; ~SnapCoordinator(); void Trace(Visitor* visitor) const {} @@ -82,8 +83,6 @@ class CORE_EXPORT SnapCoordinator final // Used for reporting to UMA when snapping on the initial layout affects the // initial scroll position. bool did_first_resnap_all_containers_ = false; - - DISALLOW_COPY_AND_ASSIGN(SnapCoordinator); }; } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/sticky_position_scrolling_constraints.h b/chromium/third_party/blink/renderer/core/page/scrolling/sticky_position_scrolling_constraints.h index a9a04127c41..b384d1a371d 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/sticky_position_scrolling_constraints.h +++ b/chromium/third_party/blink/renderer/core/page/scrolling/sticky_position_scrolling_constraints.h @@ -80,6 +80,8 @@ struct StickyPositionScrollingConstraints final { is_anchored_bottom(false) {} StickyPositionScrollingConstraints( const StickyPositionScrollingConstraints& other) = default; + StickyPositionScrollingConstraints& operator=( + const StickyPositionScrollingConstraints& other) = default; // Computes the sticky offset for a given overflow clip rect. // diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.cc b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.cc index 44557e8731e..984f9e614ce 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.cc @@ -34,8 +34,8 @@ bool ParseTextDirective(const String& fragment_directive, Vector<TextFragmentSelector>* out_selectors) { DCHECK(out_selectors); - size_t start_pos = 0; - size_t end_pos = 0; + wtf_size_t start_pos = 0; + wtf_size_t end_pos = 0; while (end_pos != kNotFound) { if (fragment_directive.Find(kTextFragmentIdentifierPrefix, start_pos) != start_pos) { @@ -117,10 +117,11 @@ bool TextFragmentAnchor::GenerateNewToken(const DocumentLoader& loader) { bool TextFragmentAnchor::GenerateNewTokenForSameDocument( const DocumentLoader& loader, WebFrameLoadType load_type, - SameDocumentNavigationSource source) { + mojom::blink::SameDocumentNavigationType same_document_navigation_type) { if ((load_type != WebFrameLoadType::kStandard && load_type != WebFrameLoadType::kReplaceCurrentItem) || - source != kSameDocumentNavigationDefault) + same_document_navigation_type != + mojom::blink::SameDocumentNavigationType::kFragment) return false; // Same-document text fragment navigations are allowed only when initiated @@ -490,7 +491,8 @@ bool TextFragmentAnchor::Dismiss() { // fragment shown in the URL matches the state of the highlight on the page. // This is equivalent to history.replaceState in javascript. frame_->DomWindow()->document()->Loader()->RunURLAndHistoryUpdateSteps( - url, /*data=*/nullptr, WebFrameLoadType::kReplaceCurrentItem, + url, mojom::blink::SameDocumentNavigationType::kFragment, + /*data=*/nullptr, WebFrameLoadType::kReplaceCurrentItem, mojom::blink::ScrollRestorationType::kAuto); return dismissed_; diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.h b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.h index c520078e49b..3d47e7b2a00 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.h +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.h @@ -5,10 +5,10 @@ #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_SCROLLING_TEXT_FRAGMENT_ANCHOR_H_ #define THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_SCROLLING_TEXT_FRAGMENT_ANCHOR_H_ +#include "third_party/blink/public/mojom/loader/same_document_navigation_type.mojom-blink.h" #include "third_party/blink/public/web/web_frame_load_type.h" #include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/core/editing/forward.h" -#include "third_party/blink/renderer/core/loader/frame_loader_types.h" #include "third_party/blink/renderer/core/page/scrolling/element_fragment_anchor.h" #include "third_party/blink/renderer/core/page/scrolling/fragment_anchor.h" #include "third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.h" @@ -50,7 +50,7 @@ class CORE_EXPORT TextFragmentAnchor final : public FragmentAnchor, static bool GenerateNewTokenForSameDocument( const DocumentLoader&, WebFrameLoadType load_type, - SameDocumentNavigationSource source); + mojom::blink::SameDocumentNavigationType same_document_navigation_type); static TextFragmentAnchor* TryCreateFragmentDirective( const KURL& url, @@ -61,6 +61,8 @@ class CORE_EXPORT TextFragmentAnchor final : public FragmentAnchor, const Vector<TextFragmentSelector>& text_fragment_selectors, LocalFrame& frame, bool should_scroll); + TextFragmentAnchor(const TextFragmentAnchor&) = delete; + TextFragmentAnchor& operator=(const TextFragmentAnchor&) = delete; ~TextFragmentAnchor() override = default; bool Invoke() override; @@ -148,8 +150,6 @@ class CORE_EXPORT TextFragmentAnchor final : public FragmentAnchor, } beforematch_state_ = kNoMatchFound; Member<TextFragmentAnchorMetrics> metrics_; - - DISALLOW_COPY_AND_ASSIGN(TextFragmentAnchor); }; } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc index badf5d9a3a2..95db66cce0b 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc @@ -10,6 +10,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "third_party/blink/public/mojom/scroll/scroll_enums.mojom-blink.h" #include "third_party/blink/public/platform/scheduler/test/renderer_scheduler_test_support.h" +#include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h" #include "third_party/blink/renderer/core/frame/local_frame.h" #include "third_party/blink/renderer/core/frame/local_frame_view.h" #include "third_party/blink/renderer/core/input/event_handler.h" @@ -37,7 +38,7 @@ class TextFragmentAnchorMetricsTest : public SimTest { void RunAsyncMatchingTasks() { auto* scheduler = - ThreadScheduler::Current()->GetWebMainThreadSchedulerForTest(); + blink::scheduler::WebThreadScheduler::MainThreadScheduler(); blink::scheduler::RunIdleTasksForTesting(scheduler, base::BindOnce([]() {})); RunPendingTasks(); diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_test.cc b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_test.cc index 8a13df75679..8559bef4176 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_test.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_test.cc @@ -8,8 +8,8 @@ #include "testing/gtest/include/gtest/gtest.h" #include "third_party/blink/public/common/input/web_menu_source_type.h" #include "third_party/blink/public/platform/scheduler/test/renderer_scheduler_test_support.h" +#include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h" #include "third_party/blink/public/public_buildflags.h" -#include "third_party/blink/renderer/bindings/core/v8/string_or_array_buffer_or_array_buffer_view.h" #include "third_party/blink/renderer/bindings/core/v8/v8_font_face_descriptors.h" #include "third_party/blink/renderer/bindings/core/v8/v8_mouse_event_init.h" #include "third_party/blink/renderer/bindings/core/v8/v8_union_arraybuffer_arraybufferview_string.h" @@ -59,7 +59,7 @@ class TextFragmentAnchorTest : public SimTest { void RunAsyncMatchingTasks() { auto* scheduler = - ThreadScheduler::Current()->GetWebMainThreadSchedulerForTest(); + blink::scheduler::WebThreadScheduler::MainThreadScheduler(); blink::scheduler::RunIdleTasksForTesting(scheduler, base::BindOnce([]() {})); RunPendingTasks(); @@ -94,31 +94,35 @@ class TextFragmentAnchorTest : public SimTest { 0, WebInputEvent::Modifiers::kLeftButtonDown, base::TimeTicks::Now()); event.SetFrameScale(1); - GetDocument().GetFrame()->GetEventHandler().HandleMousePressEvent(event); + WebView().MainFrameWidget()->ProcessInputEventSynchronouslyForTesting( + WebCoalescedInputEvent(event, ui::LatencyInfo()), base::DoNothing()); } - void SimulateTap(int x, int y) { - WebGestureEvent event(WebInputEvent::Type::kGestureTap, - WebInputEvent::kNoModifiers, base::TimeTicks::Now(), - WebGestureDevice::kTouchscreen); - event.SetPositionInWidget(gfx::PointF(x, y)); - event.SetPositionInScreen(gfx::PointF(x, y)); + void SimulateRightClick(int x, int y) { + WebMouseEvent event(WebInputEvent::Type::kMouseDown, gfx::PointF(x, y), + gfx::PointF(x, y), WebPointerProperties::Button::kRight, + 0, WebInputEvent::Modifiers::kLeftButtonDown, + base::TimeTicks::Now()); event.SetFrameScale(1); - GetDocument().GetFrame()->GetEventHandler().HandleGestureEvent(event); + WebView().MainFrameWidget()->ProcessInputEventSynchronouslyForTesting( + WebCoalescedInputEvent(event, ui::LatencyInfo()), base::DoNothing()); + } + + void SimulateTap(int x, int y) { + InjectEvent(WebInputEvent::Type::kTouchStart, x, y); + InjectEvent(WebInputEvent::Type::kTouchEnd, x, y); + InjectEvent(WebInputEvent::Type::kGestureTapDown, x, y); + InjectEvent(WebInputEvent::Type::kGestureTapUnconfirmed, x, y); + InjectEvent(WebInputEvent::Type::kGestureShowPress, x, y); + InjectEvent(WebInputEvent::Type::kGestureTap, x, y); } void LoadAhem() { scoped_refptr<SharedBuffer> shared_buffer = test::ReadFromFile(test::CoreTestDataPath("Ahem.ttf")); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* buffer = MakeGarbageCollected<V8UnionArrayBufferOrArrayBufferViewOrString>( DOMArrayBuffer::Create(shared_buffer)); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - StringOrArrayBufferOrArrayBufferView buffer = - StringOrArrayBufferOrArrayBufferView::FromArrayBuffer( - DOMArrayBuffer::Create(shared_buffer)); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) FontFace* ahem = FontFace::Create(GetDocument().GetFrame()->DomWindow(), "Ahem", buffer, FontFaceDescriptors::Create()); @@ -129,6 +133,41 @@ class TextFragmentAnchorTest : public SimTest { FontFaceSetDocument::From(GetDocument()) ->addForBinding(script_state, ahem, exception_state); } + + private: + void InjectEvent(WebInputEvent::Type type, int x, int y) { + if (WebInputEvent::IsGestureEventType(type)) { + WebGestureEvent event(type, WebInputEvent::kNoModifiers, + base::TimeTicks::Now(), + WebGestureDevice::kTouchscreen); + event.SetPositionInWidget(gfx::PointF(x, y)); + event.SetPositionInScreen(gfx::PointF(x, y)); + event.SetFrameScale(1); + + WebView().MainFrameWidget()->ProcessInputEventSynchronouslyForTesting( + WebCoalescedInputEvent(event, ui::LatencyInfo()), base::DoNothing()); + } else if (WebInputEvent::IsTouchEventType(type)) { + WebTouchEvent event(type, WebInputEvent::kNoModifiers, + base::TimeTicks::Now()); + event.SetFrameScale(1); + + WebPointerProperties pointer(0, WebPointerProperties::PointerType::kTouch, + WebPointerProperties::Button::kNoButton, + gfx::PointF(x, y), gfx::PointF(x, y)); + event.touches[0] = pointer; + if (type == WebInputEvent::Type::kTouchStart) + event.touches[0].state = WebTouchPoint::State::kStatePressed; + else if (type == WebInputEvent::Type::kTouchEnd) + event.touches[0].state = WebTouchPoint::State::kStateReleased; + + WebView().MainFrameWidget()->ProcessInputEventSynchronouslyForTesting( + WebCoalescedInputEvent(event, ui::LatencyInfo()), base::DoNothing()); + WebView().MainFrameWidget()->DispatchBufferedTouchEvents(); + } else { + NOTREACHED() << "Only needed to support Gesture/Touch until now. " + "Implement others if new modality is needed."; + } + } }; // Basic test case, ensure we scroll the matching text into view. @@ -2563,6 +2602,76 @@ TEST_F(TextFragmentAnchorTest, } #endif // BUILDFLAG(ENABLE_UNHANDLED_TAP) +TEST_F(TextFragmentAnchorTest, TapOpeningContextMenuWithDirtyLifecycleNoCrash) { + ScopedTextFragmentTapOpensContextMenuForTest tap_opens_context_menu(true); + base::test::ScopedFeatureList feature_list_; + feature_list_.InitAndEnableFeature( + shared_highlighting::kSharedHighlightingV2); + + SimRequest request( + "https://example.com/" + "test.html#:~:text=This%20is%20just%20example", + "text/html"); + LoadURL( + "https://example.com/" + "test.html#:~:text=This%20is%20just%20example"); + request.Complete(R"HTML( + <!DOCTYPE html> + <html> + <head> + <style> + .content { + width: 1000px; + height: 2000px; + background-color: silver; + } + </style> + <script> + // Dirty lifecycle inside the click event. + addEventListener('click', () => { + document.body.style.width = '500px'; + }); + // This prevents calling HandleMouseReleaseEvent which has an + // UpdateLifecycle call inside it but it also prevents showing the + // context menu. + addEventListener('mouseup', (e) => { e.preventDefault(); }); + </script> + </head> + + <body> + This is just example text that will wrap. + <div class="content"></div> + </body> + </html> + )HTML"); + RunAsyncMatchingTasks(); + ContextMenuAllowedScope context_menu_allowed_scope; + + Compositor().BeginFrame(); + + EXPECT_FALSE(GetDocument() + .GetPage() + ->GetContextMenuController() + .ContextMenuNodeForFrame(GetDocument().GetFrame())); + + Node* first_paragraph = GetDocument().body()->firstChild(); + const auto& start = Position(first_paragraph, 0); + const auto& end = Position(first_paragraph, 27); + ASSERT_EQ("This is just example", PlainText(EphemeralRange(start, end))); + + Range* range = CreateRange(EphemeralRange(start, end)); + + IntPoint tap_point = range->BoundingBox().Center(); + SimulateTap(tap_point.X(), tap_point.Y()); + + // Expect that we won't see the context menu because we preventDefaulted the + // mouseup but this test passes if it doesn't crash. + EXPECT_FALSE(GetDocument() + .GetPage() + ->GetContextMenuController() + .ContextMenuNodeForFrame(GetDocument().GetFrame())); +} + } // namespace } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler.cc b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler.cc index 71aad18008f..eca0dec6587 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler.cc @@ -4,15 +4,27 @@ #include "third_party/blink/renderer/core/page/scrolling/text_fragment_handler.h" +#include "base/metrics/histogram_functions.h" +#include "base/strings/strcat.h" +#include "components/shared_highlighting/core/common/disabled_sites.h" #include "components/shared_highlighting/core/common/shared_highlighting_features.h" #include "third_party/blink/public/common/browser_interface_broker_proxy.h" #include "third_party/blink/renderer/core/editing/markers/document_marker.h" #include "third_party/blink/renderer/core/editing/markers/document_marker_controller.h" #include "third_party/blink/renderer/core/editing/position_with_affinity.h" +#include "third_party/blink/renderer/core/editing/range_in_flat_tree.h" +#include "third_party/blink/renderer/core/editing/selection_editor.h" #include "third_party/blink/renderer/core/editing/visible_units.h" #include "third_party/blink/renderer/core/frame/local_frame.h" #include "third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.h" +namespace { +bool PreemptiveGenerationEnabled() { + return base::FeatureList::IsEnabled( + shared_highlighting::kPreemptiveLinkToTextGeneration); +} +} // namespace + namespace blink { TextFragmentHandler::TextFragmentHandler(LocalFrame* main_frame) @@ -28,17 +40,35 @@ void TextFragmentHandler::BindTextFragmentReceiver( blink::TaskType::kInternalDefault)); } -TextFragmentSelectorGenerator* -TextFragmentHandler::GetTextFragmentSelectorGenerator() { - return text_fragment_selector_generator_; -} - void TextFragmentHandler::Cancel() { - GetTextFragmentSelectorGenerator()->Cancel(); + GetTextFragmentSelectorGenerator()->Reset(); } void TextFragmentHandler::RequestSelector(RequestSelectorCallback callback) { - GetTextFragmentSelectorGenerator()->RequestSelector(std::move(callback)); + DCHECK(shared_highlighting::ShouldOfferLinkToText( + GetFrame()->GetDocument()->Url())); + DCHECK(!GetFrame()->Selection().SelectedText().IsEmpty()); + + if (PreemptiveGenerationEnabled()) { + GetTextFragmentSelectorGenerator()->RecordSelectorStateUma(); + + selector_requested_before_ready_ = + !preemptive_generation_result_.has_value(); + response_callback_ = std::move(callback); + + // If preemptive link generation is enabled, the generator would have + // already been invoked when the selection was updated in + // StartPreemptiveGenerationIfNeeded. If that generation finished simply + // respond with the result. Otherwise, the response callback is stored so + // that we reply on completion. + if (!selector_requested_before_ready_.value()) + InvokeReplyCallback(preemptive_generation_result_.value()); + } else { + DCHECK(!preemptive_generation_result_.has_value()); + DCHECK(!response_callback_); + response_callback_ = std::move(callback); + StartGeneratingForCurrentSelection(); + } } void TextFragmentHandler::GetExistingSelectors( @@ -76,6 +106,12 @@ bool TextFragmentHandler::IsOverTextFragment(HitTestResult result) { return false; } + // Tree should be clean before accessing the position. + // |HitTestResult::GetPosition| calls |PositionForPoint()| which requires + // |kPrePaintClean|. + DCHECK_GE(result.InnerNodeFrame()->GetDocument()->Lifecycle().GetState(), + DocumentLifecycle::kPrePaintClean); + DocumentMarkerController& marker_controller = result.InnerNodeFrame()->GetDocument()->Markers(); PositionWithAffinity pos_with_affinity = result.GetPosition(); @@ -128,19 +164,101 @@ void TextFragmentHandler::ExtractFirstFragmentRect( PhysicalRect bounding_box( ComputeTextRect(finder->FirstMatch()->ToEphemeralRange())); rect_in_viewport = - GetTextFragmentSelectorGenerator()->GetFrame()->View()->FrameToViewport( - EnclosingIntRect(bounding_box)); + GetFrame()->View()->FrameToViewport(EnclosingIntRect(bounding_box)); break; } std::move(callback).Run(gfx::Rect(rect_in_viewport)); } +void TextFragmentHandler::DidFinishSelectorGeneration( + const TextFragmentSelector& selector) { + DCHECK(!preemptive_generation_result_.has_value()); + + if (response_callback_) { + InvokeReplyCallback(selector); + } else { + // If we don't have a callback yet, it's because we started generating + // preemptively. We'll store the result so that when the selector actually + // is requested we can simply use the stored result. + DCHECK(PreemptiveGenerationEnabled()); + preemptive_generation_result_.emplace(selector); + } +} + +void TextFragmentHandler::StartGeneratingForCurrentSelection() { + if (GetFrame()->Selection().SelectedText().IsEmpty()) + return; + + VisibleSelectionInFlatTree selection = + GetFrame()->Selection().ComputeVisibleSelectionInFlatTree(); + EphemeralRangeInFlatTree selection_range(selection.Start(), selection.End()); + RangeInFlatTree* current_selection_range = + MakeGarbageCollected<RangeInFlatTree>(selection_range.StartPosition(), + selection_range.EndPosition()); + GetTextFragmentSelectorGenerator()->Generate( + *current_selection_range, + WTF::Bind(&TextFragmentHandler::DidFinishSelectorGeneration, + WrapWeakPersistent(this))); +} + +void TextFragmentHandler::RecordPreemptiveGenerationMetrics( + const TextFragmentSelector& selector) { + DCHECK(PreemptiveGenerationEnabled()); + DCHECK(selector_requested_before_ready_.has_value()); + + bool success = + selector.Type() != TextFragmentSelector::SelectorType::kInvalid; + + std::string uma_prefix = "SharedHighlights.LinkGenerated"; + if (selector_requested_before_ready_.value()) { + uma_prefix = base::StrCat({uma_prefix, ".RequestedBeforeReady"}); + } else { + uma_prefix = base::StrCat({uma_prefix, ".RequestedAfterReady"}); + } + base::UmaHistogramBoolean(uma_prefix, success); + + if (!success) { + absl::optional<shared_highlighting::LinkGenerationError> optional_error = + GetTextFragmentSelectorGenerator()->GetError(); + shared_highlighting::LinkGenerationError error = + optional_error.has_value() + ? optional_error.value() + : shared_highlighting::LinkGenerationError::kUnknown; + base::UmaHistogramEnumeration( + "SharedHighlights.LinkGenerated.Error.Requested", error); + } +} + +void TextFragmentHandler::StartPreemptiveGenerationIfNeeded() { + if (PreemptiveGenerationEnabled() && + shared_highlighting::ShouldOfferLinkToText( + GetFrame()->GetDocument()->Url())) { + preemptive_generation_result_.reset(); + StartGeneratingForCurrentSelection(); + } +} + void TextFragmentHandler::Trace(Visitor* visitor) const { visitor->Trace(text_fragment_selector_generator_); visitor->Trace(selector_producer_); } +void TextFragmentHandler::DidDetachDocumentOrFrame() { + // Clear out any state in the generator and cancel pending tasks so they + // don't run after frame detachment. + GetTextFragmentSelectorGenerator()->Reset(); +} + +void TextFragmentHandler::InvokeReplyCallback( + const TextFragmentSelector& selector) { + if (PreemptiveGenerationEnabled()) + RecordPreemptiveGenerationMetrics(selector); + + DCHECK(response_callback_); + std::move(response_callback_).Run(selector.ToString()); +} + TextFragmentAnchor* TextFragmentHandler::GetTextFragmentAnchor() { FragmentAnchor* fragmentAnchor = GetTextFragmentSelectorGenerator() ->GetFrame() diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler.h b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler.h index 82ec06fe54e..6a74f841826 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler.h +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler.h @@ -6,68 +6,99 @@ #define THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_SCROLLING_TEXT_FRAGMENT_HANDLER_H_ #include "third_party/blink/public/mojom/link_to_text/link_to_text.mojom-blink.h" -#include "third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.h" #include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h" #include "third_party/blink/renderer/platform/mojo/heap_mojo_receiver.h" namespace blink { class LocalFrame; - -// TextFragmentHandler is responsible for handling text fragment operations -// on a LocalFrame. Generating text fragment selectors for a selection is -// delegated to TextFragmentSelectorGenerator. +class TextFragmentAnchor; + +// TextFragmentHandler is responsible for handling requests from the +// browser-side link-to-text/shared-highlighting feature. It is responsible for +// generating a text fragment URL based on the current selection as well as +// collecting information about and modifying text fragments on the current +// page. This class is registered on and owned by the frame that interacts with +// the link-to-text/shared-highlighting feature. class CORE_EXPORT TextFragmentHandler final : public GarbageCollected<TextFragmentHandler>, public blink::mojom::blink::TextFragmentReceiver { public: explicit TextFragmentHandler(LocalFrame* main_frame); + TextFragmentHandler(const TextFragmentHandler&) = delete; + TextFragmentHandler& operator=(const TextFragmentHandler&) = delete; - void BindTextFragmentReceiver( - mojo::PendingReceiver<mojom::blink::TextFragmentReceiver> producer); + // Determine if |result| represents a click on an existing highlight. + static bool IsOverTextFragment(HitTestResult result); - // Cancel any pending selector requests. + // mojom::blink::TextFragmentReceiver interface void Cancel() override; - - // Requests selector for current selection. void RequestSelector(RequestSelectorCallback callback) override; - - // Requests selectors for all existing highlights on the page. void GetExistingSelectors(GetExistingSelectorsCallback callback) override; - - // Remove all text fragments from the current frame. void RemoveFragments() override; - - // Determine if |result| represents a click on an existing highlight. - static bool IsOverTextFragment(HitTestResult result); - - // Retrieves the text fragments matches from the fragment directive. void ExtractTextFragmentsMatches( ExtractTextFragmentsMatchesCallback callback) override; - - // Request the bounding rectangle, relative to the viewport, of the first - // found match. It will accept an empty rectangle if no matches are found. void ExtractFirstFragmentRect( ExtractFirstFragmentRectCallback callback) override; + // This starts the preemptive generation on the current selection if it is not + // empty. + void StartPreemptiveGenerationIfNeeded(); + + void BindTextFragmentReceiver( + mojo::PendingReceiver<mojom::blink::TextFragmentReceiver> producer); + void Trace(Visitor*) const; - TextFragmentSelectorGenerator* GetTextFragmentSelectorGenerator(); + TextFragmentSelectorGenerator* GetTextFragmentSelectorGenerator() { + return text_fragment_selector_generator_; + } + + void DidDetachDocumentOrFrame(); private: + // The callback passed to TextFragmentSelectorGenerator that will receive the + // result. + void DidFinishSelectorGeneration(const TextFragmentSelector& selector); + + // This starts running the generator over the current selection. + // The result will be returned by invoking DidFinishSelectorGeneration(). + void StartGeneratingForCurrentSelection(); + + void RecordPreemptiveGenerationMetrics(const TextFragmentSelector& selector); + + // Called to reply to the client's RequestSelector call with the result. + void InvokeReplyCallback(const TextFragmentSelector& selector); + + TextFragmentAnchor* GetTextFragmentAnchor(); + + LocalFrame* GetFrame() { + return GetTextFragmentSelectorGenerator()->GetFrame(); + } + // Class responsible for generating text fragment selectors for the current // selection. Member<TextFragmentSelectorGenerator> text_fragment_selector_generator_; + // The result of preemptively generating on selection changes will be stored + // in this member when completed. Used only in preemptive link generation + // mode. + absl::optional<TextFragmentSelector> preemptive_generation_result_; + + // Reports whether |RequestSelector| was called before or after selector was + // ready. Used only in preemptive link generation mode. + absl::optional<bool> selector_requested_before_ready_; + + // This will hold the reply callback to the RequestSelector mojo call. This + // will be invoked in InvokeReplyCallback to send the reply back to the + // browser. + RequestSelectorCallback response_callback_; + // Used for communication between |TextFragmentHandler| in renderer // and |TextFragmentSelectorClientImpl| in browser. HeapMojoReceiver<blink::mojom::blink::TextFragmentReceiver, TextFragmentHandler> selector_producer_{this, nullptr}; - - DISALLOW_COPY_AND_ASSIGN(TextFragmentHandler); - - TextFragmentAnchor* GetTextFragmentAnchor(); }; } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler_test.cc b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler_test.cc index c46686b496e..d6b1c0a235d 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler_test.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_handler_test.cc @@ -6,18 +6,22 @@ #include <gtest/gtest.h> +#include "base/feature_list.h" +#include "base/test/metrics/histogram_tester.h" #include "base/test/scoped_feature_list.h" #include "components/shared_highlighting/core/common/shared_highlighting_features.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/blink/public/platform/scheduler/test/renderer_scheduler_test_support.h" -#include "third_party/blink/renderer/bindings/core/v8/string_or_array_buffer_or_array_buffer_view.h" +#include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h" #include "third_party/blink/renderer/bindings/core/v8/v8_font_face_descriptors.h" #include "third_party/blink/renderer/bindings/core/v8/v8_mouse_event_init.h" #include "third_party/blink/renderer/bindings/core/v8/v8_union_arraybuffer_arraybufferview_string.h" #include "third_party/blink/renderer/core/css/css_font_face.h" #include "third_party/blink/renderer/core/css/font_face_set_document.h" +#include "third_party/blink/renderer/core/editing/frame_selection.h" #include "third_party/blink/renderer/core/editing/markers/document_marker_controller.h" #include "third_party/blink/renderer/core/editing/visible_units.h" +#include "third_party/blink/renderer/core/html/html_frame_owner_element.h" #include "third_party/blink/renderer/core/testing/sim/sim_request.h" #include "third_party/blink/renderer/core/testing/sim/sim_test.h" #include "third_party/blink/renderer/platform/testing/unit_test_helpers.h" @@ -28,21 +32,73 @@ namespace { using test::RunPendingTasks; -class TextFragmentHandlerTest : public SimTest { +class TextFragmentHandlerTest : public SimTest, + public ::testing::WithParamInterface<bool> { public: void SetUp() override { SimTest::SetUp(); WebView().MainFrameViewWidget()->Resize(gfx::Size(800, 600)); + + std::vector<base::Feature> enabled; + std::vector<base::Feature> disabled; + + enabled.push_back(shared_highlighting::kSharedHighlightingV2); + + preemptive_generation_enabled_ = GetParam(); + if (preemptive_generation_enabled_) + enabled.push_back(shared_highlighting::kPreemptiveLinkToTextGeneration); + else + disabled.push_back(shared_highlighting::kPreemptiveLinkToTextGeneration); + + feature_list_.InitWithFeatures(enabled, disabled); + } + + void BeginEmptyFrame() { + // If a test case doesn't find a match and therefore doesn't schedule the + // beforematch event, we should still render a second frame as if we did + // schedule the event to retain test coverage. + // When the beforematch event is not scheduled, a DCHECK will fail on + // BeginFrame() because no event was scheduled, so we schedule an empty task + // here. + GetDocument().EnqueueAnimationFrameTask(WTF::Bind([]() {})); + Compositor().BeginFrame(); } void RunAsyncMatchingTasks() { auto* scheduler = - ThreadScheduler::Current()->GetWebMainThreadSchedulerForTest(); + blink::scheduler::WebThreadScheduler::MainThreadScheduler(); blink::scheduler::RunIdleTasksForTesting(scheduler, base::BindOnce([]() {})); RunPendingTasks(); } + void SetSelection(const Position& start, const Position& end) { + GetDocument().GetFrame()->Selection().SetSelection( + SelectionInDOMTree::Builder().SetBaseAndExtent(start, end).Build(), + SetSelectionOptions()); + } + + String SelectThenRequestSelector(const Position& start, const Position& end) { + SetSelection(start, end); + + GetTextFragmentHandler().StartPreemptiveGenerationIfNeeded(); + + bool callback_called = false; + String selector; + auto lambda = [](bool& callback_called, String& selector, + const String& generated_selector) { + selector = generated_selector; + callback_called = true; + }; + auto callback = + WTF::Bind(lambda, std::ref(callback_called), std::ref(selector)); + GetTextFragmentHandler().RequestSelector(std::move(callback)); + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(callback_called); + return selector; + } + Vector<String> ExtractTextFragmentsMatches() { bool callback_called = false; Vector<String> target_texts; @@ -54,10 +110,7 @@ class TextFragmentHandlerTest : public SimTest { auto callback = WTF::Bind(lambda, std::ref(callback_called), std::ref(target_texts)); - GetDocument() - .GetFrame() - ->GetTextFragmentHandler() - ->ExtractTextFragmentsMatches(std::move(callback)); + GetTextFragmentHandler().ExtractTextFragmentsMatches(std::move(callback)); EXPECT_TRUE(callback_called); return target_texts; @@ -74,10 +127,7 @@ class TextFragmentHandlerTest : public SimTest { auto callback = WTF::Bind(lambda, std::ref(callback_called), std::ref(text_fragment_rect)); - GetDocument() - .GetFrame() - ->GetTextFragmentHandler() - ->ExtractFirstFragmentRect(std::move(callback)); + GetTextFragmentHandler().ExtractFirstFragmentRect(std::move(callback)); EXPECT_TRUE(callback_called); return text_fragment_rect; @@ -86,15 +136,9 @@ class TextFragmentHandlerTest : public SimTest { void LoadAhem() { scoped_refptr<SharedBuffer> shared_buffer = test::ReadFromFile(test::CoreTestDataPath("Ahem.ttf")); -#if defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) auto* buffer = MakeGarbageCollected<V8UnionArrayBufferOrArrayBufferViewOrString>( DOMArrayBuffer::Create(shared_buffer)); -#else // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) - StringOrArrayBufferOrArrayBufferView buffer = - StringOrArrayBufferOrArrayBufferView::FromArrayBuffer( - DOMArrayBuffer::Create(shared_buffer)); -#endif // defined(USE_BLINK_V8_BINDING_NEW_IDL_UNION) FontFace* ahem = FontFace::Create(GetDocument().GetExecutionContext(), "Ahem", buffer, FontFaceDescriptors::Create()); @@ -105,12 +149,60 @@ class TextFragmentHandlerTest : public SimTest { FontFaceSetDocument::From(GetDocument()) ->addForBinding(script_state, ahem, exception_state); } -}; -TEST_F(TextFragmentHandlerTest, RemoveTextFragments) { + void VerifyPreemptiveGenerationMetrics(bool success) { + if (!preemptive_generation_enabled_) { + histogram_tester_.ExpectTotalCount( + "SharedHighlights.LinkGenerated.Error.Requested", 0); + histogram_tester_.ExpectTotalCount( + "SharedHighlights.LinkGenerated.RequestedAfterReady", 0); + histogram_tester_.ExpectTotalCount( + "SharedHighlights.LinkGenerated.RequestedBeforeReady", 0); + } else { + EXPECT_EQ( + 1u, histogram_tester_ + .GetAllSamples( + "SharedHighlights.LinkGenerated.RequestedAfterReady") + .size() + + histogram_tester_ + .GetAllSamples( + "SharedHighlights.LinkGenerated.RequestedBeforeReady") + .size()); + + if (!success) { + histogram_tester_.ExpectTotalCount( + "SharedHighlights.LinkGenerated.Error.Requested", 1); + } else { + histogram_tester_.ExpectTotalCount( + "SharedHighlights.LinkGenerated.Error.Requested", 0); + } + } + + // Check async task metrics. + EXPECT_LT(0u, histogram_tester_ + .GetAllSamples("SharedHighlights.AsyncTask.Iterations") + .size()); + EXPECT_LT(0u, + histogram_tester_ + .GetAllSamples("SharedHighlights.AsyncTask.SearchDuration") + .size()); + } + + TextFragmentHandler& GetTextFragmentHandler() { + return *GetDocument().GetFrame()->GetTextFragmentHandler(); + } + + bool HasTextFragmentHandler(LocalFrame* frame) { + return frame->GetTextFragmentHandler(); + } + + protected: + base::HistogramTester histogram_tester_; base::test::ScopedFeatureList feature_list_; - feature_list_.InitAndEnableFeature( - shared_highlighting::kSharedHighlightingV2); + bool preemptive_generation_enabled_; +}; + +TEST_P(TextFragmentHandlerTest, RemoveTextFragments) { SimRequest request( "https://example.com/" "test.html#:~:text=test%20page&text=more%20text", @@ -144,7 +236,7 @@ TEST_F(TextFragmentHandlerTest, RemoveTextFragments) { EXPECT_EQ(2u, GetDocument().Markers().Markers().size()); - GetDocument().GetFrame()->GetTextFragmentHandler()->RemoveFragments(); + GetTextFragmentHandler().RemoveFragments(); EXPECT_EQ(0u, GetDocument().Markers().Markers().size()); @@ -152,11 +244,8 @@ TEST_F(TextFragmentHandlerTest, RemoveTextFragments) { EXPECT_FALSE(GetDocument().View()->GetFragmentAnchor()); } -TEST_F(TextFragmentHandlerTest, +TEST_P(TextFragmentHandlerTest, ExtractTextFragmentWithWithMultipleTextFragments) { - base::test::ScopedFeatureList feature_list_; - feature_list_.InitAndEnableFeature( - shared_highlighting::kSharedHighlightingV2); SimRequest request( "https://example.com/" "test.html#:~:text=test%20page&text=more%20text", @@ -195,10 +284,7 @@ TEST_F(TextFragmentHandlerTest, EXPECT_EQ("more text", target_texts[1]); } -TEST_F(TextFragmentHandlerTest, ExtractTextFragmentWithNoMatch) { - base::test::ScopedFeatureList feature_list_; - feature_list_.InitAndEnableFeature( - shared_highlighting::kSharedHighlightingV2); +TEST_P(TextFragmentHandlerTest, ExtractTextFragmentWithNoMatch) { SimRequest request( "https://example.com/" "test.html#:~:text=not%20on%20the%20page", @@ -234,10 +320,7 @@ TEST_F(TextFragmentHandlerTest, ExtractTextFragmentWithNoMatch) { EXPECT_EQ(0u, target_texts.size()); } -TEST_F(TextFragmentHandlerTest, ExtractTextFragmentWithRange) { - base::test::ScopedFeatureList feature_list_; - feature_list_.InitAndEnableFeature( - shared_highlighting::kSharedHighlightingV2); +TEST_P(TextFragmentHandlerTest, ExtractTextFragmentWithRange) { SimRequest request( "https://example.com/" "test.html#:~:text=This,text", @@ -274,10 +357,7 @@ TEST_F(TextFragmentHandlerTest, ExtractTextFragmentWithRange) { EXPECT_EQ("This is a test page, with some more text", target_texts[0]); } -TEST_F(TextFragmentHandlerTest, ExtractTextFragmentWithRangeAndContext) { - base::test::ScopedFeatureList feature_list_; - feature_list_.InitAndEnableFeature( - shared_highlighting::kSharedHighlightingV2); +TEST_P(TextFragmentHandlerTest, ExtractTextFragmentWithRangeAndContext) { SimRequest request( "https://example.com/" "test.html#:~:text=this,is&text=a-,test,page&text=with,some,-content&" @@ -308,10 +388,7 @@ TEST_F(TextFragmentHandlerTest, ExtractTextFragmentWithRangeAndContext) { EXPECT_EQ("nothing at", target_texts[3]); } -TEST_F(TextFragmentHandlerTest, ExtractFirstTextFragmentRect) { - base::test::ScopedFeatureList feature_list_; - feature_list_.InitAndEnableFeature( - shared_highlighting::kSharedHighlightingV2); +TEST_P(TextFragmentHandlerTest, ExtractFirstTextFragmentRect) { SimRequest request( "https://example.com/" "test.html#:~:text=This,page", @@ -349,10 +426,7 @@ TEST_F(TextFragmentHandlerTest, ExtractFirstTextFragmentRect) { EXPECT_EQ(expected_rect.ToString(), text_fragment_rect.ToString()); } -TEST_F(TextFragmentHandlerTest, ExtractFirstTextFragmentRectScroll) { - base::test::ScopedFeatureList feature_list_; - feature_list_.InitAndEnableFeature( - shared_highlighting::kSharedHighlightingV2); +TEST_P(TextFragmentHandlerTest, ExtractFirstTextFragmentRectScroll) { // Android settings to correctly extract the rect when the page is loaded // zoomed in WebView().GetPage()->GetSettings().SetViewportEnabled(true); @@ -401,10 +475,7 @@ TEST_F(TextFragmentHandlerTest, ExtractFirstTextFragmentRectScroll) { EXPECT_EQ(expected_rect.ToString(), text_fragment_rect.ToString()); } -TEST_F(TextFragmentHandlerTest, ExtractFirstTextFragmentRectMultipleHighlight) { - base::test::ScopedFeatureList feature_list_; - feature_list_.InitAndEnableFeature( - shared_highlighting::kSharedHighlightingV2); +TEST_P(TextFragmentHandlerTest, ExtractFirstTextFragmentRectMultipleHighlight) { SimRequest request( "https://example.com/" "test.html#:~:text=test%20page&text=more%20text", @@ -453,11 +524,8 @@ TEST_F(TextFragmentHandlerTest, ExtractFirstTextFragmentRectMultipleHighlight) { EXPECT_EQ(expected_rect.ToString(), text_fragment_rect.ToString()); } -TEST_F(TextFragmentHandlerTest, +TEST_P(TextFragmentHandlerTest, ExtractFirstTextFragmentRectMultipleHighlightWithNoFoundText) { - base::test::ScopedFeatureList feature_list_; - feature_list_.InitAndEnableFeature( - shared_highlighting::kSharedHighlightingV2); SimRequest request( "https://example.com/" "test.html#:~:text=fake&text=test%20page", @@ -505,10 +573,7 @@ TEST_F(TextFragmentHandlerTest, EXPECT_EQ(expected_rect.ToString(), text_fragment_rect.ToString()); } -TEST_F(TextFragmentHandlerTest, RejectExtractFirstTextFragmentRect) { - base::test::ScopedFeatureList feature_list_; - feature_list_.InitAndEnableFeature( - shared_highlighting::kSharedHighlightingV2); +TEST_P(TextFragmentHandlerTest, RejectExtractFirstTextFragmentRect) { SimRequest request( "https://example.com/" "test.html#:~:text=not%20on%20the%20page", @@ -546,6 +611,331 @@ TEST_F(TextFragmentHandlerTest, RejectExtractFirstTextFragmentRect) { EXPECT_TRUE(text_fragment_rect.IsEmpty()); } +// Checks that the selector is preemptively generated. +TEST_P(TextFragmentHandlerTest, CheckPreemptiveGeneration) { + if (!preemptive_generation_enabled_) + return; + + SimRequest request("https://example.com/test.html", "text/html"); + LoadURL("https://example.com/test.html"); + request.Complete(R"HTML( + <!DOCTYPE html> + <p id='first'>First paragraph</p> + )HTML"); + + Node* first_paragraph = GetDocument().getElementById("first")->firstChild(); + const auto& selected_start = Position(first_paragraph, 0); + const auto& selected_end = Position(first_paragraph, 5); + ASSERT_EQ("First", PlainText(EphemeralRange(selected_start, selected_end))); + + SetSelection(selected_start, selected_end); + GetTextFragmentHandler().StartPreemptiveGenerationIfNeeded(); + + base::RunLoop().RunUntilIdle(); + + histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated", 1); + histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated.Error", 0); +} + +// When URL is blocklisted, the selector shouldn't be preemptively generated. +TEST_P(TextFragmentHandlerTest, CheckNoPreemptiveGenerationBlocklist) { + if (!preemptive_generation_enabled_) + return; + + SimRequest request("https://instagram.com/test.html", "text/html"); + LoadURL("https://instagram.com/test.html"); + request.Complete(R"HTML( + <!DOCTYPE html> + <p id='first'>First paragraph</p> + )HTML"); + + Node* first_paragraph = GetDocument().getElementById("first")->firstChild(); + const auto& selected_start = Position(first_paragraph, 0); + const auto& selected_end = Position(first_paragraph, 5); + ASSERT_EQ("First", PlainText(EphemeralRange(selected_start, selected_end))); + + SetSelection(selected_start, selected_end); + GetTextFragmentHandler().StartPreemptiveGenerationIfNeeded(); + + base::RunLoop().RunUntilIdle(); + + histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated", 0); + histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated.Error", 0); +} + +// Check that selector is not generated for editable text. +TEST_P(TextFragmentHandlerTest, CheckNoPreemptiveGenerationEditable) { + if (!preemptive_generation_enabled_) + return; + + SimRequest request("https://instagram.com/test.html", "text/html"); + LoadURL("https://instagram.com/test.html"); + request.Complete(R"HTML( + <!DOCTYPE html> + <input type="text" id="input" value="default text in input"> + )HTML"); + + Node* input_text = + FlatTreeTraversal::Next(*GetDocument().getElementById("input")) + ->firstChild(); + const auto& selected_start = Position(input_text, 0); + const auto& selected_end = Position(input_text, 12); + ASSERT_EQ("default text", + PlainText(EphemeralRange(selected_start, selected_end))); + + SetSelection(selected_start, selected_end); + GetTextFragmentHandler().StartPreemptiveGenerationIfNeeded(); + + base::RunLoop().RunUntilIdle(); + + histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated", 0); + histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated.Error", 0); +} + +// TODO(crbug.com/1192047): Update the test to better reflect the real repro +// steps. Test case for crash in crbug.com/1190137. When selector is requested +// after callback is set and unused. +TEST_P(TextFragmentHandlerTest, SecondGenerationCrash) { + SimRequest request("https://example.com/test.html", "text/html"); + LoadURL("https://example.com/test.html"); + request.Complete(R"HTML( + <!DOCTYPE html> + <p id='p'>First paragraph text</p> + )HTML"); + GetDocument().UpdateStyleAndLayoutTree(); + Node* p = GetDocument().getElementById("p"); + const auto& start = Position(p->lastChild(), 0); + const auto& end = Position(p->lastChild(), 15); + ASSERT_EQ("First paragraph", PlainText(EphemeralRange(start, end))); + SetSelection(start, end); + + auto callback = WTF::Bind([](const TextFragmentSelector& selector) {}); + GetDocument() + .GetFrame() + ->GetTextFragmentHandler() + ->GetTextFragmentSelectorGenerator() + ->SetCallbackForTesting(std::move(callback)); + + // This shouldn't crash. + GetTextFragmentHandler().StartPreemptiveGenerationIfNeeded(); + base::RunLoop().RunUntilIdle(); +} + +// Verifies metrics for preemptive generation are correctly recorded when the +// selector is successfully generated. +TEST_P(TextFragmentHandlerTest, CheckMetrics_Success) { + base::test::ScopedFeatureList feature_list; + // Basic exact selector case. + SimRequest request("https://example.com/test.html", "text/html"); + LoadURL("https://example.com/test.html"); + request.Complete(R"HTML( + <!DOCTYPE html> + <div>Test page</div> + <p id='first'>First paragraph text that is longer than 20 chars</p> + <p id='second'>Second paragraph text</p> + )HTML"); + Node* first_paragraph = GetDocument().getElementById("first")->firstChild(); + const auto& selected_start = Position(first_paragraph, 0); + const auto& selected_end = Position(first_paragraph, 28); + ASSERT_EQ("First paragraph text that is", + PlainText(EphemeralRange(selected_start, selected_end))); + + String selector = SelectThenRequestSelector(selected_start, selected_end); + EXPECT_EQ(selector, "First%20paragraph%20text%20that%20is"); + VerifyPreemptiveGenerationMetrics(true); +} + +// Verifies metrics for preemptive generation are correctly recorded when the +// selector request fails, in this case, because the context limit is reached. +TEST_P(TextFragmentHandlerTest, CheckMetrics_Failure) { + SimRequest request("https://example.com/test.html", "text/html"); + LoadURL("https://example.com/test.html"); + request.Complete(R"HTML( + <!DOCTYPE html> + <div>Test page</div> + <p id='first'>First paragraph prefix one two three four five six seven + eight nine ten to not unique snippet of text followed by suffix</p> + <p id='second'>Second paragraph prefix one two three four five six seven + eight nine ten to not unique snippet of text followed by suffix</p> + )HTML"); + Node* first_paragraph = GetDocument().getElementById("first")->firstChild(); + const auto& selected_start = Position(first_paragraph, 80); + const auto& selected_end = Position(first_paragraph, 106); + ASSERT_EQ("not unique snippet of text", + PlainText(EphemeralRange(selected_start, selected_end))); + String selector = SelectThenRequestSelector(selected_start, selected_end); + EXPECT_EQ(selector, ""); + VerifyPreemptiveGenerationMetrics(false); +} + +TEST_P(TextFragmentHandlerTest, + ShouldCreateTextFragmentHandlerAndRemoveHighlightForIframes) { + base::test::ScopedFeatureList feature_list_; + feature_list_.InitAndEnableFeature( + shared_highlighting::kSharedHighlightingAmp); + SimRequest main_request("https://example.com/test.html", "text/html"); + SimRequest child_request("https://example.com/child.html", "text/html"); + LoadURL("https://example.com/test.html"); + main_request.Complete(R"HTML( + <!DOCTYPE html> + <iframe id="iframe" src="child.html"></iframe> + )HTML"); + + child_request.Complete(R"HTML( + <!DOCTYPE html> + <style> + p { + margin-top: 1000px; + } + </style> + <p> + test + </p> + <script> + window.location.hash = ':~:text=test'; + </script> + )HTML"); + RunAsyncMatchingTasks(); + + // Render two frames to handle the async step added by the beforematch event. + Compositor().BeginFrame(); + BeginEmptyFrame(); + + Element* iframe = GetDocument().getElementById("iframe"); + auto* child_frame = + To<LocalFrame>(To<HTMLFrameOwnerElement>(iframe)->ContentFrame()); + + EXPECT_EQ(1u, child_frame->GetDocument()->Markers().Markers().size()); + EXPECT_FALSE(HasTextFragmentHandler(child_frame)); + + child_frame->CreateTextFragmentHandler(); + GetTextFragmentHandler().StartPreemptiveGenerationIfNeeded(); + + mojo::Remote<mojom::blink::TextFragmentReceiver> remote; + EXPECT_FALSE(remote.is_bound()); + child_frame->BindTextFragmentReceiver(remote.BindNewPipeAndPassReceiver()); + + EXPECT_TRUE(HasTextFragmentHandler(child_frame)); + EXPECT_TRUE(remote.is_bound()); + remote.get()->RemoveFragments(); + + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(0u, child_frame->GetDocument()->Markers().Markers().size()); + + // Ensure the fragment is uninstalled + EXPECT_FALSE(child_frame->GetDocument()->View()->GetFragmentAnchor()); +} + +TEST_P(TextFragmentHandlerTest, + ShouldCreateTextFragmentHandlerAndRemoveHighlight) { + SimRequest request( + "https://example.com/" + "test.html#:~:text=test%20page&text=more%20text", + "text/html"); + LoadURL( + "https://example.com/" + "test.html#:~:text=test%20page&text=more%20text"); + request.Complete(R"HTML( + <!DOCTYPE html> + <style> + body { + height: 2200px; + } + #first { + position: absolute; + top: 1000px; + } + #second { + position: absolute; + top: 2000px; + } + </style> + <p id="first">This is a test page</p> + <p id="second">With some more text</p> + )HTML"); + RunAsyncMatchingTasks(); + + // Render two frames to handle the async step added by the beforematch event. + Compositor().BeginFrame(); + Compositor().BeginFrame(); + + EXPECT_EQ(2u, GetDocument().Markers().Markers().size()); + EXPECT_TRUE(HasTextFragmentHandler(GetDocument().GetFrame())); + + mojo::Remote<mojom::blink::TextFragmentReceiver> remote; + EXPECT_FALSE(remote.is_bound()); + GetDocument().GetFrame()->BindTextFragmentReceiver( + remote.BindNewPipeAndPassReceiver()); + + EXPECT_TRUE(HasTextFragmentHandler(GetDocument().GetFrame())); + EXPECT_TRUE(remote.is_bound()); + remote.get()->RemoveFragments(); + + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(0u, GetDocument().Markers().Markers().size()); + + // Ensure the fragment is uninstalled + EXPECT_FALSE(GetDocument().View()->GetFragmentAnchor()); +} + +TEST_P(TextFragmentHandlerTest, + ShouldCreateTextFragmentHandlerAndRequestSelector) { + SimRequest request("https://example.com/test.html", "text/html"); + LoadURL("https://example.com/test.html"); + request.Complete(R"HTML( + <!DOCTYPE html> + <div>Test page</div> + <p id='first'>First paragraph text that is longer than 20 chars</p> + <p id='second'>Second paragraph text</p> + )HTML"); + + Node* first_paragraph = GetDocument().getElementById("first")->firstChild(); + const auto& selected_start = Position(first_paragraph, 0); + const auto& selected_end = Position(first_paragraph, 28); + ASSERT_EQ("First paragraph text that is", + PlainText(EphemeralRange(selected_start, selected_end))); + + SetSelection(selected_start, selected_end); + + mojo::Remote<mojom::blink::TextFragmentReceiver> remote; + EXPECT_TRUE(HasTextFragmentHandler(GetDocument().GetFrame())); + EXPECT_FALSE(remote.is_bound()); + + GetTextFragmentHandler().StartPreemptiveGenerationIfNeeded(); + GetDocument().GetFrame()->BindTextFragmentReceiver( + remote.BindNewPipeAndPassReceiver()); + + EXPECT_TRUE(HasTextFragmentHandler(GetDocument().GetFrame())); + EXPECT_TRUE(remote.is_bound()); + + bool callback_called = false; + String selector; + auto lambda = [](bool& callback_called, String& selector, + const String& generated_selector) { + selector = generated_selector; + callback_called = true; + }; + auto callback = + WTF::Bind(lambda, std::ref(callback_called), std::ref(selector)); + remote->RequestSelector(std::move(callback)); + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(callback_called); + + EXPECT_EQ(selector, "First%20paragraph%20text%20that%20is"); + VerifyPreemptiveGenerationMetrics(true); +} + +struct PreemptiveLinkGenerationTestPassToString { + std::string operator()(const testing::TestParamInfo<bool> b) const { + return b.param ? "Preemptive" : "NonPreemptive"; + } +}; + +INSTANTIATE_TEST_SUITE_P(All, + TextFragmentHandlerTest, + ::testing::Bool(), + PreemptiveLinkGenerationTestPassToString()); + } // namespace } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector.cc b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector.cc index 787fb1bc3b0..4c6d208aec2 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector.cc @@ -16,8 +16,8 @@ namespace { // return an empty string to indicate no prefix/suffix was specified or it // was malformed and should be ignored. String ExtractPrefix(String* target_text) { - size_t comma_pos = target_text->find(','); - size_t hyphen_pos = target_text->find('-'); + wtf_size_t comma_pos = target_text->find(','); + wtf_size_t hyphen_pos = target_text->find('-'); if (hyphen_pos != kNotFound && hyphen_pos == comma_pos - 1) { String prefix = target_text->Substring(0, hyphen_pos); @@ -28,8 +28,8 @@ String ExtractPrefix(String* target_text) { } String ExtractSuffix(String* target_text) { - size_t last_comma_pos = target_text->ReverseFind(','); - size_t last_hyphen_pos = target_text->ReverseFind('-'); + wtf_size_t last_comma_pos = target_text->ReverseFind(','); + wtf_size_t last_hyphen_pos = target_text->ReverseFind('-'); if (last_hyphen_pos != kNotFound && last_hyphen_pos == last_comma_pos + 1) { String suffix = target_text->Substring(last_hyphen_pos + 1); @@ -49,10 +49,10 @@ TextFragmentSelector TextFragmentSelector::Create(String target_text) { String prefix = ExtractPrefix(&target_text); String suffix = ExtractSuffix(&target_text); - size_t comma_pos = target_text.find(','); + wtf_size_t comma_pos = target_text.find(','); // If there are more commas, this is an invalid text fragment selector. - size_t next_comma_pos = target_text.find(',', comma_pos + 1); + wtf_size_t next_comma_pos = target_text.find(',', comma_pos + 1); if (next_comma_pos != kNotFound) return TextFragmentSelector(kInvalid); diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.cc b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.cc index 3c38e5fbcc2..3aa90bfd6fa 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.cc @@ -4,10 +4,9 @@ #include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h" +#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_macros.h" -#include "base/strings/strcat.h" #include "base/time/default_tick_clock.h" -#include "components/shared_highlighting/core/common/disabled_sites.h" #include "components/shared_highlighting/core/common/shared_highlighting_features.h" #include "components/shared_highlighting/core/common/shared_highlighting_metrics.h" #include "third_party/blink/public/common/browser_interface_broker_proxy.h" @@ -15,9 +14,11 @@ #include "third_party/blink/renderer/core/editing/ephemeral_range.h" #include "third_party/blink/renderer/core/editing/finder/find_buffer.h" #include "third_party/blink/renderer/core/editing/iterators/text_iterator.h" +#include "third_party/blink/renderer/core/editing/range_in_flat_tree.h" #include "third_party/blink/renderer/core/frame/local_frame.h" #include "third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.h" #include "third_party/blink/renderer/core/page/scrolling/text_fragment_finder.h" +#include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector.h" #include "third_party/blink/renderer/platform/text/text_boundaries.h" using LinkGenerationError = shared_highlighting::LinkGenerationError; @@ -83,6 +84,23 @@ Node* NextNonEmptyVisibleTextNode(Node* start_node) { if (!start_node) return nullptr; + // Filter out nodes without layout object. + if (base::FeatureList::IsEnabled( + shared_highlighting::kSharedHighlightingLayoutObjectFix)) { + for (Node* node = start_node; node; node = Direction::Next(*node)) { + Node* next_node = Direction::GetVisibleTextNode(*node); + if (!next_node) + return nullptr; + if (next_node->GetLayoutObject() && + !PlainText(EphemeralRange::RangeOfContents(*next_node)) + .StripWhiteSpace() + .IsEmpty()) + return next_node; + node = next_node; + } + return nullptr; + } + // Move forward/backward until non empty visible text node is found. for (Node* node = start_node; node; node = Direction::Next(*node)) { Node* next_node = Direction::GetVisibleTextNode(*node); @@ -185,31 +203,87 @@ constexpr int kMinWordCount_ = 3; TextFragmentSelectorGenerator::TextFragmentSelectorGenerator( LocalFrame* main_frame) - : selection_frame_(main_frame) { - // Scroll-to-text doesn't support iframes. - DCHECK(main_frame->IsMainFrame()); + : frame_(main_frame) { + // Links are generally generated in the main frame except when the main frame + // delegates the text fragment to an iframe (e.g AMP Viewer pages). + if (!base::FeatureList::IsEnabled( + shared_highlighting::kSharedHighlightingAmp)) { + DCHECK(main_frame->IsMainFrame()); + } } -void TextFragmentSelectorGenerator::UpdateSelection( - const EphemeralRangeInFlatTree& selection_range) { - selection_range_ = MakeGarbageCollected<Range>( - selection_range.GetDocument(), - ToPositionInDOMTree(selection_range.StartPosition()), - ToPositionInDOMTree(selection_range.EndPosition())); - if (base::FeatureList::IsEnabled( - shared_highlighting::kPreemptiveLinkToTextGeneration) && - shared_highlighting::ShouldOfferLinkToText( - selection_frame_->GetDocument()->Url())) { - Reset(); - GenerateSelector(); +void TextFragmentSelectorGenerator::Generate(const RangeInFlatTree& range, + GenerateCallback callback) { + DCHECK(callback); + Reset(); + range_ = MakeGarbageCollected<RangeInFlatTree>(range.StartPosition(), + range.EndPosition()); + pending_generate_selector_callback_ = std::move(callback); + + StartGeneration(); +} + +void TextFragmentSelectorGenerator::Reset() { + if (finder_) + finder_->Cancel(); + + generation_start_time_ = base::DefaultTickClock::GetInstance()->NowTicks(); + state_ = kNotStarted; + error_.reset(); + step_ = kExact; + max_available_prefix_ = ""; + max_available_suffix_ = ""; + max_available_range_start_ = ""; + max_available_range_end_ = ""; + num_context_words_ = 0; + num_range_words_ = 0; + iteration_ = 0; + selector_ = nullptr; + range_ = nullptr; + pending_generate_selector_callback_.Reset(); +} + +void TextFragmentSelectorGenerator::Trace(Visitor* visitor) const { + visitor->Trace(frame_); + visitor->Trace(range_); + visitor->Trace(finder_); +} + +void TextFragmentSelectorGenerator::RecordSelectorStateUma() const { + base::UmaHistogramEnumeration("SharedHighlights.LinkGenerated.StateAtRequest", + state_); +} + +void TextFragmentSelectorGenerator::DidFindMatch( + const EphemeralRangeInFlatTree& match, + const TextFragmentAnchorMetrics::Match match_metrics, + bool is_unique) { + if (is_unique && + PlainText(match).StripWhiteSpace().length() == + PlainText(range_->ToEphemeralRange()).StripWhiteSpace().length()) { + state_ = kSuccess; + ResolveSelectorState(); + } else { + state_ = kNeedsNewCandidate; + + // If already tried exact selector then should continue by adding context. + if (step_ == kExact) + step_ = kContext; + GenerateSelectorCandidate(); } } +void TextFragmentSelectorGenerator::NoMatchFound() { + state_ = kFailure; + error_ = LinkGenerationError::kIncorrectSelector; + ResolveSelectorState(); +} + void TextFragmentSelectorGenerator::AdjustSelection() { - if (!selection_range_) + if (!range_) return; - EphemeralRangeInFlatTree ephemeral_range(selection_range_); + EphemeralRangeInFlatTree ephemeral_range = range_->ToEphemeralRange(); Node* start_container = ephemeral_range.StartPosition().ComputeContainerNode(); Node* end_container = ephemeral_range.EndPosition().ComputeContainerNode(); @@ -274,55 +348,45 @@ void TextFragmentSelectorGenerator::AdjustSelection() { } if (corrected_start != start_container || - corrected_start_offset != + static_cast<int>(corrected_start_offset) != ephemeral_range.StartPosition().ComputeOffsetInContainerNode() || corrected_end != end_container || - corrected_end_offset != + static_cast<int>(corrected_end_offset) != ephemeral_range.EndPosition().ComputeOffsetInContainerNode()) { - selection_range_ = MakeGarbageCollected<Range>( - selection_range_->OwnerDocument(), - Position(corrected_start, corrected_start_offset), - Position(corrected_end, corrected_end_offset)); - } -} - -void TextFragmentSelectorGenerator::Cancel() { - Reset(); -} - -void TextFragmentSelectorGenerator::RequestSelector( - RequestSelectorCallback callback) { - DCHECK(callback); - if (!base::FeatureList::IsEnabled( - shared_highlighting::kPreemptiveLinkToTextGeneration)) { - Reset(); - pending_generate_selector_callback_ = std::move(callback); - GenerateSelector(); - } else { - pending_generate_selector_callback_ = std::move(callback); - DCHECK_NE(state_, kNotStarted); - if (state_ == kFailure || state_ == kSuccess) { - selector_requested_before_ready_ = false; - if (state_ == kFailure) { - NotifyClientSelectorReady( - TextFragmentSelector(TextFragmentSelector::SelectorType::kInvalid)); - } else { - NotifyClientSelectorReady(*selector_); - } + PositionInFlatTree start(corrected_start, corrected_start_offset); + PositionInFlatTree end(corrected_end, corrected_end_offset); + + // TODO(bokan): This can sometimes occur from a selection. Avoid crashing + // from this case but this can come from a seemingly correct range so we + // should investigate the source of the bug. https://crbug.com/1216357 + if (start >= end) { + range_ = nullptr; return; } - selector_requested_before_ready_ = true; + + range_ = MakeGarbageCollected<RangeInFlatTree>(start, end); } } -void TextFragmentSelectorGenerator::GenerateSelector() { - DCHECK(selection_range_); +void TextFragmentSelectorGenerator::StartGeneration() { + DCHECK(range_); - selection_range_->OwnerDocument().UpdateStyleAndLayout( + range_->StartPosition().GetDocument()->UpdateStyleAndLayout( DocumentUpdateReason::kFindInPage); - // Shouldn't continue is selection is empty. - EphemeralRangeInFlatTree ephemeral_range(selection_range_); + // TODO(bokan): This can sometimes occur from a selection. Avoid crashing from + // this case but this can come from a seemingly correct range so we should + // investigate the source of the bug. + // https://crbug.com/1216357 + EphemeralRangeInFlatTree ephemeral_range = range_->ToEphemeralRange(); + if (ephemeral_range.StartPosition() >= ephemeral_range.EndPosition()) { + state_ = kFailure; + error_ = LinkGenerationError::kEmptySelection; + ResolveSelectorState(); + return; + } + + // Shouldn't continue if selection is empty. String selected_text = PlainText(ephemeral_range).StripWhiteSpace(); if (selected_text.IsEmpty()) { state_ = kFailure; @@ -332,9 +396,20 @@ void TextFragmentSelectorGenerator::GenerateSelector() { } AdjustSelection(); - UMA_HISTOGRAM_COUNTS_1000( - "SharedHighlights.LinkGenerated.SelectionLength", - PlainText(EphemeralRange(selection_range_)).length()); + + // TODO(bokan): This can sometimes occur from a selection. Avoid crashing from + // this case but this can come from a seemingly correct range so we should + // investigate the source of the bug. + // https://crbug.com/1216357 + if (!range_) { + state_ = kFailure; + error_ = LinkGenerationError::kEmptySelection; + ResolveSelectorState(); + return; + } + + UMA_HISTOGRAM_COUNTS_1000("SharedHighlights.LinkGenerated.SelectionLength", + PlainText(range_->ToEphemeralRange()).length()); state_ = kNeedsNewCandidate; GenerateSelectorCandidate(); } @@ -377,79 +452,87 @@ void TextFragmentSelectorGenerator::RunTextFinder() { iteration_++; // |FindMatch| will call |DidFindMatch| indicating if the match was unique. finder_ = MakeGarbageCollected<TextFragmentFinder>( - *this, *selector_, selection_frame_->GetDocument(), + *this, *selector_, frame_->GetDocument(), TextFragmentFinder::FindBufferRunnerType::kAsynchronous); finder_->FindMatch(); } -void TextFragmentSelectorGenerator::DidFindMatch( - const EphemeralRangeInFlatTree& match, - const TextFragmentAnchorMetrics::Match match_metrics, - bool is_unique) { - if (is_unique && PlainText(match).StripWhiteSpace().length() == - PlainText(EphemeralRangeInFlatTree(selection_range_)) - .StripWhiteSpace() - .length()) { - state_ = kSuccess; - ResolveSelectorState(); - } else { - state_ = kNeedsNewCandidate; - - // If already tried exact selector then should continue by adding context. - if (step_ == kExact) - step_ = kContext; - GenerateSelectorCandidate(); - } -} - -void TextFragmentSelectorGenerator::NoMatchFound() { - state_ = kFailure; - error_ = LinkGenerationError::kIncorrectSelector; - ResolveSelectorState(); -} +String TextFragmentSelectorGenerator::GetPreviousTextBlock( + const Position& prefix_end_position) { + Node* prefix_end = prefix_end_position.ComputeContainerNode(); + unsigned prefix_end_offset = + prefix_end_position.ComputeOffsetInContainerNode(); -void TextFragmentSelectorGenerator::OnSelectorReady( - const TextFragmentSelector& selector) { - // Check that frame is not deattched and generator is still valid. - DCHECK(selection_frame_); + // If given position point to the first visible text in its containiner node, + // use the preceding visible node for the suffix. + if (IsFirstVisiblePosition(prefix_end, prefix_end_offset)) { + prefix_end = BackwardNonEmptyVisibleTextNode( + FlatTreeTraversal::Previous(*prefix_end)); - RecordAllMetrics(selector); - if (pending_generate_selector_callback_) { - NotifyClientSelectorReady(selector); + if (!prefix_end) + return ""; + prefix_end_offset = prefix_end->textContent().length(); } -} -void TextFragmentSelectorGenerator::NotifyClientSelectorReady( - const TextFragmentSelector& selector) { - DCHECK(pending_generate_selector_callback_); - if (base::FeatureList::IsEnabled( - shared_highlighting::kPreemptiveLinkToTextGeneration)) - RecordPreemptiveGenerationMetrics(selector); - std::move(pending_generate_selector_callback_).Run(selector.ToString()); + // The furthest node within same block without crossing block boundaries would + // be the prefix start. + Node* prefix_start = LastVisibleTextNodeWithinBlock(prefix_end); + if (!prefix_start) + return ""; + + auto range_start = Position(prefix_start, 0); + auto range_end = Position(prefix_end, prefix_end_offset); + // TODO(gayane): Find test case when this happens, seems related to shadow + // root. See crbug.com/1220830 + if (range_start >= range_end) + return ""; + return PlainText(EphemeralRange(range_start, range_end)).StripWhiteSpace(); } -void TextFragmentSelectorGenerator::ClearSelection() { - if (selection_range_) { - selection_range_->Dispose(); - selection_range_ = nullptr; +String TextFragmentSelectorGenerator::GetNextTextBlock( + const Position& suffix_start_position) { + Node* suffix_start = suffix_start_position.ComputeContainerNode(); + unsigned suffix_start_offset = + suffix_start_position.ComputeOffsetInContainerNode(); + // If given position point to the last visible text in its containiner node, + // use the following visible node for the suffix. + if (IsLastVisiblePosition(suffix_start, suffix_start_offset)) { + suffix_start = FirstNonEmptyVisibleTextNode( + FlatTreeTraversal::NextSkippingChildren(*suffix_start)); + suffix_start_offset = 0; } -} + if (!suffix_start) + return ""; -void TextFragmentSelectorGenerator::Detach() { - Reset(); - selection_frame_ = nullptr; -} + // The furthest node within same block without crossing block boundaries would + // be the suffix end. + Node* suffix_end = FirstVisibleTextNodeWithinBlock(suffix_start); + if (!suffix_end) + return ""; -void TextFragmentSelectorGenerator::Trace(Visitor* visitor) const { - visitor->Trace(selection_frame_); - visitor->Trace(selection_range_); - visitor->Trace(finder_); + auto range_start = Position(suffix_start, suffix_start_offset); + auto range_end = Position(suffix_end, suffix_end->textContent().length()); + + // TODO(gayane): Find test case when this happens, seems related to shadow + // root. See crbug.com/1220830 + if (range_start >= range_end) + return ""; + return PlainText(EphemeralRange(range_start, range_end)).StripWhiteSpace(); } void TextFragmentSelectorGenerator::GenerateExactSelector() { DCHECK_EQ(kExact, step_); DCHECK_EQ(kNeedsNewCandidate, state_); - EphemeralRangeInFlatTree ephemeral_range(selection_range_); + EphemeralRangeInFlatTree ephemeral_range = range_->ToEphemeralRange(); + + // TODO(bokan): Another case where the range appears to not have valid nodes. + // Not sure how this happens. https://crbug.com/1216773. + if (!ephemeral_range.StartPosition().ComputeContainerNode() || + !ephemeral_range.EndPosition().ComputeContainerNode()) { + state_ = kFailure; + error_ = LinkGenerationError::kEmptySelection; + return; + } // If not in same block, should use ranges. if (!TextFragmentFinder::IsInSameUninterruptedBlock( @@ -488,7 +571,7 @@ void TextFragmentSelectorGenerator::ExtendRangeSelector() { // Initialize range start/end and word min count, if needed. if (max_available_range_start_.IsEmpty() && max_available_range_end_.IsEmpty()) { - EphemeralRangeInFlatTree ephemeral_range(selection_range_); + EphemeralRangeInFlatTree ephemeral_range = range_->ToEphemeralRange(); // If selection starts and ends in the same block, then split selected text // roughly in the middle. @@ -498,8 +581,8 @@ void TextFragmentSelectorGenerator::ExtendRangeSelector() { selection_text.Ensure16Bit(); int selection_length = selection_text.length(); int mid_point = - FindNextWordForward(selection_text.Characters16(), selection_length, - selection_length / 2); + FindNextWordForward(selection_text.Characters16(), + selection_text.length(), selection_length / 2); max_available_range_start_ = selection_text.Left(mid_point); // If from middle till end of selection there is no word break, then we @@ -516,9 +599,9 @@ void TextFragmentSelectorGenerator::ExtendRangeSelector() { // If not the same node, then we use first and last block of the selection // range. max_available_range_start_ = - GetNextTextBlock(selection_range_->StartPosition()); + GetNextTextBlock(ToPositionInDOMTree(range_->StartPosition())); max_available_range_end_ = - GetPreviousTextBlock(selection_range_->EndPosition()); + GetPreviousTextBlock(ToPositionInDOMTree(range_->EndPosition())); } // Use at least 3 words from both sides for more robust link to text. @@ -556,8 +639,9 @@ void TextFragmentSelectorGenerator::ExtendContext() { // Try initiating properties necessary for calculating prefix and suffix. if (max_available_prefix_.IsEmpty() && max_available_suffix_.IsEmpty()) { max_available_prefix_ = - GetPreviousTextBlock(selection_range_->StartPosition()); - max_available_suffix_ = GetNextTextBlock(selection_range_->EndPosition()); + GetPreviousTextBlock(ToPositionInDOMTree(range_->StartPosition())); + max_available_suffix_ = + GetNextTextBlock(ToPositionInDOMTree(range_->EndPosition())); // Use at least 3 words from both sides for more robust link to text. num_context_words_ = kMinWordCount_; @@ -585,97 +669,14 @@ void TextFragmentSelectorGenerator::ExtendContext() { state_ = kTestCandidate; } -String TextFragmentSelectorGenerator::GetPreviousTextBlock( - const Position& prefix_end_position) { - Node* prefix_end = prefix_end_position.ComputeContainerNode(); - unsigned prefix_end_offset = - prefix_end_position.ComputeOffsetInContainerNode(); - - // If given position point to the first visible text in its containiner node, - // use the preceding visible node for the suffix. - if (IsFirstVisiblePosition(prefix_end, prefix_end_offset)) { - prefix_end = BackwardNonEmptyVisibleTextNode( - FlatTreeTraversal::Previous(*prefix_end)); - - if (!prefix_end) - return ""; - prefix_end_offset = prefix_end->textContent().length(); - } - - // The furthest node within same block without crossing block boundaries would - // be the prefix start. - Node* prefix_start = LastVisibleTextNodeWithinBlock(prefix_end); - if (!prefix_start) - return ""; - - auto range_start = Position(prefix_start, 0); - auto range_end = Position(prefix_end, prefix_end_offset); - // TODO(gayane): Find test case when this happens, seems related to shadow - // root. See crbug.com/1220830 - if (range_start >= range_end) - return ""; - return PlainText(EphemeralRange(range_start, range_end)).StripWhiteSpace(); -} - -String TextFragmentSelectorGenerator::GetNextTextBlock( - const Position& suffix_start_position) { - Node* suffix_start = suffix_start_position.ComputeContainerNode(); - unsigned suffix_start_offset = - suffix_start_position.ComputeOffsetInContainerNode(); - // If given position point to the last visible text in its containiner node, - // use the following visible node for the suffix. - if (IsLastVisiblePosition(suffix_start, suffix_start_offset)) { - suffix_start = FirstNonEmptyVisibleTextNode( - FlatTreeTraversal::NextSkippingChildren(*suffix_start)); - suffix_start_offset = 0; - } - if (!suffix_start) - return ""; - - // The furthest node within same block without crossing block boundaries would - // be the suffix end. - Node* suffix_end = FirstVisibleTextNodeWithinBlock(suffix_start); - if (!suffix_end) - return ""; - - auto range_start = Position(suffix_start, suffix_start_offset); - auto range_end = Position(suffix_end, suffix_end->textContent().length()); - - // TODO(gayane): Find test case when this happens, seems related to shadow - // root. See crbug.com/1220830 - if (range_start >= range_end) - return ""; - return PlainText(EphemeralRange(range_start, range_end)).StripWhiteSpace(); -} - -void TextFragmentSelectorGenerator::Reset() { - if (finder_) - finder_->Cancel(); - - generation_start_time_ = base::DefaultTickClock::GetInstance()->NowTicks(); - state_ = kNotStarted; - error_.reset(); - step_ = kExact; - max_available_prefix_ = ""; - max_available_suffix_ = ""; - max_available_range_start_ = ""; - max_available_range_end_ = ""; - num_context_words_ = 0; - num_range_words_ = 0; - iteration_ = 0; - selector_ = nullptr; - selector_requested_before_ready_.reset(); - pending_generate_selector_callback_.Reset(); -} - void TextFragmentSelectorGenerator::RecordAllMetrics( const TextFragmentSelector& selector) { UMA_HISTOGRAM_BOOLEAN( "SharedHighlights.LinkGenerated", selector.Type() != TextFragmentSelector::SelectorType::kInvalid); - ukm::UkmRecorder* recorder = selection_frame_->GetDocument()->UkmRecorder(); - ukm::SourceId source_id = selection_frame_->GetDocument()->UkmSourceID(); + ukm::UkmRecorder* recorder = frame_->GetDocument()->UkmRecorder(); + ukm::SourceId source_id = frame_->GetDocument()->UkmSourceID(); if (selector.Type() != TextFragmentSelector::SelectorType::kInvalid) { UMA_HISTOGRAM_COUNTS_1000("SharedHighlights.LinkGenerated.ParamLength", @@ -707,27 +708,21 @@ void TextFragmentSelectorGenerator::RecordAllMetrics( } } -void TextFragmentSelectorGenerator::RecordPreemptiveGenerationMetrics( +void TextFragmentSelectorGenerator::OnSelectorReady( const TextFragmentSelector& selector) { - DCHECK(selector_requested_before_ready_.has_value()); - - bool success = - selector.Type() != TextFragmentSelector::SelectorType::kInvalid; + // Check that frame is not deattched and generator is still valid. + DCHECK(frame_); - std::string uma_prefix = "SharedHighlights.LinkGenerated"; - if (selector_requested_before_ready_.value()) { - uma_prefix = base::StrCat({uma_prefix, ".RequestedBeforeReady"}); - } else { - uma_prefix = base::StrCat({uma_prefix, ".RequestedAfterReady"}); + RecordAllMetrics(selector); + if (pending_generate_selector_callback_) { + NotifyClientSelectorReady(selector); } - base::UmaHistogramBoolean(uma_prefix, success); +} - if (!success) { - LinkGenerationError error = - error_.has_value() ? error_.value() : LinkGenerationError::kUnknown; - base::UmaHistogramEnumeration( - "SharedHighlights.LinkGenerated.Error.Requested", error); - } +void TextFragmentSelectorGenerator::NotifyClientSelectorReady( + const TextFragmentSelector& selector) { + DCHECK(pending_generate_selector_callback_); + std::move(pending_generate_selector_callback_).Run(selector); } } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h index b20e8a90dfb..2d54bc5e1b1 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h @@ -10,52 +10,45 @@ #include "third_party/blink/public/mojom/link_to_text/link_to_text.mojom-blink.h" #include "third_party/blink/renderer/core/editing/forward.h" #include "third_party/blink/renderer/core/page/scrolling/text_fragment_finder.h" -#include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector.h" #include "third_party/blink/renderer/platform/mojo/heap_mojo_receiver.h" namespace blink { -using RequestSelectorCallback = base::OnceCallback<void(const WTF::String&)>; - class LocalFrame; +class RangeInFlatTree; +class TextFragmentSelector; -// TextFragmentSelectorGenerator is responsible for generating text fragment -// selectors for the user selected text according to spec in +// TextFragmentSelectorGenerator is used to generate a TextFragmentSelector, +// given a range of DOM in a document. The TextFragmentSelector provides the +// necessary portions of a text fragment URL such that it scrolls to the given +// range when navigated. For more details, see: // 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. // -// TextFragmentSelectorGenerator generates candidate selectors and tries it -// against the page content to ensure the correct and unique match. Repeats the -// process adding context/range to the selector as necessary until the correct -// match is uniquely identified or no new context/range can be added. +// TextFragmentSelectorGenerator works by starting with a candidate selector +// and repeatedly trying it against the page content to ensure the correct and +// unique match. While we don't have a unique match, we repeatedly adding +// context/range to the selector until the correct match is uniquely identified +// or no new context/range can be added. class CORE_EXPORT TextFragmentSelectorGenerator final : public GarbageCollected<TextFragmentSelectorGenerator>, public TextFragmentFinder::Client { + using GenerateCallback = + base::OnceCallback<void(const TextFragmentSelector&)>; + public: explicit TextFragmentSelectorGenerator(LocalFrame* main_frame); + TextFragmentSelectorGenerator(const TextFragmentSelectorGenerator&) = delete; + TextFragmentSelectorGenerator& operator=( + const TextFragmentSelectorGenerator&) = delete; - // Sets the frame and range of the current selection. - void UpdateSelection(const EphemeralRangeInFlatTree& selection_range); - - // Adjust the selection start/end to a valid position. That includes skipping - // non text start/end nodes and extending selection from start and end to - // contain full words. - void AdjustSelection(); - - // blink::mojom::blink::TextFragmentSelectorProducer interface - void Cancel(); + // Requests a TextFragmentSelector be generated for the selection of DOM + // specified by |range|. Will be generated asynchronously and returned by + // invoking |callback|. + void Generate(const RangeInFlatTree& range, GenerateCallback callback); - // Requests selector for current selection. - void RequestSelector(RequestSelectorCallback callback); - - // TextFragmentFinder::Client interface - void DidFindMatch(const EphemeralRangeInFlatTree& match, - const TextFragmentAnchorMetrics::Match match_metrics, - bool is_unique) override; - - void NoMatchFound() override; + // Resets generator state to initial values and cancels any existing async + // tasks. + void Reset(); // Wrappers for tests. String GetPreviousTextBlockForTesting(const Position& position) { @@ -64,18 +57,23 @@ class CORE_EXPORT TextFragmentSelectorGenerator final String GetNextTextBlockForTesting(const Position& position) { return GetNextTextBlock(position); } - void SetCallbackForTesting(RequestSelectorCallback callback) { + void SetCallbackForTesting(GenerateCallback callback) { pending_generate_selector_callback_ = std::move(callback); } - // Releases members if necessary. - void ClearSelection(); + void Trace(Visitor*) const; - void Detach(); + // Temporary diagnostic metric recorded to help explain discrepancies in + // other metrics. + void RecordSelectorStateUma() const; - void Trace(Visitor*) const; + LocalFrame* GetFrame() { return frame_; } - LocalFrame* GetFrame() { return selection_frame_; } + // If generation fails, returns the reason that generation failed. If + // generation hasn't finished, or was successful, returns an empty optional. + absl::optional<shared_highlighting::LinkGenerationError> GetError() { + return error_; + } private: // Used for determining the next step of selector generation. @@ -98,11 +96,24 @@ class CORE_EXPORT TextFragmentSelectorGenerator final kFailure, // Selector is found. No further attempts are necessary. - kSuccess + kSuccess, + + kMaxValue = kSuccess }; + // TextFragmentFinder::Client interface + void DidFindMatch(const EphemeralRangeInFlatTree& match, + const TextFragmentAnchorMetrics::Match match_metrics, + bool is_unique) override; + void NoMatchFound() override; + + // Adjust the selection start/end to a valid position. That includes skipping + // non text start/end nodes and extending selection from start and end to + // contain full words. + void AdjustSelection(); + // Generates selector for current selection. - void GenerateSelector(); + void StartGeneration(); void GenerateSelectorCandidate(); @@ -121,30 +132,26 @@ class CORE_EXPORT TextFragmentSelectorGenerator final void ExtendRangeSelector(); void ExtendContext(); - void Reset(); - void RecordAllMetrics(const TextFragmentSelector& selector); - void RecordPreemptiveGenerationMetrics(const TextFragmentSelector& selector); // Called when selector generation is complete. void OnSelectorReady(const TextFragmentSelector& selector); - // Called to notify clients of the result of |GenerateSelector|. + // Called to notify clients of the result of |Generate|. void NotifyClientSelectorReady(const TextFragmentSelector& selector); - Member<LocalFrame> selection_frame_; - Member<Range> selection_range_; + Member<LocalFrame> frame_; + + // This is the Range for which we're generating a selector. + Member<RangeInFlatTree> range_; + std::unique_ptr<TextFragmentSelector> selector_; - RequestSelectorCallback pending_generate_selector_callback_; + GenerateCallback pending_generate_selector_callback_; GenerationStep step_ = kExact; SelectorState state_ = kNeedsNewCandidate; - // Used when preemptive link generation is enabled to report - // whether |RequestSelector| was called before or after selector was ready. - absl::optional<bool> selector_requested_before_ready_; - absl::optional<shared_highlighting::LinkGenerationError> error_; // Fields used for keeping track of context. @@ -166,8 +173,6 @@ class CORE_EXPORT TextFragmentSelectorGenerator final base::TimeTicks generation_start_time_; Member<TextFragmentFinder> finder_; - - DISALLOW_COPY_AND_ASSIGN(TextFragmentSelectorGenerator); }; } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator_test.cc b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator_test.cc index 1168cb9d8e7..71e09988cd8 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator_test.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator_test.cc @@ -20,6 +20,7 @@ #include "third_party/blink/renderer/core/editing/ephemeral_range.h" #include "third_party/blink/renderer/core/editing/iterators/text_iterator.h" #include "third_party/blink/renderer/core/frame/local_frame.h" +#include "third_party/blink/renderer/core/page/scrolling/text_fragment_handler.h" #include "third_party/blink/renderer/core/testing/scoped_fake_ukm_recorder.h" #include "third_party/blink/renderer/core/testing/sim/sim_request.h" #include "third_party/blink/renderer/core/testing/sim/sim_test.h" @@ -33,21 +34,11 @@ const char kSuccessUkmMetric[] = "Success"; const char kErrorUkmMetric[] = "Error"; } // namespace -class TextFragmentSelectorGeneratorTest - : public SimTest, - public ::testing::WithParamInterface<bool> { +class TextFragmentSelectorGeneratorTest : public SimTest { public: void SetUp() override { SimTest::SetUp(); WebView().MainFrameViewWidget()->Resize(gfx::Size(800, 600)); - preemptive_generation_enabled_ = GetParam(); - if (preemptive_generation_enabled_) { - feature_list_.InitAndEnableFeature( - shared_highlighting::kPreemptiveLinkToTextGeneration); - } else { - feature_list_.InitAndDisableFeature( - shared_highlighting::kPreemptiveLinkToTextGeneration); - } } void VerifySelector(Position selected_start, @@ -95,67 +86,32 @@ class TextFragmentSelectorGeneratorTest String GenerateSelector(Position selected_start, Position selected_end) { generate_call_count_++; - GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->UpdateSelection(ToEphemeralRangeInFlatTree( - EphemeralRange(selected_start, selected_end))); bool callback_called = false; String selector; auto lambda = [](bool& callback_called, String& selector, - const String& generated_selector) { - selector = generated_selector; + const TextFragmentSelector& generated_selector) { + selector = generated_selector.ToString(); callback_called = true; }; auto callback = WTF::Bind(lambda, std::ref(callback_called), std::ref(selector)); - GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->RequestSelector(std::move(callback)); + GetTextFragmentSelectorGenerator()->Generate( + *MakeGarbageCollected<RangeInFlatTree>( + ToPositionInFlatTree(selected_start), + ToPositionInFlatTree(selected_end)), + std::move(callback)); base::RunLoop().RunUntilIdle(); EXPECT_TRUE(callback_called); return selector; } - void VerifyPreemptiveGenerationMetrics(bool success) { - if (!preemptive_generation_enabled_) { - histogram_tester_.ExpectTotalCount( - "SharedHighlights.LinkGenerated.Error.Requested", 0); - histogram_tester_.ExpectTotalCount( - "SharedHighlights.LinkGenerated.RequestedAfterReady", 0); - histogram_tester_.ExpectTotalCount( - "SharedHighlights.LinkGenerated.RequestedBeforeReady", 0); - } else { - EXPECT_EQ( - 1u, histogram_tester_ - .GetAllSamples( - "SharedHighlights.LinkGenerated.RequestedAfterReady") - .size() + - histogram_tester_ - .GetAllSamples( - "SharedHighlights.LinkGenerated.RequestedBeforeReady") - .size()); - - if (!success) { - histogram_tester_.ExpectTotalCount( - "SharedHighlights.LinkGenerated.Error.Requested", 1); - } else { - histogram_tester_.ExpectTotalCount( - "SharedHighlights.LinkGenerated.Error.Requested", 0); - } - } - - // Check async task metrics. - EXPECT_LT(0u, histogram_tester_ - .GetAllSamples("SharedHighlights.AsyncTask.Iterations") - .size()); - EXPECT_LT(0u, - histogram_tester_ - .GetAllSamples("SharedHighlights.AsyncTask.SearchDuration") - .size()); + TextFragmentSelectorGenerator* GetTextFragmentSelectorGenerator() { + return GetDocument() + .GetFrame() + ->GetTextFragmentHandler() + ->GetTextFragmentSelectorGenerator(); } protected: @@ -166,91 +122,10 @@ class TextFragmentSelectorGeneratorTest base::HistogramTester histogram_tester_; ScopedFakeUkmRecorder scoped_ukm_recorder_; int generate_call_count_ = 0; - bool preemptive_generation_enabled_; - base::test::ScopedFeatureList feature_list_; }; -// Checks that the selector is preemptively generated. -TEST_P(TextFragmentSelectorGeneratorTest, CheckPreemptiveGeneration) { - if (!preemptive_generation_enabled_) - return; - - SimRequest request("https://example.com/test.html", "text/html"); - LoadURL("https://example.com/test.html"); - request.Complete(R"HTML( - <!DOCTYPE html> - <p id='first'>First paragraph</p> - )HTML"); - - Node* first_paragraph = GetDocument().getElementById("first")->firstChild(); - const auto& selected_start = Position(first_paragraph, 0); - const auto& selected_end = Position(first_paragraph, 5); - ASSERT_EQ("First", PlainText(EphemeralRange(selected_start, selected_end))); - - GetDocument().GetFrame()->GetTextFragmentSelectorGenerator()->UpdateSelection( - ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end))); - base::RunLoop().RunUntilIdle(); - - histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated", 1); - histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated.Error", 0); -} - -// When URL is blocklisted, the selector shouldn't be preemptively generated. -TEST_P(TextFragmentSelectorGeneratorTest, - CheckNoPreemptiveGenerationBlocklist) { - if (!preemptive_generation_enabled_) - return; - - SimRequest request("https://instagram.com/test.html", "text/html"); - LoadURL("https://instagram.com/test.html"); - request.Complete(R"HTML( - <!DOCTYPE html> - <p id='first'>First paragraph</p> - )HTML"); - - Node* first_paragraph = GetDocument().getElementById("first")->firstChild(); - const auto& selected_start = Position(first_paragraph, 0); - const auto& selected_end = Position(first_paragraph, 5); - ASSERT_EQ("First", PlainText(EphemeralRange(selected_start, selected_end))); - - GetDocument().GetFrame()->GetTextFragmentSelectorGenerator()->UpdateSelection( - ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end))); - base::RunLoop().RunUntilIdle(); - - histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated", 0); - histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated.Error", 0); -} - -// Check that selector is not generated for editable text. -TEST_P(TextFragmentSelectorGeneratorTest, CheckNoPreemptiveGenerationEditable) { - if (!preemptive_generation_enabled_) - return; - - SimRequest request("https://instagram.com/test.html", "text/html"); - LoadURL("https://instagram.com/test.html"); - request.Complete(R"HTML( - <!DOCTYPE html> - <input type="text" id="input" value="default text in input"> - )HTML"); - - Node* input_text = - FlatTreeTraversal::Next(*GetDocument().getElementById("input")) - ->firstChild(); - const auto& selected_start = Position(input_text, 0); - const auto& selected_end = Position(input_text, 12); - ASSERT_EQ("default text", - PlainText(EphemeralRange(selected_start, selected_end))); - - GetDocument().GetFrame()->GetTextFragmentSelectorGenerator()->UpdateSelection( - ToEphemeralRangeInFlatTree(EphemeralRange(selected_start, selected_end))); - base::RunLoop().RunUntilIdle(); - - histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated", 0); - histogram_tester_.ExpectTotalCount("SharedHighlights.LinkGenerated.Error", 0); -} - // Basic exact selector case. -TEST_P(TextFragmentSelectorGeneratorTest, EmptySelection) { +TEST_F(TextFragmentSelectorGeneratorTest, EmptySelection) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -267,7 +142,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, EmptySelection) { } // Basic exact selector case. -TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector) { +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -287,7 +162,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector) { } // Exact selector test where selection contains nested <i> node. -TEST_P(TextFragmentSelectorGeneratorTest, ExactTextWithNestedTextNodes) { +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextWithNestedTextNodes) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -308,7 +183,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, ExactTextWithNestedTextNodes) { } // Exact selector test where selection contains multiple spaces. -TEST_P(TextFragmentSelectorGeneratorTest, ExactTextWithExtraSpace) { +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextWithExtraSpace) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -329,7 +204,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, ExactTextWithExtraSpace) { // Exact selector where selection is too short, in which case context is // required. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_TooShortNeedsContext) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -351,7 +226,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, // Exact selector with context test. Case when only one word for prefix and // suffix is enough to disambiguate the selection. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_WithOneWordContext) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -373,7 +248,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, // Exact selector with context test. Case when multiple words for prefix and // suffix is necessary to disambiguate the selection. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_MultipleWordContext) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -397,7 +272,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, // Exact selector with context test. Case when multiple words for prefix and // suffix is necessary to disambiguate the selection and prefix and suffix // contain extra space. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_MultipleWordContext_ExtraSpace) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -421,7 +296,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, // Exact selector with context test. Case when available prefix for all the // occurrences of selected text is the same. In this case suffix should be // extended until unique selector is found. -TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_SamePrefix) { +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_SamePrefix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -444,7 +319,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_SamePrefix) { // Exact selector with context test. Case when available suffix for all the // occurrences of selected text is the same. In this case prefix should be // extended until unique selector is found. -TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_SameSuffix) { +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_SameSuffix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -467,7 +342,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_SameSuffix) { // Exact selector with context test. Case when available prefix and suffix for // all the occurrences of selected text are the same. In this case generation // should be unsuccessful. -TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_SamePrefixSuffix) { +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_SamePrefixSuffix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -489,7 +364,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_SamePrefixSuffix) { // Exact selector with context test. Case when available prefix and suffix for // all the occurrences of selected text are the same for the first 10 words. In // this case generation should be unsuccessful. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_SimilarLongPreffixSuffix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -512,7 +387,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, } // Exact selector with context test. Case when no prefix is available. -TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_NoPrefix) { +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_NoPrefix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -531,7 +406,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_NoPrefix) { } // Exact selector with context test. Case when no suffix is available. -TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_NoSuffix) { +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_NoSuffix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -553,7 +428,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_NoSuffix) { // Exact selector with context test. Case when available prefix is the // preceding block. -TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_PrevNodePrefix) { +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_PrevNodePrefix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -574,7 +449,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_PrevNodePrefix) { // Exact selector with context test. Case when available prefix is the // preceding block, which is a text node. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_PrevTextNodePrefix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -597,7 +472,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, // Exact selector with context test. Case when available suffix is the next // block. -TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_NextNodeSuffix) { +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_NextNodeSuffix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -619,7 +494,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, ExactTextSelector_NextNodeSuffix) { // Exact selector with context test. Case when available suffix is the next // block, which is a text node. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, ExactTextSelector_NexttextNodeSuffix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -640,7 +515,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, "First%20paragraph%20with-,not%20unique%20snippet,-text"); } -TEST_P(TextFragmentSelectorGeneratorTest, RangeSelector) { +TEST_F(TextFragmentSelectorGeneratorTest, RangeSelector) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -662,7 +537,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, RangeSelector) { // It should be more than 300 characters selected from the same node so that // ranges are used. -TEST_P(TextFragmentSelectorGeneratorTest, RangeSelector_SameNode) { +TEST_F(TextFragmentSelectorGeneratorTest, RangeSelector_SameNode) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -691,7 +566,7 @@ text text text text text text text text text and last text", // It should be more than 300 characters selected from the same node so that // ranges are used. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, RangeSelector_SameNode_MultipleSelections) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -741,7 +616,7 @@ text text text text text text text text text text and last text", // When using all the selected text for the range is not enough for unique // match, context should be added. -TEST_P(TextFragmentSelectorGeneratorTest, RangeSelector_RangeNotUnique) { +TEST_F(TextFragmentSelectorGeneratorTest, RangeSelector_RangeNotUnique) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -763,7 +638,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, RangeSelector_RangeNotUnique) { // When using all the selected text for the range is not enough for unique // match, context should be added, but only prefxi and no suffix is available. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, RangeSelector_RangeNotUnique_NoSuffix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -786,7 +661,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, // When no range end is available it should return empty selector. // There is no range end available because there is no word break in the second // half of the selection. -TEST_P(TextFragmentSelectorGeneratorTest, RangeSelector_NoRangeEnd) { +TEST_F(TextFragmentSelectorGeneratorTest, RangeSelector_NoRangeEnd) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -812,7 +687,7 @@ text_text_text_text_text_text_text_text_text_and_last_text", } // Selection should be autocompleted to contain full words. -TEST_P(TextFragmentSelectorGeneratorTest, WordLimit) { +TEST_F(TextFragmentSelectorGeneratorTest, WordLimit) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -832,7 +707,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, WordLimit) { // Selection should be autocompleted to contain full words. The autocompletion // should work with extra spaces. -TEST_P(TextFragmentSelectorGeneratorTest, WordLimit_ExtraSpaces) { +TEST_F(TextFragmentSelectorGeneratorTest, WordLimit_ExtraSpaces) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -854,7 +729,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, WordLimit_ExtraSpaces) { // When selection starts at the end of a word, selection shouldn't be // autocompleted to contain extra words. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, WordLimit_SelectionStartsAndEndsAtWordLimit) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -874,7 +749,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, } // Check the case when selections starts with an non text node. -TEST_P(TextFragmentSelectorGeneratorTest, StartsWithImage) { +TEST_F(TextFragmentSelectorGeneratorTest, StartsWithImage) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -893,7 +768,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, StartsWithImage) { } // Check the case when selections starts with an non text node. -TEST_P(TextFragmentSelectorGeneratorTest, StartsWithBlockWithImage) { +TEST_F(TextFragmentSelectorGeneratorTest, StartsWithBlockWithImage) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -915,7 +790,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, StartsWithBlockWithImage) { // Check the case when selections starts with a node nested in "inline-block" // node. crbug.com/1151474 -TEST_P(TextFragmentSelectorGeneratorTest, StartsWithInlineBlockChild) { +TEST_F(TextFragmentSelectorGeneratorTest, StartsWithInlineBlockChild) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -951,7 +826,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, StartsWithInlineBlockChild) { } // Check the case when selections ends with an non text node. -TEST_P(TextFragmentSelectorGeneratorTest, EndswithImage) { +TEST_F(TextFragmentSelectorGeneratorTest, EndswithImage) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -971,7 +846,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, EndswithImage) { } // Check the case when selections starts at the end of the previous block. -TEST_P(TextFragmentSelectorGeneratorTest, StartIsEndofPrevBlock) { +TEST_F(TextFragmentSelectorGeneratorTest, StartIsEndofPrevBlock) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -989,7 +864,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, StartIsEndofPrevBlock) { } // Check the case when selections starts at the end of the previous block. -TEST_P(TextFragmentSelectorGeneratorTest, EndIsStartofNextBlock) { +TEST_F(TextFragmentSelectorGeneratorTest, EndIsStartofNextBlock) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1019,7 +894,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, EndIsStartofNextBlock) { // want to ensure it correctly traverses the tree back to the previous text node // and not to the <div>(sibling of second <p>). // See crbug.com/1154308 for more context. -TEST_P(TextFragmentSelectorGeneratorTest, PrevNodeIsSiblingsChild) { +TEST_F(TextFragmentSelectorGeneratorTest, PrevNodeIsSiblingsChild) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1053,7 +928,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, PrevNodeIsSiblingsChild) { // want to ensure it correctly traverses the tree back to the previous text by // correctly skipping the invisible div but not skipping the second <p>. // See crbug.com/1154308 for more context. -TEST_P(TextFragmentSelectorGeneratorTest, PrevPrevNodeIsSiblingsChild) { +TEST_F(TextFragmentSelectorGeneratorTest, PrevPrevNodeIsSiblingsChild) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); // HTML is intentionally not formatted. Adding new lines and indentation @@ -1074,7 +949,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, PrevPrevNodeIsSiblingsChild) { // Checks that for short selection that have nested block element range selector // is used. -TEST_P(TextFragmentSelectorGeneratorTest, RangeSelector_SameNode_Interrupted) { +TEST_F(TextFragmentSelectorGeneratorTest, RangeSelector_SameNode_Interrupted) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1091,7 +966,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, RangeSelector_SameNode_Interrupted) { } // Check min number of words is used for context if possible. -TEST_P(TextFragmentSelectorGeneratorTest, MultiwordContext) { +TEST_F(TextFragmentSelectorGeneratorTest, MultiwordContext) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1111,7 +986,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, MultiwordContext) { } // Check min number of words is used for range if possible. -TEST_P(TextFragmentSelectorGeneratorTest, MultiWordRangeSelector) { +TEST_F(TextFragmentSelectorGeneratorTest, MultiWordRangeSelector) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1132,7 +1007,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, MultiWordRangeSelector) { } // Checks the case when selection end position is a non text node. -TEST_P(TextFragmentSelectorGeneratorTest, SelectionEndsWithNonText) { +TEST_F(TextFragmentSelectorGeneratorTest, SelectionEndsWithNonText) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1154,7 +1029,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, SelectionEndsWithNonText) { // Checks the case when selection end position is a non text node which doesn't // have text child node. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, SelectionEndsWithNonTextWithNoTextChild) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1177,7 +1052,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, // Checks the case when selection end position is a non text node which doesn't // have text child node. -TEST_P(TextFragmentSelectorGeneratorTest, SelectionEndsWithImageDiv) { +TEST_F(TextFragmentSelectorGeneratorTest, SelectionEndsWithImageDiv) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1199,7 +1074,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, SelectionEndsWithImageDiv) { // Checks the case when selected range contains a range with same start and end. // The problematic case should have both range end and suffix. -TEST_P(TextFragmentSelectorGeneratorTest, OverlappingRange) { +TEST_F(TextFragmentSelectorGeneratorTest, OverlappingRange) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1217,7 +1092,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, OverlappingRange) { } // Checks selection across table cells. -TEST_P(TextFragmentSelectorGeneratorTest, Table) { +TEST_F(TextFragmentSelectorGeneratorTest, Table) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1246,7 +1121,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, Table) { } // Checks selection across an input element. -TEST_P(TextFragmentSelectorGeneratorTest, Input) { +TEST_F(TextFragmentSelectorGeneratorTest, Input) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1266,7 +1141,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, Input) { // Checks selection across a shadow tree. Input that has text value will create // a shadow tree, -TEST_P(TextFragmentSelectorGeneratorTest, InputSubmit) { +TEST_F(TextFragmentSelectorGeneratorTest, InputSubmit) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1286,7 +1161,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, InputSubmit) { // Checks selection right after a shadow tree will use the shadow tree for // prefix. Input with text value will create a shadow tree. -TEST_P(TextFragmentSelectorGeneratorTest, InputSubmitPrefix) { +TEST_F(TextFragmentSelectorGeneratorTest, InputSubmitPrefix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1306,7 +1181,7 @@ TEST_P(TextFragmentSelectorGeneratorTest, InputSubmitPrefix) { // Checks selection right after a shadow tree will use the shadow tree for // prefix. Input with text value will create a shadow tree. -TEST_P(TextFragmentSelectorGeneratorTest, InputSubmitOneWordPrefix) { +TEST_F(TextFragmentSelectorGeneratorTest, InputSubmitOneWordPrefix) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1324,36 +1199,8 @@ TEST_P(TextFragmentSelectorGeneratorTest, InputSubmitOneWordPrefix) { VerifySelector(start, end, "button-,paragraph,-text"); } -// TODO(crbug.com/1192047): Update the test to better reflect the real repro -// steps. Test case for crash in crbug.com/1190137. When selector is requested -// after callback is set and unused. -TEST_P(TextFragmentSelectorGeneratorTest, SecondGenerationCrash) { - SimRequest request("https://example.com/test.html", "text/html"); - LoadURL("https://example.com/test.html"); - request.Complete(R"HTML( - <!DOCTYPE html> - <p id='p'>First paragraph text</p> - )HTML"); - GetDocument().UpdateStyleAndLayoutTree(); - Node* p = GetDocument().getElementById("p"); - const auto& start = Position(p->lastChild(), 0); - const auto& end = Position(p->lastChild(), 15); - ASSERT_EQ("First paragraph", PlainText(EphemeralRange(start, end))); - - auto callback = WTF::Bind([](const String& generated_selector) {}); - GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->SetCallbackForTesting(std::move(callback)); - - // This shouldn't crash. - GetDocument().GetFrame()->GetTextFragmentSelectorGenerator()->UpdateSelection( - ToEphemeralRangeInFlatTree(EphemeralRange(start, end))); - base::RunLoop().RunUntilIdle(); -} - // Basic test case for |GetNextTextBlock|. -TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock) { +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1365,14 +1212,13 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock) { const auto& end = Position(first_paragraph, 20); ASSERT_EQ("text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("First paragraph", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + EXPECT_EQ("First paragraph", + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when available prefix contains collapsible space. -TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_ExtraSpace) { +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_ExtraSpace) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1386,15 +1232,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_ExtraSpace) { const auto& end = Position(first_paragraph, 30); ASSERT_EQ("text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("First paragraph", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + EXPECT_EQ("First paragraph", + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when available prefix complete text content of the previous // block. -TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_PrevNode) { +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_PrevNode) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1408,15 +1253,13 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_PrevNode) { ASSERT_EQ("Second", PlainText(EphemeralRange(start, end))); EXPECT_EQ("First paragraph text", - GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when there is a commented block between selection and the // available prefix. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_PrevNode_WithComment) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1434,15 +1277,13 @@ TEST_P(TextFragmentSelectorGeneratorTest, ASSERT_EQ("Second", PlainText(EphemeralRange(start, end))); EXPECT_EQ("First paragraph text", - GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when available prefix is a text node outside of selection // block. -TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_PrevTextNode) { +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_PrevTextNode) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1455,15 +1296,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_PrevTextNode) { const auto& end = Position(first_paragraph, 5); ASSERT_EQ("First", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + EXPECT_EQ("text", + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when available prefix is a parent node text content outside of // selection block. -TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_ParentNode) { +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_ParentNode) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1476,14 +1316,13 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_ParentNode) { const auto& end = Position(first_paragraph, 5); ASSERT_EQ("First", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("nested", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + EXPECT_EQ("nested", + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when available prefix contains non-block tag(e.g. <b>). -TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_NestedTextNode) { +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_NestedTextNode) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1496,14 +1335,12 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_NestedTextNode) { ASSERT_EQ("text", PlainText(EphemeralRange(start, end))); EXPECT_EQ("First bold text paragraph", - GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when available prefix is collected until nested block. -TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_NestedBlock) { +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_NestedBlock) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1515,15 +1352,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_NestedBlock) { const auto& end = Position(first_paragraph, 15); ASSERT_EQ("text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("paragraph", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + EXPECT_EQ("paragraph", + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when available prefix includes non-block element but stops at // nested block. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_NestedBlockInNestedText) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1536,14 +1372,13 @@ TEST_P(TextFragmentSelectorGeneratorTest, const auto& end = Position(first_paragraph, 15); ASSERT_EQ("text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("bold paragraph", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + EXPECT_EQ("bold paragraph", + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when available prefix includes invisible block. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_NestedInvisibleBlock) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1556,15 +1391,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, const auto& end = Position(first_paragraph, 10); ASSERT_EQ("paragraph", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("First", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + EXPECT_EQ("First", + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when previous node is used for available prefix when selection // is not at index=0 but there is only space before it. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_SpacesBeforeSelection) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1581,15 +1415,13 @@ TEST_P(TextFragmentSelectorGeneratorTest, ASSERT_EQ("Second", PlainText(EphemeralRange(start, end))); EXPECT_EQ("First paragraph text", - GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Check the case when previous node is used for available prefix when selection // is not at index=0 but there is only invisible block. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock_InvisibleBeforeSelection) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1610,16 +1442,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, ASSERT_EQ("Second", PlainText(EphemeralRange(start, end))); EXPECT_EQ("First paragraph text", - GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetPreviousTextBlockForTesting(start)); + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } // Similar test for suffix. // Basic test case for |GetNextTextBlock|. -TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock) { +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1631,14 +1461,13 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock) { const auto& end = Position(first_paragraph, 5); ASSERT_EQ("First", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("paragraph text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "paragraph text", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when available suffix contains collapsible space. -TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_ExtraSpace) { +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_ExtraSpace) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1653,15 +1482,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_ExtraSpace) { const auto& end = Position(first_paragraph, 5); ASSERT_EQ("First", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("paragraph text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "paragraph text", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when available suffix is complete text content of the next // block. -TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NextNode) { +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NextNode) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1674,15 +1502,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NextNode) { const auto& end = Position(first_paragraph, 20); ASSERT_EQ("First paragraph text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("Second paragraph text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "Second paragraph text", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when there is a commented block between selection and the // available suffix. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NextNode_WithComment) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1699,15 +1526,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, const auto& end = Position(first_paragraph, 20); ASSERT_EQ("First paragraph text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("Second paragraph text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "Second paragraph text", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when available suffix is a text node outside of selection // block. -TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NextTextNode) { +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NextTextNode) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1720,15 +1546,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NextTextNode) { const auto& end = Position(first_paragraph, 20); ASSERT_EQ("First paragraph text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "text", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when available suffix is a parent node text content outside of // selection block. -TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_ParentNode) { +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_ParentNode) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1740,14 +1565,13 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_ParentNode) { const auto& end = Position(first_paragraph, 20); ASSERT_EQ("First paragraph text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("nested", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "nested", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when available suffix contains non-block tag(e.g. <b>). -TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NestedTextNode) { +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NestedTextNode) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1759,14 +1583,13 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NestedTextNode) { const auto& end = Position(first_paragraph, 5); ASSERT_EQ("First", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("bold text paragraph text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "bold text paragraph text", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when available suffix is collected until nested block. -TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NestedBlock) { +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NestedBlock) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( @@ -1778,15 +1601,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NestedBlock) { const auto& end = Position(first_paragraph, 5); ASSERT_EQ("First", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("paragraph", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "paragraph", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when available suffix includes non-block element but stops at // nested block. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NestedBlockInNestedText) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1799,14 +1621,13 @@ TEST_P(TextFragmentSelectorGeneratorTest, const auto& end = Position(first_paragraph, 5); ASSERT_EQ("First", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("bold", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "bold", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when available suffix includes invisible block. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_NestedInvisibleBlock) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1819,15 +1640,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, const auto& end = Position(first_paragraph, 5); ASSERT_EQ("First", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("paragraph text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "paragraph text", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when next node is used for available suffix when selection is // not at last index but there is only space after it. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_SpacesAfterSelection) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1845,15 +1665,14 @@ TEST_P(TextFragmentSelectorGeneratorTest, const auto& end = Position(first_paragraph, 27); ASSERT_EQ("text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("Second paragraph text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "Second paragraph text", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when next node is used for available suffix when selection is // not at last index but there is only invisible block after it. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_InvisibleAfterSelection) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1874,16 +1693,15 @@ TEST_P(TextFragmentSelectorGeneratorTest, const auto& end = Position(first_paragraph, 27); ASSERT_EQ("text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("Second paragraph text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "Second paragraph text", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); } // Check the case when previous node is used for available prefix when selection // is not at last index but there is only invisible block. Invisible block // contains another block which also should be invisible. -TEST_P(TextFragmentSelectorGeneratorTest, +TEST_F(TextFragmentSelectorGeneratorTest, GetNextTextBlock_InvisibleAfterSelection_WithNestedInvisible) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); @@ -1914,56 +1732,92 @@ TEST_P(TextFragmentSelectorGeneratorTest, const auto& end = Position(first_paragraph, 27); ASSERT_EQ("text", PlainText(EphemeralRange(start, end))); - EXPECT_EQ("Second paragraph text", GetDocument() - .GetFrame() - ->GetTextFragmentSelectorGenerator() - ->GetNextTextBlockForTesting(end)); + EXPECT_EQ( + "Second paragraph text", + GetTextFragmentSelectorGenerator()->GetNextTextBlockForTesting(end)); +} + +TEST_F(TextFragmentSelectorGeneratorTest, BeforeAndAfterAnchor) { + SimRequest request("https://example.com/test.html", "text/html"); + LoadURL("https://example.com/test.html"); + request.Complete(R"HTML( + <!DOCTYPE html> + Foo + <div id="first">Hello World</div> + Bar + )HTML"); + + Node* node = GetDocument().getElementById("first"); + const auto& start = Position(node, PositionAnchorType::kBeforeAnchor); + const auto& end = Position(node, PositionAnchorType::kAfterAnchor); + VerifySelectorFails(start, end, LinkGenerationError::kEmptySelection); } -TEST_P(TextFragmentSelectorGeneratorTest, CheckMetrics_Success) { +// Check the case when GetPreviousTextBlock is an EOL node from Shadow Root. +// SharedHighlightingLayoutObjectFix feature disabled does not ensures that the +// next previous non-empty visible text has a layout object. See +// crbug.com/1233762 for more context. +TEST_F(TextFragmentSelectorGeneratorTest, + GetPreviousTextBlock_ShouldCrashWithNoLayoutObject) { base::test::ScopedFeatureList feature_list; - // Basic exact selector case. + feature_list.InitAndDisableFeature( + shared_highlighting::kSharedHighlightingLayoutObjectFix); SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( <!DOCTYPE html> - <div>Test page</div> - <p id='first'>First paragraph text that is longer than 20 chars</p> - <p id='second'>Second paragraph text</p> + <div id="host1"></div> )HTML"); - Node* first_paragraph = GetDocument().getElementById("first")->firstChild(); - const auto& selected_start = Position(first_paragraph, 0); - const auto& selected_end = Position(first_paragraph, 28); - ASSERT_EQ("First paragraph text that is", - PlainText(EphemeralRange(selected_start, selected_end))); + ShadowRoot& shadow1 = + GetDocument().getElementById("host1")->AttachShadowRootInternal( + ShadowRootType::kOpen); + shadow1.setInnerHTML(R"HTML( + <style> + :host {display: contents;} + </style> + <p>Right click the link below to experience a crash:</p> + <a href="/foo" id='first'>I crash</a> + )HTML"); + GetDocument().View()->UpdateAllLifecyclePhasesForTest(); - VerifySelector(selected_start, selected_end, - "First%20paragraph%20text%20that%20is"); - VerifyPreemptiveGenerationMetrics(true); + EXPECT_FALSE(GetDocument().View()->NeedsLayout()); + Node* first_paragraph = shadow1.getElementById("first")->firstChild(); + const auto& start = Position(first_paragraph, 0); + EXPECT_DEATH( + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting(start), + ""); } -TEST_P(TextFragmentSelectorGeneratorTest, CheckMetrics_Failure) { +// Check the case when GetPreviousTextBlock is an EOL node from Shadow Root. +// SharedHighlightingLayoutObjectFix feature enabled ensures that the next +// previous non-empty visible text node has a layout object. See +// crbug.com/1233762 for more context. +TEST_F(TextFragmentSelectorGeneratorTest, + GetPreviousTextBlock_ShouldSkipNodesWithNoLayoutObject) { SimRequest request("https://example.com/test.html", "text/html"); LoadURL("https://example.com/test.html"); request.Complete(R"HTML( <!DOCTYPE html> - <div>Test page</div> - <p id='first'>First paragraph prefix one two three four five six seven - eight nine ten to not unique snippet of text followed by suffix</p> - <p id='second'>Second paragraph prefix one two three four five six seven - eight nine ten to not unique snippet of text followed by suffix</p> + <div id="host1"></div> )HTML"); - Node* first_paragraph = GetDocument().getElementById("first")->firstChild(); - const auto& selected_start = Position(first_paragraph, 80); - const auto& selected_end = Position(first_paragraph, 106); - ASSERT_EQ("not unique snippet of text", - PlainText(EphemeralRange(selected_start, selected_end))); - VerifySelectorFails(selected_start, selected_end, - LinkGenerationError::kContextLimitReached); - VerifyPreemptiveGenerationMetrics(false); + ShadowRoot& shadow1 = + GetDocument().getElementById("host1")->AttachShadowRootInternal( + ShadowRootType::kOpen); + shadow1.setInnerHTML(R"HTML( + <style> + :host {display: contents;} + </style> + <p>Right click the link below to experience a crash:</p> + <a href="/foo" id='first'>I crash</a> + )HTML"); + GetDocument().View()->UpdateAllLifecyclePhasesForTest(); + + EXPECT_FALSE(GetDocument().View()->NeedsLayout()); + Node* first_paragraph = shadow1.getElementById("first")->firstChild(); + const auto& start = Position(first_paragraph, 0); + EXPECT_EQ("Right click the link below to experience a crash:", + GetTextFragmentSelectorGenerator()->GetPreviousTextBlockForTesting( + start)); } -INSTANTIATE_TEST_SUITE_P(All, - TextFragmentSelectorGeneratorTest, - ::testing::Bool()); } // namespace blink |