0

Store OverscrollControllerDelegate as a WeakPtr.

This is a speculative fix for https://crbug.com/1183933. The
OverscrollControllerDelegate is owned by the WebContentsViewAura but an
unowned pointer is set on the given view's OverscrollController. It's
unclear whether the WebContentsViewAura is guaranteed to outlive or live
as long as the RenderWidgetHostViewAura. By using a weak pointer we
ensure the delegate is no longer called if it is destructed.

Bug: 1183933
Change-Id: I41dc6e2e4ba26dd5be958fff5e7da3a5fc66c513
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2791883
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#867641}
This commit is contained in:
Robert Flack
2021-03-30 16:18:09 +00:00
committed by Chromium LUCI CQ
parent 2d80cb6016
commit c3670194bb
8 changed files with 35 additions and 9 deletions

@ -1502,6 +1502,7 @@ source_set("browser") {
"renderer_host/overscroll_configuration.cc",
"renderer_host/overscroll_controller.cc",
"renderer_host/overscroll_controller.h",
"renderer_host/overscroll_controller_delegate.cc",
"renderer_host/overscroll_controller_delegate.h",
"renderer_host/p2p/socket_dispatcher_host.cc",
"renderer_host/p2p/socket_dispatcher_host.h",

@ -65,7 +65,7 @@ class CONTENT_EXPORT OverscrollController {
OverscrollMode overscroll_mode() const { return overscroll_mode_; }
void set_delegate(OverscrollControllerDelegate* delegate) {
void set_delegate(base::WeakPtr<OverscrollControllerDelegate> delegate) {
delegate_ = delegate;
}
@ -159,7 +159,7 @@ class CONTENT_EXPORT OverscrollController {
// The delegate that receives the overscroll updates. The delegate is not
// owned by this controller.
OverscrollControllerDelegate* delegate_ = nullptr;
base::WeakPtr<OverscrollControllerDelegate> delegate_;
// A inertial scroll (fling) event may complete an overscroll gesture and
// navigate to a new page or cancel the overscroll animation. In both cases

@ -0,0 +1,17 @@
// Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "content/browser/renderer_host/overscroll_controller_delegate.h"
namespace content {
OverscrollControllerDelegate::OverscrollControllerDelegate() = default;
OverscrollControllerDelegate::~OverscrollControllerDelegate() = default;
base::WeakPtr<OverscrollControllerDelegate>
OverscrollControllerDelegate::GetWeakPtr() {
return weak_factory_.GetWeakPtr();
}
} // namespace content

@ -7,6 +7,7 @@
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "content/browser/renderer_host/overscroll_controller.h"
#include "content/common/content_export.h"
@ -19,8 +20,8 @@ namespace content {
// overscroll deltas maintained and reported by the controller.
class CONTENT_EXPORT OverscrollControllerDelegate {
public:
OverscrollControllerDelegate() {}
virtual ~OverscrollControllerDelegate() {}
OverscrollControllerDelegate();
virtual ~OverscrollControllerDelegate();
// Get the size of the display containing the view corresponding to the
// delegate.
@ -47,7 +48,10 @@ class CONTENT_EXPORT OverscrollControllerDelegate {
// overscroll delta corresponding to the current overscroll mode.
virtual base::Optional<float> GetMaxOverscrollDelta() const = 0;
base::WeakPtr<OverscrollControllerDelegate> GetWeakPtr();
private:
base::WeakPtrFactory<OverscrollControllerDelegate> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(OverscrollControllerDelegate);
};

@ -30,7 +30,7 @@ class OverscrollControllerTest : public ::testing::Test {
features::kTouchpadOverscrollHistoryNavigation);
delegate_ = std::make_unique<TestOverscrollDelegate>(gfx::Size(400, 300));
controller_ = std::make_unique<OverscrollController>();
controller_->set_delegate(delegate_.get());
controller_->set_delegate(delegate_->GetWeakPtr());
}
void TearDown() override {

@ -808,7 +808,8 @@ class RenderWidgetHostViewAuraOverscrollTest
->GetDisplayNearestView(view_->GetNativeView())
.size();
overscroll_delegate_.reset(new TestOverscrollDelegate(display_size));
view_->overscroll_controller()->set_delegate(overscroll_delegate_.get());
view_->overscroll_controller()->set_delegate(
overscroll_delegate_->GetWeakPtr());
InitViewForFrame(nullptr);
view_->SetBounds(gfx::Rect(0, 0, 400, 200));

@ -2350,7 +2350,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessHitTestBrowserTest,
std::unique_ptr<MockOverscrollControllerDelegateAura>
mock_overscroll_delegate =
std::make_unique<MockOverscrollControllerDelegateAura>(rwhva);
rwhva->overscroll_controller()->set_delegate(mock_overscroll_delegate.get());
rwhva->overscroll_controller()->set_delegate(
mock_overscroll_delegate->GetWeakPtr());
MockOverscrollObserver* mock_overscroll_observer =
mock_overscroll_delegate.get();
#elif defined(OS_ANDROID)

@ -765,8 +765,10 @@ void WebContentsViewAura::InstallOverscrollControllerDelegate(
if (!gesture_nav_simple_)
gesture_nav_simple_ = std::make_unique<GestureNavSimple>(web_contents_);
if (view)
view->overscroll_controller()->set_delegate(gesture_nav_simple_.get());
if (view) {
view->overscroll_controller()->set_delegate(
gesture_nav_simple_->GetWeakPtr());
}
}
ui::TouchSelectionController* WebContentsViewAura::GetSelectionController()