Commit 30a00d86 authored by Steve Kobes's avatar Steve Kobes Committed by Commit Bot

Buffer layout shifts after pointerdown.

We treat tap but not scroll as an excluding input for the purposes of
cumulative layout shift calculation and the hadRecentInput bit on the
performance entry.

NotifyInput looks at WebInputEvent types to determine exclusion status.
If kPointerDown is followed by kPointerUp, the pending shifts are
excluded.  But if followed by kPointerCancel or kPointerCausedUaAction,
the pending shifts are not excluded.

Bug: 978027
Change-Id: I43f48b0001df168a46e2b44955742144f22c691b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715232
Commit-Queue: Steve Kobes <skobes@chromium.org>
Reviewed-by: default avatarBryan McQuade <bmcquade@chromium.org>
Reviewed-by: default avatarNicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680598}
parent 3aff0c20
......@@ -6,6 +6,7 @@
#include "cc/layers/heads_up_display_layer.h"
#include "cc/layers/picture_layer.h"
#include "third_party/blink/public/platform/web_pointer_event.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/core/frame/local_frame_client.h"
......@@ -321,15 +322,20 @@ void LayoutShiftTracker::NotifyPrePaintFinished() {
}
#endif
ReportShift(score_delta, weighted_score_delta);
if (pointerdown_pending_data_.saw_pointerdown) {
pointerdown_pending_data_.score_delta += score_delta;
pointerdown_pending_data_.weighted_score_delta += weighted_score_delta;
} else {
ReportShift(score_delta, weighted_score_delta);
}
if (use_sweep_line) {
if (!region_experimental_.IsEmpty() && !HadRecentInput()) {
if (!region_experimental_.IsEmpty()) {
SetLayoutShiftRects(region_experimental_.GetRects(), 1, true);
}
region_experimental_.Reset();
} else {
if (!region_.IsEmpty() && !HadRecentInput()) {
if (!region_.IsEmpty()) {
SetLayoutShiftRects(region_.Rects(), granularity_scale, false);
}
region_ = Region();
......@@ -341,7 +347,7 @@ void LayoutShiftTracker::NotifyPrePaintFinished() {
void LayoutShiftTracker::ReportShift(double score_delta,
double weighted_score_delta) {
LocalFrame& frame = frame_view_->GetFrame();
bool had_recent_input = HadRecentInput();
bool had_recent_input = timer_.IsActive();
if (!had_recent_input) {
score_ += score_delta;
......@@ -379,24 +385,44 @@ void LayoutShiftTracker::ReportShift(double score_delta,
}
void LayoutShiftTracker::NotifyInput(const WebInputEvent& event) {
bool event_is_meaningful =
event.GetType() == WebInputEvent::kMouseDown ||
event.GetType() == WebInputEvent::kKeyDown ||
event.GetType() == WebInputEvent::kRawKeyDown ||
const WebInputEvent::Type type = event.GetType();
const bool saw_pointerdown = pointerdown_pending_data_.saw_pointerdown;
const bool pointerdown_became_tap =
saw_pointerdown && type == WebInputEvent::kPointerUp;
const bool event_type_stops_pointerdown_buffering =
type == WebInputEvent::kPointerUp ||
type == WebInputEvent::kPointerCausedUaAction ||
type == WebInputEvent::kPointerCancel;
// Only non-hovering pointerdown requires buffering.
const bool is_hovering_pointerdown =
type == WebInputEvent::kPointerDown &&
static_cast<const WebPointerEvent&>(event).hovering;
const bool should_trigger_shift_exclusion =
type == WebInputEvent::kMouseDown || type == WebInputEvent::kKeyDown ||
type == WebInputEvent::kRawKeyDown ||
// We need to explicitly include tap, as if there are no listeners, we
// won't receive the pointer events.
event.GetType() == WebInputEvent::kGestureTap ||
// Ignore kPointerDown, since it might be a scroll.
event.GetType() == WebInputEvent::kPointerUp;
type == WebInputEvent::kGestureTap || is_hovering_pointerdown ||
pointerdown_became_tap;
if (!event_is_meaningful)
return;
if (should_trigger_shift_exclusion) {
observed_input_or_scroll_ = true;
observed_input_or_scroll_ = true;
// This cancels any previously scheduled task from the same timer.
timer_.StartOneShot(kTimerDelay, FROM_HERE);
UpdateInputTimestamp(event.TimeStamp());
}
// This cancels any previously scheduled task from the same timer.
timer_.StartOneShot(kTimerDelay, FROM_HERE);
UpdateInputTimestamp(event.TimeStamp());
if (saw_pointerdown && event_type_stops_pointerdown_buffering) {
double score_delta = pointerdown_pending_data_.score_delta;
if (score_delta > 0)
ReportShift(score_delta, pointerdown_pending_data_.weighted_score_delta);
pointerdown_pending_data_ = PointerdownPendingData();
}
if (type == WebInputEvent::kPointerDown && !is_hovering_pointerdown)
pointerdown_pending_data_.saw_pointerdown = true;
}
void LayoutShiftTracker::UpdateInputTimestamp(base::TimeTicks timestamp) {
......@@ -424,10 +450,6 @@ void LayoutShiftTracker::NotifyViewportSizeChanged() {
UpdateInputTimestamp(base::TimeTicks::Now());
}
bool LayoutShiftTracker::HadRecentInput() {
return timer_.IsActive();
}
bool LayoutShiftTracker::IsActive() {
// This eliminates noise from the private Page object created by
// SVGImage::DataChanged.
......
......@@ -42,7 +42,6 @@ class CORE_EXPORT LayoutShiftTracker {
void NotifyInput(const WebInputEvent&);
void NotifyScroll(ScrollType, ScrollOffset delta);
void NotifyViewportSizeChanged();
bool HadRecentInput();
bool IsActive();
double Score() const { return score_; }
double WeightedScore() const { return weighted_score_; }
......@@ -85,6 +84,22 @@ class CORE_EXPORT LayoutShiftTracker {
// half of the main frame's reported size; see SubframeWeightingFactor().
double weighted_score_;
// Stores information related to buffering layout shifts after pointerdown.
// We accumulate score deltas in this object until we know whether the
// pointerdown should be treated as a tap (triggering layout shift exclusion)
// or a scroll (not triggering layout shift exclusion). Once the correct
// treatment is known, the pending layout shifts are reported appropriately
// and the PointerdownPendingData object is reset.
struct PointerdownPendingData {
PointerdownPendingData()
: saw_pointerdown(false), score_delta(0), weighted_score_delta(0) {}
bool saw_pointerdown;
double score_delta;
double weighted_score_delta;
};
PointerdownPendingData pointerdown_pending_data_;
// The per-animation-frame impact region.
Region region_;
......
......@@ -10,6 +10,9 @@
#include "third_party/blink/renderer/core/testing/core_unit_test_helper.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/core/timing/dom_window_performance.h"
#include "third_party/blink/renderer/core/timing/layout_shift.h"
#include "third_party/blink/renderer/core/timing/window_performance.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
namespace blink {
......@@ -416,11 +419,15 @@ TEST_F(LayoutShiftTrackerTest, LocalShiftWithoutViewportShift) {
EXPECT_FLOAT_EQ(0.0, GetLayoutShiftTracker().Score());
}
class LayoutShiftTrackerSimTest : public SimTest {};
class LayoutShiftTrackerSimTest : public SimTest {
protected:
void SetUp() override {
SimTest::SetUp();
WebView().MainFrameWidget()->Resize(WebSize(800, 600));
}
};
TEST_F(LayoutShiftTrackerSimTest, SubframeWeighting) {
WebView().MainFrameWidget()->Resize(WebSize(800, 600));
// TODO(crbug.com/943668): Test OOPIF path.
SimRequest main_resource("https://example.com/", "text/html");
SimRequest child_resource("https://example.com/sub.html", "text/html");
......@@ -473,8 +480,6 @@ TEST_F(LayoutShiftTrackerSimTest, SubframeWeighting) {
}
TEST_F(LayoutShiftTrackerSimTest, ViewportSizeChange) {
WebView().MainFrameWidget()->Resize(WebSize(800, 600));
SimRequest main_resource("https://example.com/", "text/html");
LoadURL("https://example.com/");
main_resource.Complete(R"HTML(
......@@ -508,6 +513,88 @@ TEST_F(LayoutShiftTrackerSimTest, ViewportSizeChange) {
EXPECT_FLOAT_EQ(0.0, layout_shift_tracker.Score());
}
class LayoutShiftTrackerPointerdownTest : public LayoutShiftTrackerSimTest {
protected:
void RunTest(WebInputEvent::Type completion_type, bool expect_exclusion);
};
void LayoutShiftTrackerPointerdownTest::RunTest(
WebInputEvent::Type completion_type,
bool expect_exclusion) {
SimRequest main_resource("https://example.com/", "text/html");
LoadURL("https://example.com/");
main_resource.Complete(R"HTML(
<style>
body { margin: 0; height: 1500px; }
#box {
left: 0px;
top: 0px;
width: 400px;
height: 600px;
background: yellow;
position: relative;
}
</style>
<div id="box"></div>
<script>
box.addEventListener("pointerdown", (e) => {
box.style.top = "100px";
e.preventDefault();
});
</script>
)HTML");
Compositor().BeginFrame();
test::RunPendingTasks();
WebPointerProperties pointer_properties = WebPointerProperties(
1 /* PointerId */, WebPointerProperties::PointerType::kTouch,
WebPointerProperties::Button::kLeft);
WebPointerEvent event1(WebInputEvent::kPointerDown, pointer_properties, 5, 5);
WebPointerEvent event2(completion_type, pointer_properties, 5, 5);
// Coordinates inside #box.
event1.SetPositionInWidget(50, 150);
event2.SetPositionInWidget(50, 160);
WebView().MainFrameWidget()->HandleInputEvent(WebCoalescedInputEvent(event1));
Compositor().BeginFrame();
test::RunPendingTasks();
WindowPerformance& perf = *DOMWindowPerformance::performance(Window());
auto& tracker = MainFrame().GetFrameView()->GetLayoutShiftTracker();
EXPECT_EQ(0u, perf.getBufferedEntriesByType("layout-shift").size());
EXPECT_FLOAT_EQ(0.0, tracker.Score());
WebView().MainFrameWidget()->HandleInputEvent(WebCoalescedInputEvent(event2));
// region fraction 50%, distance fraction 1/8
const double expected_shift = 0.5 * 0.125;
auto entries = perf.getBufferedEntriesByType("layout-shift");
EXPECT_EQ(1u, entries.size());
LayoutShift* shift = static_cast<LayoutShift*>(entries.front().Get());
EXPECT_EQ(expect_exclusion, shift->hadRecentInput());
EXPECT_FLOAT_EQ(expected_shift, shift->value());
EXPECT_FLOAT_EQ(expect_exclusion ? 0.0 : expected_shift, tracker.Score());
}
TEST_F(LayoutShiftTrackerPointerdownTest, PointerdownBecomesTap) {
RunTest(WebInputEvent::kPointerUp, true /* expect_exclusion */);
}
TEST_F(LayoutShiftTrackerPointerdownTest, PointerdownCancelled) {
RunTest(WebInputEvent::kPointerCancel, false /* expect_exclusion */);
}
TEST_F(LayoutShiftTrackerPointerdownTest, PointerdownBecomesScroll) {
RunTest(WebInputEvent::kPointerCausedUaAction, false /* expect_exclusion */);
}
TEST_F(LayoutShiftTrackerTest, StableCompositingChanges) {
SetBodyInnerHTML(R"HTML(
<style>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment