diff --git a/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom b/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom index e2ae773e928105..a9730cfb0d126a 100644 --- a/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom +++ b/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom @@ -3634,6 +3634,7 @@ enum WebFeature { kLinkRelPrefetchAsDocumentSameOrigin = 4313, kLinkRelPrefetchAsDocumentCrossOrigin = 4314, kPersistentQuotaType = 4315, + kCrossOriginScrollIntoView = 4316, // Add new features immediately above this line. Don't change assigned // numbers of any item, and don't reuse removed slots. diff --git a/third_party/blink/renderer/core/frame/remote_frame.cc b/third_party/blink/renderer/core/frame/remote_frame.cc index 6479eeebf9999b..c1d90c90934dbe 100644 --- a/third_party/blink/renderer/core/frame/remote_frame.cc +++ b/third_party/blink/renderer/core/frame/remote_frame.cc @@ -697,7 +697,8 @@ void RemoteFrame::ScrollRectToVisible( PhysicalRect::EnclosingRect(rect_to_scroll), owner_object->View()); scroll_into_view_util::ScrollRectToVisible(*owner_object, absolute_rect, - std::move(params)); + std::move(params), + /*from_remote_frame=*/true); } void RemoteFrame::IntrinsicSizingInfoOfChildChanged( diff --git a/third_party/blink/renderer/core/frame/root_frame_viewport.cc b/third_party/blink/renderer/core/frame/root_frame_viewport.cc index 634b0d4eb1c693..3bc2939b018ead 100644 --- a/third_party/blink/renderer/core/frame/root_frame_viewport.cc +++ b/third_party/blink/renderer/core/frame/root_frame_viewport.cc @@ -372,6 +372,10 @@ PhysicalRect RootFrameViewport::ScrollIntoView( // Return the newly moved rect to absolute coordinates. // TODO(szager): PaintLayerScrollableArea::ScrollIntoView clips the return // value to the visible content rect, but this does not. + // TODO(bokan): This returns an unchanged rect for scroll sequences (the PLSA + // version correctly computes what the rect will be when the sequence is + // executed) and we can't just adjust by `new_scroll_offset` since, to get to + // absolute coordinates, we must offset by only the layout viewport's scroll. rect_in_document.Move( -PhysicalOffset::FromVector2dFRound(LayoutViewport().GetScrollOffset())); return rect_in_document; diff --git a/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc b/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc index 5fbf828bb656f1..e39e0f2f94c05d 100644 --- a/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc +++ b/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc @@ -28,6 +28,7 @@ #include "third_party/blink/renderer/core/testing/sim/sim_compositor.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/instrumentation/use_counter.h" namespace blink { @@ -777,6 +778,97 @@ TEST_F(ScrollIntoViewTest, LongDistanceSmoothScrollFinishedInThreeSeconds) { ASSERT_EQ(Window().scrollY(), target->OffsetTop()); } +TEST_F(ScrollIntoViewTest, OriginCrossingUseCounter) { + v8::HandleScope HandleScope(v8::Isolate::GetCurrent()); + WebView().MainFrameViewWidget()->Resize(gfx::Size(800, 600)); + SimRequest main("https://example.com/test.html", "text/html"); + SimRequest local_child("https://example.com/child.html", "text/html"); + SimRequest xorigin_child("https://xorigin.com/child.html", "text/html"); + LoadURL("https://example.com/test.html"); + + main.Complete( + R"HTML( + + + + + )HTML"); + + String child_html = + R"HTML( + + +
Target
+ )HTML"; + + local_child.Complete(child_html); + xorigin_child.Complete(child_html); + + Element* local_child_frame = GetDocument().getElementById("localChildFrame"); + Element* xorigin_child_frame = + GetDocument().getElementById("xoriginChildFrame"); + Document* local_child_document = + To(local_child_frame)->contentDocument(); + Document* xorigin_child_document = + To(xorigin_child_frame)->contentDocument(); + + // Same origin frames shouldn't count the scroll into view. + { + ASSERT_EQ(GetDocument().View()->GetScrollableArea()->GetScrollOffset(), + ScrollOffset(0, 0)); + + Element* target = local_child_document->getElementById("target"); + target->scrollIntoView(); + + ASSERT_NE(GetDocument().View()->GetScrollableArea()->GetScrollOffset(), + ScrollOffset(0, 0)); + EXPECT_FALSE( + GetDocument().IsUseCounted(WebFeature::kCrossOriginScrollIntoView)); + } + + GetDocument().View()->GetScrollableArea()->SetScrollOffset( + ScrollOffset(0, 0), mojom::blink::ScrollType::kProgrammatic); + + // Cross origin frames should record the scroll into view use count. + { + ASSERT_EQ(GetDocument().View()->GetScrollableArea()->GetScrollOffset(), + ScrollOffset(0, 0)); + + Element* target = xorigin_child_document->getElementById("target"); + target->scrollIntoView(); + + ASSERT_NE(GetDocument().View()->GetScrollableArea()->GetScrollOffset(), + ScrollOffset(0, 0)); + EXPECT_TRUE( + GetDocument().IsUseCounted(WebFeature::kCrossOriginScrollIntoView)); + } +} + TEST_F(ScrollIntoViewTest, FromDisplayNoneIframe) { v8::HandleScope HandleScope(v8::Isolate::GetCurrent()); WebView().MainFrameViewWidget()->Resize(gfx::Size(800, 600)); diff --git a/third_party/blink/renderer/core/scroll/scroll_into_view_util.cc b/third_party/blink/renderer/core/scroll/scroll_into_view_util.cc index 3b5850715a7047..ed80c498ec7ae0 100644 --- a/third_party/blink/renderer/core/scroll/scroll_into_view_util.cc +++ b/third_party/blink/renderer/core/scroll/scroll_into_view_util.cc @@ -20,6 +20,7 @@ #include "third_party/blink/renderer/core/page/frame_tree.h" #include "third_party/blink/renderer/core/page/page.h" #include "third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h" +#include "third_party/blink/renderer/platform/instrumentation/use_counter.h" #include "third_party/blink/renderer/platform/weborigin/security_origin.h" #include "ui/gfx/geometry/rect_f.h" @@ -117,7 +118,8 @@ absl::optional GetScrollParent( absl::optional PerformBubblingScrollIntoView( const LayoutBox& box, const PhysicalRect& absolute_rect, - mojom::blink::ScrollIntoViewParamsPtr& params) { + mojom::blink::ScrollIntoViewParamsPtr& params, + bool from_remote_frame) { DCHECK(params->type == mojom::blink::ScrollType::kProgrammatic || params->type == mojom::blink::ScrollType::kUser); @@ -127,6 +129,11 @@ absl::optional PerformBubblingScrollIntoView( const LayoutBox* current_box = &box; PhysicalRect absolute_rect_to_scroll = absolute_rect; + // TODO(bokan): Temporary, to track cross-origin scroll-into-view prevalence. + // https://crbug.com/1339003. + const SecurityOrigin* starting_frame_origin = + box.GetFrame()->GetSecurityContext()->GetSecurityOrigin(); + while (current_box) { if (absolute_rect_to_scroll.Width() <= 0) absolute_rect_to_scroll.SetWidth(LayoutUnit(1)); @@ -157,8 +164,40 @@ absl::optional PerformBubblingScrollIntoView( } if (area_to_scroll) { + ScrollOffset scroll_before = area_to_scroll->GetScrollOffset(); + DCHECK(area_to_scroll->GetSmoothScrollSequencer()); + wtf_size_t num_scroll_sequences = + area_to_scroll->GetSmoothScrollSequencer()->GetCount(); + absolute_rect_to_scroll = area_to_scroll->ScrollIntoView(absolute_rect_to_scroll, params); + + // TODO(bokan): Temporary, to track cross-origin scroll-into-view + // prevalence. https://crbug.com/1339003. + // If this is for a scroll sequence, GetScrollOffset won't change until + // all the animations in the sequence are run which happens + // asynchronously after this method returns. Thus, for scroll sequences, + // check instead if an entry was added to the sequence which occurs only + // if the scroll offset is changed as a result of ScrollIntoView. + bool scroll_changed = + params->is_for_scroll_sequence + ? area_to_scroll->GetSmoothScrollSequencer()->GetCount() != + num_scroll_sequences + : area_to_scroll->GetScrollOffset() != scroll_before; + if (scroll_changed && !params->for_focused_editable && + params->type == mojom::blink::ScrollType::kProgrammatic) { + const SecurityOrigin* current_frame_origin = + current_box->GetFrame()->GetSecurityContext()->GetSecurityOrigin(); + if (!current_frame_origin->CanAccess(starting_frame_origin) || + from_remote_frame) { + // ScrollIntoView caused a visible scroll in an origin that can't be + // accessed from where the ScrollIntoView was initiated. + DCHECK(params->cross_origin_boundaries); + UseCounter::Count( + current_box->GetFrame()->LocalFrameRoot().GetDocument(), + WebFeature::kCrossOriginScrollIntoView); + } + } } bool is_fixed_to_frame = @@ -226,7 +265,8 @@ namespace scroll_into_view_util { void ScrollRectToVisible(const LayoutObject& layout_object, const PhysicalRect& absolute_rect, - mojom::blink::ScrollIntoViewParamsPtr params) { + mojom::blink::ScrollIntoViewParamsPtr params, + bool from_remote_frame) { LayoutBox* enclosing_box = layout_object.EnclosingBox(); if (!enclosing_box) return; @@ -239,7 +279,8 @@ void ScrollRectToVisible(const LayoutObject& layout_object, params->type == mojom::blink::ScrollType::kProgrammatic; absl::optional updated_absolute_rect = - PerformBubblingScrollIntoView(*enclosing_box, absolute_rect, params); + PerformBubblingScrollIntoView(*enclosing_box, absolute_rect, params, + from_remote_frame); frame->GetSmoothScrollSequencer().RunQueuedAnimations(); diff --git a/third_party/blink/renderer/core/scroll/scroll_into_view_util.h b/third_party/blink/renderer/core/scroll/scroll_into_view_util.h index 9ef1c3b44c9c76..3c7877a2bb38a7 100644 --- a/third_party/blink/renderer/core/scroll/scroll_into_view_util.h +++ b/third_party/blink/renderer/core/scroll/scroll_into_view_util.h @@ -24,9 +24,12 @@ namespace scroll_into_view_util { // LayoutObject, and scrolls the LayoutObject and all its containers such that // the child content of the LayoutObject at that rect is visible in the // viewport. +// TODO(bokan): `from_remote_frame` is temporary, to track cross-origin +// scroll-into-view prevalence. https://crbug.com/1339003. void CORE_EXPORT ScrollRectToVisible(const LayoutObject&, const PhysicalRect&, - mojom::blink::ScrollIntoViewParamsPtr); + mojom::blink::ScrollIntoViewParamsPtr, + bool from_remote_frame = false); // ScrollFocusedEditableIntoView uses the caret rect for ScrollIntoView but // stores enough information in `params` so that the editable element's bounds diff --git a/third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.cc b/third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.cc index cc4a299ec11073..a9349ca6388982 100644 --- a/third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.cc +++ b/third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.cc @@ -66,6 +66,10 @@ bool SmoothScrollSequencer::FilterNewScrollOrAbortCurrent( return false; } +wtf_size_t SmoothScrollSequencer::GetCount() const { + return queue_.size(); +} + void SmoothScrollSequencer::DidDisposeScrollableArea( const ScrollableArea& area) { for (Member& sequenced_scroll : queue_) { diff --git a/third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h b/third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h index 6066fd8e43bb13..a9b19cdd7ca6f3 100644 --- a/third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h +++ b/third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h @@ -60,6 +60,11 @@ class CORE_EXPORT SmoothScrollSequencer final // incoming scroll. It may also abort the current sequenced scroll. bool FilterNewScrollOrAbortCurrent(mojom::blink::ScrollType incoming_type); + // Returns the size of the scroll sequence queue. + // TODO(bokan): Temporary, to track cross-origin scroll-into-view prevalence. + // https://crbug.com/1339003. + wtf_size_t GetCount() const; + void DidDisposeScrollableArea(const ScrollableArea&); void Trace(Visitor*) const; diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index 0e9010ec78566f..06d0c689197a49 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -40890,6 +40890,7 @@ Called by update_use_counter_feature_enum.py.--> +