Commit ccfc1eb8 authored by Sadrul Habib Chowdhury's avatar Sadrul Habib Chowdhury Committed by Commit Bot

web-contents: Fix a uAF crash.

It is possible for a WebContents to be destroyed while it is being
activated in response to a mouse-click. Handle such case gracefully.

BUG=1040725

Change-Id: Ic73e631bf04e170bc8b1cd818da8d39f1963e006
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1994394
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730244}
parent 0876d796
...@@ -1240,7 +1240,13 @@ void WebContentsViewAura::OnMouseEvent(ui::MouseEvent* event) { ...@@ -1240,7 +1240,13 @@ void WebContentsViewAura::OnMouseEvent(ui::MouseEvent* event) {
// raise-on-click manually, this may override user settings that prevent // raise-on-click manually, this may override user settings that prevent
// focus-stealing. // focus-stealing.
#if !defined(USE_X11) #if !defined(USE_X11)
// It is possible for the web-contents to be destroyed while it is being
// activated. Use a weak-ptr to track whether that happened or not.
// More in https://crbug.com/1040725
auto weak_this = weak_ptr_factory_.GetWeakPtr();
web_contents_->GetDelegate()->ActivateContents(web_contents_); web_contents_->GetDelegate()->ActivateContents(web_contents_);
if (!weak_this)
return;
#endif #endif
} }
......
...@@ -327,8 +327,6 @@ class CONTENT_EXPORT WebContentsViewAura ...@@ -327,8 +327,6 @@ class CONTENT_EXPORT WebContentsViewAura
bool init_rwhv_with_null_parent_for_testing_; bool init_rwhv_with_null_parent_for_testing_;
// Used to ensure that the drag and drop callbacks bound to this
// object are canceled when this object is destroyed.
base::WeakPtrFactory<WebContentsViewAura> weak_ptr_factory_{this}; base::WeakPtrFactory<WebContentsViewAura> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(WebContentsViewAura); DISALLOW_COPY_AND_ASSIGN(WebContentsViewAura);
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "content/browser/web_contents/web_contents_impl.h" #include "content/browser/web_contents/web_contents_impl.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/common/content_features.h" #include "content/public/common/content_features.h"
#include "content/public/test/test_renderer_host.h" #include "content/public/test/test_renderer_host.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -26,6 +27,7 @@ ...@@ -26,6 +27,7 @@
#include "ui/base/dragdrop/drop_target_event.h" #include "ui/base/dragdrop/drop_target_event.h"
#include "ui/base/dragdrop/os_exchange_data.h" #include "ui/base/dragdrop/os_exchange_data.h"
#include "ui/display/display_switches.h" #include "ui/display/display_switches.h"
#include "ui/events/base_event_utils.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
#if defined(OS_WIN) #if defined(OS_WIN)
...@@ -38,6 +40,26 @@ namespace { ...@@ -38,6 +40,26 @@ namespace {
constexpr gfx::Rect kBounds = gfx::Rect(0, 0, 20, 20); constexpr gfx::Rect kBounds = gfx::Rect(0, 0, 20, 20);
constexpr gfx::PointF kClientPt = {5, 10}; constexpr gfx::PointF kClientPt = {5, 10};
constexpr gfx::PointF kScreenPt = {17, 3}; constexpr gfx::PointF kScreenPt = {17, 3};
// Runs a specified callback when a ui::MouseEvent is received.
class RunCallbackOnActivation : public WebContentsDelegate {
public:
explicit RunCallbackOnActivation(base::OnceClosure closure)
: closure_(std::move(closure)) {}
~RunCallbackOnActivation() override = default;
// WebContentsDelegate:
void ActivateContents(WebContents* contents) override {
std::move(closure_).Run();
}
private:
base::OnceClosure closure_;
DISALLOW_COPY_AND_ASSIGN(RunCallbackOnActivation);
};
} // namespace } // namespace
class WebContentsViewAuraTest : public RenderViewHostTestHarness { class WebContentsViewAuraTest : public RenderViewHostTestHarness {
...@@ -145,6 +167,26 @@ TEST_F(WebContentsViewAuraTest, ShowHideParent) { ...@@ -145,6 +167,26 @@ TEST_F(WebContentsViewAuraTest, ShowHideParent) {
EXPECT_EQ(web_contents()->GetVisibility(), content::Visibility::VISIBLE); EXPECT_EQ(web_contents()->GetVisibility(), content::Visibility::VISIBLE);
} }
TEST_F(WebContentsViewAuraTest, WebContentsDestroyedDuringClick) {
RunCallbackOnActivation delegate(base::BindOnce(
&RenderViewHostTestHarness::DeleteContents, base::Unretained(this)));
web_contents()->SetDelegate(&delegate);
// Simulates the mouse press.
ui::MouseEvent mouse_event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
ui::EventTimeForNow(), ui::EF_LEFT_MOUSE_BUTTON,
0);
ui::EventHandler* event_handler = GetView();
event_handler->OnMouseEvent(&mouse_event);
#if defined(USE_X11)
// The web-content is not activated during mouse-press on X11.
// See comment in WebContentsViewAura::OnMouseEvent() for more details.
EXPECT_NE(web_contents(), nullptr);
#else
EXPECT_EQ(web_contents(), nullptr);
#endif
}
TEST_F(WebContentsViewAuraTest, OccludeView) { TEST_F(WebContentsViewAuraTest, OccludeView) {
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(features::kWebContentsOcclusion); scoped_feature_list.InitAndEnableFeature(features::kWebContentsOcclusion);
......
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