Skip to content

Commit

Permalink
Revert "Reland "Correctly initialize and test SnapContainerData in cc.""
Browse files Browse the repository at this point in the history
This reverts commit 043b97c.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Reland "Correctly initialize and test SnapContainerData in cc."
> 
> This reverts commit 0fc70ab.
> 
> Reason for revert: <INSERT REASONING HERE>
> 
> Original change's description:
> > Revert "Correctly initialize and test SnapContainerData in cc."
> > 
> > This reverts commit 37061f8.
> > 
> > Reason for revert:
> > The layout tests added by this CL is failing on the CL:
> > https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Win10/37804
> > 
> > Original change's description:
> > > Correctly initialize and test SnapContainerData in cc.
> > > 
> > > This patch initializes the rect_ field in SnapContainerData in copy
> > > constructor and move constructor. Fixing a snapping issue in the
> > > composited pages.
> > > 
> > > We also adds external/wpt/css/css-scroll-snap/ to the virtual/threaded
> > > test suite so that they are tested with the composited cases.
> > > 
> > > snap-at-user-scroll-end-manual-automation.js calls
> > > mouseClickInTarget() of pointerevent_common_input.js. That method
> > > invokes programmatic scrolls. The test was written before programmatic
> > > scroll snapping was implemented so it worked at that time. However,
> > > with programmatic scroll snapping implemented, it will always snap
> > > in mouseClickIntarget() and cannot test whether the user scroll snaps.
> > > This patch adds a parameter shouldScrollToTarget to mouseClickInTarget
> > > to avoid invoking programmatic scrolls in the test.
> > > 
> > > This patch also checks nullptr for layout_box in
> > > ScrollManager::SnapAtGestureScrollEnd() to fix a crash.
> > > 
> > > Bug: 862406, 862571
> > > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
> > > Change-Id: I6a53818cf74421a4100ad6f908158abf302a5b8e
> > > Reviewed-on: https://chromium-review.googlesource.com/1132386
> > > Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
> > > Reviewed-by: Majid Valipour <majidvp@chromium.org>
> > > Reviewed-by: Robert Flack <flackr@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#574781}
> > 
> > TBR=flackr@chromium.org,majidvp@chromium.org,sunyunjia@chromium.org
> > 
> > Change-Id: I55c77c05c0381c8ac638bd106d2d18b1b4332745
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 862406, 862571
> > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
> > Reviewed-on: https://chromium-review.googlesource.com/1136171
> > Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
> > Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#574825}
> 
> TBR=flackr@chromium.org,majidvp@chromium.org,sunyunjia@chromium.org,tzik@chromium.org
> 
> Change-Id: I4a9480b68c15e0dfcfd13df6ed6c0e6b8ab5a8e3
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 862406, 862571
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
> Reviewed-on: https://chromium-review.googlesource.com/1136671
> Reviewed-by: Sandra Sun <sunyunjia@chromium.org>
> Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#574926}

TBR=flackr@chromium.org,majidvp@chromium.org,sunyunjia@chromium.org,tzik@chromium.org

Change-Id: If9d1e6f73393a7dddc9f8f418606946f070b45be
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 862406, 862571
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1136612
Reviewed-by: Sandra Sun <sunyunjia@chromium.org>
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574931}
  • Loading branch information
sunyunjia authored and Commit Bot committed Jul 13, 2018
1 parent 08a9265 commit 004406c
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 22 deletions.
15 changes: 12 additions & 3 deletions cc/input/scroll_snap_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,24 @@ SnapContainerData::SnapContainerData(ScrollSnapType type,

SnapContainerData::SnapContainerData(const SnapContainerData& other) = default;

SnapContainerData::SnapContainerData(SnapContainerData&& other) = default;
SnapContainerData::SnapContainerData(SnapContainerData&& other)
: scroll_snap_type_(other.scroll_snap_type_),
max_position_(other.max_position_),
proximity_range_(other.proximity_range_),
snap_area_list_(std::move(other.snap_area_list_)) {}

SnapContainerData::~SnapContainerData() = default;

SnapContainerData& SnapContainerData::operator=(
const SnapContainerData& other) = default;

SnapContainerData& SnapContainerData::operator=(SnapContainerData&& other) =
default;
SnapContainerData& SnapContainerData::operator=(SnapContainerData&& other) {
scroll_snap_type_ = other.scroll_snap_type_;
max_position_ = other.max_position_;
proximity_range_ = other.proximity_range_;
snap_area_list_ = std::move(other.snap_area_list_);
return *this;
}

void SnapContainerData::AddSnapAreaData(SnapAreaData snap_area_data) {
snap_area_list_.push_back(snap_area_data);
Expand Down
5 changes: 0 additions & 5 deletions third_party/WebKit/LayoutTests/VirtualTestSuites
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@
"base": "http/tests/worklet",
"args": ["--enable-threaded-compositing"]
},
{
"prefix": "threaded",
"base": "external/wpt/css/css-scroll-snap",
"args": ["--enable-threaded-compositing"]
},
{
"prefix": "threaded",
"base": "compositing/webgl",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ <h4>Tests that the window can snap at user scroll end.</h4>
</div>

<script>
var snap_test = async_test('Tests that window should snap at user scroll end.');
var body = document.body;
var button = document.getElementById("btn");
var target = document.getElementById("target");
Expand Down Expand Up @@ -81,7 +80,7 @@ <h4>Tests that the window can snap at user scroll end.</h4>
// To make the test result visible.
var content = document.getElementById("content");
body.removeChild(content);
snap_test.done();
done();
}

</script>
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ function waitForAnimationEnd() {
}

function inject_input() {
return smoothScroll(100, 20, 20, 'downright', 4000).then(() => {
return smoothScroll(2, 20, 20, 'downright', 4000).then(() => {
return waitForAnimationEnd();
}).then(() => {
return mouseClickInTarget('#btn', undefined, 'left',
/* shouldScrollToTarget = */ false);
return mouseClickInTarget('#btn');
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function mouseChordedButtonPress(targetSelector) {
});
}

function mouseClickInTarget(targetSelector, targetFrame, button, shouldScrollToTarget = true) {
function mouseClickInTarget(targetSelector, targetFrame, button) {
var targetDocument = document;
var frameLeft = 0;
var frameTop = 0;
Expand All @@ -122,8 +122,7 @@ function mouseClickInTarget(targetSelector, targetFrame, button, shouldScrollToT
}
return new Promise(function(resolve, reject) {
if (window.chrome && chrome.gpuBenchmarking) {
if (shouldScrollToTarget)
scrollPageIfNeeded(targetSelector, targetDocument);
scrollPageIfNeeded(targetSelector, targetDocument);
var target = targetDocument.querySelector(targetSelector);
var targetRect = target.getBoundingClientRect();
var xPosition = frameLeft + targetRect.left + boundaryOffset;
Expand Down

This file was deleted.

4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/input/scroll_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -628,10 +628,10 @@ void ScrollManager::SnapAtGestureScrollEnd() {
return;
SnapCoordinator* snap_coordinator =
frame_->GetDocument()->GetSnapCoordinator();
LayoutBox* layout_box = LayoutBoxForSnapping();
if (!snap_coordinator || !layout_box)
if (!snap_coordinator)
return;

LayoutBox* layout_box = LayoutBoxForSnapping();
snap_coordinator->PerformSnapping(*layout_box,
did_scroll_x_for_scroll_gesture_,
did_scroll_y_for_scroll_gesture_);
Expand Down

0 comments on commit 004406c

Please sign in to comment.