Commit 67d6379a authored by Sarthak Shah's avatar Sarthak Shah Committed by Commit Bot

Fix Crash in RenderWidgetHostImpl::GetRootWidgetViewportSize

In current case, if the view is destroyed and fling controller is trying
to generate fling curve, it crashes when it calls
RenderWidgetHostImpl::GetRootWidgetViewportSize to get the size of the
root view.

This CL fixes the issue by performing a null check for the view. If
destroyed, it will not generate fling curve.

Bug: 1030354
Change-Id: I81f7e7170691cbe9c8cd274d71a4825e046b78a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972609
Commit-Queue: Sarthak Shah <sarsha@microsoft.com>
Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Reviewed-by: default avatarDaniel Libby <dlibby@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#731870}
parent 50a96c58
......@@ -503,6 +503,26 @@ IN_PROC_BROWSER_TEST_F(BrowserSideFlingBrowserTest,
EXPECT_TRUE(
router->forced_last_fling_start_target_to_stop_flinging_for_test());
}
// Check that fling controller does not generate fling curve when view is
// destroyed.
IN_PROC_BROWSER_TEST_F(BrowserSideFlingBrowserTest,
NoFlingWhenViewIsDestroyed) {
LoadURL(kBrowserFlingDataURL);
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetFrameTree()
->root();
GetWidgetHost()->GetView()->Destroy();
SimulateTouchscreenFling(GetWidgetHost());
// As the view is destroyed, there shouldn't be any active fling.
EXPECT_FALSE(static_cast<InputRouterImpl*>(GetWidgetHost()->input_router())
->IsFlingActiveForTest());
EXPECT_EQ(
0, EvalJs(root->current_frame_host(), "window.scrollY").ExtractDouble());
}
#endif // !defined(OS_MACOSX)
class PhysicsBasedFlingCurveBrowserTest : public BrowserSideFlingBrowserTest {
......
......@@ -391,14 +391,24 @@ bool FlingController::UpdateCurrentFlingState(
return false;
}
gfx::Size root_widget_viewport_size =
event_sender_client_->GetRootWidgetViewportSize();
// If the view is destroyed while FlingController is generating fling curve,
// |GetRootWidgetViewportSize()| will return empty size. Reset the
// state of fling_booster_ and return false.
if (root_widget_viewport_size.IsEmpty()) {
fling_booster_.Reset();
EndCurrentFling();
return false;
}
fling_curve_ = std::unique_ptr<blink::WebGestureCurve>(
ui::WebGestureCurveImpl::CreateFromDefaultPlatformCurve(
current_fling_parameters_.source_device,
current_fling_parameters_.velocity,
gfx::Vector2dF() /*initial_offset*/, false /*on_main_thread*/,
GetContentClient()->browser()->ShouldUseMobileFlingCurve(),
current_fling_parameters_.global_point,
event_sender_client_->GetRootWidgetViewportSize()));
current_fling_parameters_.global_point, root_widget_viewport_size));
return true;
}
......
......@@ -132,6 +132,8 @@ class CONTENT_EXPORT GestureEventQueue {
InputEventAckSource ack_source,
InputEventAckState ack_result);
bool IsFlingActiveForTest() { return FlingInProgressForTest(); }
private:
friend class GestureEventQueueTest;
friend class MockRenderWidgetHost;
......
......@@ -710,6 +710,10 @@ void InputRouterImpl::ForceResetTouchActionForTest() {
touch_action_filter_.ForceResetTouchActionForTest();
}
bool InputRouterImpl::IsFlingActiveForTest() {
return gesture_event_queue_.IsFlingActiveForTest();
}
void InputRouterImpl::OnSetTouchAction(cc::TouchAction touch_action) {
TRACE_EVENT1("input", "InputRouterImpl::OnSetTouchAction", "action",
touch_action);
......
......@@ -122,6 +122,8 @@ class CONTENT_EXPORT InputRouterImpl : public InputRouter,
void ForceResetTouchActionForTest();
bool IsFlingActiveForTest();
private:
friend class InputRouterImplTest;
friend class InputRouterImplTestBase;
......
......@@ -1228,18 +1228,20 @@ void RenderWidgetHostImpl::ForwardGestureEventWithLatencyInfo(
is_in_gesture_scroll_[static_cast<int>(gesture_event.SourceDevice())] =
false;
is_in_touchpad_gesture_fling_ = false;
if (scroll_peak_gpu_mem_tracker_ &&
!view_->is_currently_scrolling_viewport()) {
// We start tracking peak gpu-memory usage when the initial scroll-begin
// is dispatched. However, it is possible that the scroll-begin did not
// trigger any scrolls (e.g. the page is not scrollable). In such cases,
// we do not want to report the peak-memory usage metric. So it is
// canceled here.
scroll_peak_gpu_mem_tracker_->Cancel();
if (view_) {
if (scroll_peak_gpu_mem_tracker_ &&
!view_->is_currently_scrolling_viewport()) {
// We start tracking peak gpu-memory usage when the initial scroll-begin
// is dispatched. However, it is possible that the scroll-begin did not
// trigger any scrolls (e.g. the page is not scrollable). In such cases,
// we do not want to report the peak-memory usage metric. So it is
// canceled here.
scroll_peak_gpu_mem_tracker_->Cancel();
}
view_->set_is_currently_scrolling_viewport(false);
}
scroll_peak_gpu_mem_tracker_ = nullptr;
if (view_)
view_->set_is_currently_scrolling_viewport(false);
} else if (gesture_event.GetType() ==
blink::WebInputEvent::kGestureFlingStart) {
if (gesture_event.SourceDevice() == blink::WebGestureDevice::kTouchpad) {
......@@ -3144,7 +3146,15 @@ void RenderWidgetHostImpl::OnZoomToFindInPageRectInMainFrame(
}
gfx::Size RenderWidgetHostImpl::GetRootWidgetViewportSize() {
if (!view_)
return gfx::Size();
// if |view_| is RWHVCF and |frame_connector_| is destroyed, then call to
// GetRootView will return null pointer.
auto* root_view = view_->GetRootView();
if (!root_view)
return gfx::Size();
return root_view->GetVisibleViewportSize();
}
......
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