Commit 6c90e03d authored by Sandra Sun's avatar Sandra Sun Committed by Commit Bot

Check nullptrs for SnapFling in ScrollManager.

The previous code doesn't check nullptrs thoroughly, causing
clusterfuzz pages to crash. This patch adds the checks for those cases
to make sure they don't crash.

Bug: 863338
Change-Id: Ibed005057fa376cb65200ad51e6dbb16bafa2c6a
Reviewed-on: https://chromium-review.googlesource.com/1142457
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Reviewed-by: default avatarRobert Flack <flackr@chromium.org>
Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580281}
parent e5000300
...@@ -33,7 +33,9 @@ div { ...@@ -33,7 +33,9 @@ div {
</div> </div>
<script> <script>
var body = document.body;
var scroller = document.getElementById("scroller"); var scroller = document.getElementById("scroller");
var space = document.getElementById("space");
function scrollLeft() { function scrollLeft() {
return scroller.scrollLeft; return scroller.scrollLeft;
...@@ -51,7 +53,7 @@ promise_test (async () => { ...@@ -51,7 +53,7 @@ promise_test (async () => {
promise_test (async () => { promise_test (async () => {
scroller.scrollTo(0, 0); scroller.scrollTo(0, 0);
await swipe(100, 200, 200, 'upleft', 1000); await swipe(100, 200, 200, 'upleft', 900);
await waitForAnimationEnd(scrollLeft, 1000, 30); await waitForAnimationEnd(scrollLeft, 1000, 30);
await waitFor( () => { await waitFor( () => {
return approx_equals(scroller.scrollLeft, 200, 1) && return approx_equals(scroller.scrollLeft, 200, 1) &&
...@@ -59,4 +61,32 @@ promise_test (async () => { ...@@ -59,4 +61,32 @@ promise_test (async () => {
}); });
}, "With fling enabled, the scroll ends at the closest snap point to the fling destination.") }, "With fling enabled, the scroll ends at the closest snap point to the fling destination.")
function MakeUnscrollable() {
scroller.removeChild(space);
}
function MakeScrollable() {
scroller.appendChild(space);
}
promise_test (async () => {
scroller.scrollTo(0, 0);
await swipe(100, 200, 200, 'upleft', 1000);
await waitFor( () => {
return scroller.scrollLeft > 120;
});
MakeUnscrollable();
await waitForAnimationEnd(scrollLeft, 1000, 20);
MakeScrollable();
}, "Should not crash if the scroller becomes unscrollable during fling.");
promise_test (async () => {
scroller.scrollTo(0, 0);
await swipe(100, 200, 200, 'upleft', 1000);
await waitFor( () => {
return scroller.scrollLeft > 120;
});
body.removeChild(scroller);
await waitForAnimationEnd(scrollLeft, 1000, 20);
body.appendChild(scroller);
}, "Should not crash if the scroller is removed during fling.");
</script> </script>
...@@ -57,6 +57,15 @@ SnapFlingController::GestureScrollUpdateInfo GetGestureScrollUpdateInfo( ...@@ -57,6 +57,15 @@ SnapFlingController::GestureScrollUpdateInfo GetGestureScrollUpdateInfo(
.event_time = event.TimeStamp()}; .event_time = event.TimeStamp()};
} }
ScrollableArea* ScrollableAreaForSnapping(LayoutBox* layout_box) {
if (!layout_box)
return nullptr;
return layout_box->IsLayoutView()
? layout_box->GetFrameView()->GetScrollableArea()
: layout_box->GetScrollableArea();
}
} // namespace } // namespace
ScrollManager::ScrollManager(LocalFrame& frame) : frame_(frame) { ScrollManager::ScrollManager(LocalFrame& frame) : frame_(frame) {
...@@ -608,20 +617,14 @@ WebInputEventResult ScrollManager::HandleGestureScrollEnd( ...@@ -608,20 +617,14 @@ WebInputEventResult ScrollManager::HandleGestureScrollEnd(
} }
LayoutBox* ScrollManager::LayoutBoxForSnapping() const { LayoutBox* ScrollManager::LayoutBoxForSnapping() const {
if (!previous_gesture_scrolled_element_)
return nullptr;
Element* document_element = frame_->GetDocument()->documentElement(); Element* document_element = frame_->GetDocument()->documentElement();
return previous_gesture_scrolled_element_ == document_element return previous_gesture_scrolled_element_ == document_element
? frame_->GetDocument()->GetLayoutView() ? frame_->GetDocument()->GetLayoutView()
: previous_gesture_scrolled_element_->GetLayoutBox(); : previous_gesture_scrolled_element_->GetLayoutBox();
} }
ScrollableArea* ScrollManager::ScrollableAreaForSnapping() const {
Element* document_element = frame_->GetDocument()->documentElement();
return previous_gesture_scrolled_element_ == document_element
? frame_->View()->GetScrollableArea()
: previous_gesture_scrolled_element_->GetLayoutBox()
->GetScrollableArea();
}
void ScrollManager::SnapAtGestureScrollEnd() { void ScrollManager::SnapAtGestureScrollEnd() {
if (!previous_gesture_scrolled_element_ || if (!previous_gesture_scrolled_element_ ||
(!did_scroll_x_for_scroll_gesture_ && !did_scroll_y_for_scroll_gesture_)) (!did_scroll_x_for_scroll_gesture_ && !did_scroll_y_for_scroll_gesture_))
...@@ -654,7 +657,11 @@ bool ScrollManager::GetSnapFlingInfo(const gfx::Vector2dF& natural_displacement, ...@@ -654,7 +657,11 @@ bool ScrollManager::GetSnapFlingInfo(const gfx::Vector2dF& natural_displacement,
gfx::Vector2dF ScrollManager::ScrollByForSnapFling( gfx::Vector2dF ScrollManager::ScrollByForSnapFling(
const gfx::Vector2dF& delta) { const gfx::Vector2dF& delta) {
DCHECK(previous_gesture_scrolled_element_); ScrollableArea* scrollable_area =
ScrollableAreaForSnapping(LayoutBoxForSnapping());
if (!scrollable_area)
return gfx::Vector2dF();
std::unique_ptr<ScrollStateData> scroll_state_data = std::unique_ptr<ScrollStateData> scroll_state_data =
std::make_unique<ScrollStateData>(); std::make_unique<ScrollStateData>();
...@@ -672,13 +679,16 @@ gfx::Vector2dF ScrollManager::ScrollByForSnapFling( ...@@ -672,13 +679,16 @@ gfx::Vector2dF ScrollManager::ScrollByForSnapFling(
previous_gesture_scrolled_element_); previous_gesture_scrolled_element_);
CustomizedScroll(*scroll_state); CustomizedScroll(*scroll_state);
ScrollableArea* scrollable_area = ScrollableAreaForSnapping();
FloatPoint end_position = scrollable_area->ScrollPosition(); FloatPoint end_position = scrollable_area->ScrollPosition();
return gfx::Vector2dF(end_position.X(), end_position.Y()); return gfx::Vector2dF(end_position.X(), end_position.Y());
} }
void ScrollManager::ScrollEndForSnapFling() { void ScrollManager::ScrollEndForSnapFling() {
if (current_scroll_chain_.empty()) {
NotifyScrollPhaseEndForCustomizedScroll();
ClearGestureScrollState();
return;
}
std::unique_ptr<ScrollStateData> scroll_state_data = std::unique_ptr<ScrollStateData> scroll_state_data =
std::make_unique<ScrollStateData>(); std::make_unique<ScrollStateData>();
scroll_state_data->is_ending = true; scroll_state_data->is_ending = true;
......
...@@ -27,7 +27,6 @@ class LocalFrame; ...@@ -27,7 +27,6 @@ class LocalFrame;
class PaintLayer; class PaintLayer;
class PaintLayerScrollableArea; class PaintLayerScrollableArea;
class Page; class Page;
class ScrollableArea;
class Scrollbar; class Scrollbar;
class ScrollState; class ScrollState;
class WebGestureEvent; class WebGestureEvent;
...@@ -140,7 +139,6 @@ class CORE_EXPORT ScrollManager ...@@ -140,7 +139,6 @@ class CORE_EXPORT ScrollManager
void NotifyScrollPhaseEndForCustomizedScroll(); void NotifyScrollPhaseEndForCustomizedScroll();
LayoutBox* LayoutBoxForSnapping() const; LayoutBox* LayoutBoxForSnapping() const;
ScrollableArea* ScrollableAreaForSnapping() const;
// NOTE: If adding a new field to this class please ensure that it is // NOTE: If adding a new field to this class please ensure that it is
// cleared in |ScrollManager::clear()|. // cleared in |ScrollManager::clear()|.
......
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