From c3670194bbba14ba4657b01f40e5266ac848e6cf Mon Sep 17 00:00:00 2001
From: Robert Flack <flackr@chromium.org>
Date: Tue, 30 Mar 2021 16:18:09 +0000
Subject: [PATCH] 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}
---
content/browser/BUILD.gn | 1 +
.../renderer_host/overscroll_controller.h | 4 ++--
.../overscroll_controller_delegate.cc | 17 +++++++++++++++++
.../overscroll_controller_delegate.h | 8 ++++++--
.../overscroll_controller_unittest.cc | 2 +-
.../render_widget_host_view_aura_unittest.cc | 3 ++-
.../site_per_process_hit_test_browsertest.cc | 3 ++-
.../web_contents/web_contents_view_aura.cc | 6 ++++--
8 files changed, 35 insertions(+), 9 deletions(-)
create mode 100644 content/browser/renderer_host/overscroll_controller_delegate.cc
diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn
index 702c05df5bb64..b90a3a4d7a74c 100644
--- a/content/browser/BUILD.gn
+++ b/content/browser/BUILD.gn
@@ -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",
diff --git a/content/browser/renderer_host/overscroll_controller.h b/content/browser/renderer_host/overscroll_controller.h
index c0ad10492be0d..e77d592ed8c60 100644
--- a/content/browser/renderer_host/overscroll_controller.h
+++ b/content/browser/renderer_host/overscroll_controller.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
diff --git a/content/browser/renderer_host/overscroll_controller_delegate.cc b/content/browser/renderer_host/overscroll_controller_delegate.cc
new file mode 100644
index 0000000000000..c2e79cb4d6515
--- /dev/null
+++ b/content/browser/renderer_host/overscroll_controller_delegate.cc
@@ -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
diff --git a/content/browser/renderer_host/overscroll_controller_delegate.h b/content/browser/renderer_host/overscroll_controller_delegate.h
index 3ae6679573f4d..91f34dae989b3 100644
--- a/content/browser/renderer_host/overscroll_controller_delegate.h
+++ b/content/browser/renderer_host/overscroll_controller_delegate.h
@@ -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);
};
diff --git a/content/browser/renderer_host/overscroll_controller_unittest.cc b/content/browser/renderer_host/overscroll_controller_unittest.cc
index 986a654f3b5dd..e9e0a5bee3524 100644
--- a/content/browser/renderer_host/overscroll_controller_unittest.cc
+++ b/content/browser/renderer_host/overscroll_controller_unittest.cc
@@ -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 {
diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
index 86344e96352cd..e90a9a9312a0b 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
@@ -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));
diff --git a/content/browser/site_per_process_hit_test_browsertest.cc b/content/browser/site_per_process_hit_test_browsertest.cc
index 43b73b74fa6fe..28b437e04c711 100644
--- a/content/browser/site_per_process_hit_test_browsertest.cc
+++ b/content/browser/site_per_process_hit_test_browsertest.cc
@@ -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)
diff --git a/content/browser/web_contents/web_contents_view_aura.cc b/content/browser/web_contents/web_contents_view_aura.cc
index d8d434d386b40..14a77d49fd5df 100644
--- a/content/browser/web_contents/web_contents_view_aura.cc
+++ b/content/browser/web_contents/web_contents_view_aura.cc
@@ -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()