Commit e58b75b0 authored by Miyoung Shin's avatar Miyoung Shin Committed by Commit Bot

Fix the null pointer access in WebFrameWidgetBase::SetToolTipText

WebViewPlugin didn't have the implementation of
blink::mojo::WidgetHost, it was able to access the null pointer
access when the mouse pointer is moved to on the plugin view, so
this CL implements blink::mojo::WidgetHost in WebViewPlugin and
adds WebWidget::GetTooltipText for the web test.

Fuzzer report: https://clusterfuzz.com/testcase?key=5128177771413504

Bug: 1081999, 1082648
Change-Id: Ia24d71b687850d796bff02bcca496dc04217c17d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2198517Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Cr-Commit-Position: refs/heads/master@{#769952}
parent 33d082e4
...@@ -277,10 +277,8 @@ WebViewPlugin::WebViewHelper::WebViewHelper(WebViewPlugin* plugin, ...@@ -277,10 +277,8 @@ WebViewPlugin::WebViewHelper::WebViewHelper(WebViewPlugin* plugin,
blink::mojom::FrameWidgetHostInterfaceBase>(), blink::mojom::FrameWidgetHostInterfaceBase>(),
blink::CrossVariantMojoAssociatedReceiver< blink::CrossVariantMojoAssociatedReceiver<
blink::mojom::FrameWidgetInterfaceBase>(), blink::mojom::FrameWidgetInterfaceBase>(),
blink::CrossVariantMojoAssociatedRemote< blink_widget_host_receiver_.BindNewEndpointAndPassRemote(),
blink::mojom::WidgetHostInterfaceBase>(), blink_widget_.BindNewEndpointAndPassReceiver());
blink::CrossVariantMojoAssociatedReceiver<
blink::mojom::WidgetInterfaceBase>());
// The WebFrame created here was already attached to the Page as its // The WebFrame created here was already attached to the Page as its
// main frame, and the WebFrameWidget has been initialized, so we can call // main frame, and the WebFrameWidget has been initialized, so we can call
...@@ -311,10 +309,12 @@ blink::WebScreenInfo WebViewPlugin::WebViewHelper::GetScreenInfo() { ...@@ -311,10 +309,12 @@ blink::WebScreenInfo WebViewPlugin::WebViewHelper::GetScreenInfo() {
} }
void WebViewPlugin::WebViewHelper::SetToolTipText( void WebViewPlugin::WebViewHelper::SetToolTipText(
const WebString& text, const base::string16& tooltip_text,
base::i18n::TextDirection hint) { base::i18n::TextDirection hint) {
if (plugin_->container_) if (plugin_->container_) {
plugin_->container_->GetElement().SetAttribute("title", text); plugin_->container_->GetElement().SetAttribute(
"title", WebString::FromUTF16(tooltip_text));
}
} }
void WebViewPlugin::WebViewHelper::StartDragging(network::mojom::ReferrerPolicy, void WebViewPlugin::WebViewHelper::StartDragging(network::mojom::ReferrerPolicy,
......
...@@ -11,7 +11,10 @@ ...@@ -11,7 +11,10 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/sequenced_task_runner_helpers.h" #include "base/sequenced_task_runner_helpers.h"
#include "content/public/renderer/render_view_observer.h" #include "content/public/renderer/render_view_observer.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "third_party/blink/public/mojom/input/focus_type.mojom-forward.h" #include "third_party/blink/public/mojom/input/focus_type.mojom-forward.h"
#include "third_party/blink/public/mojom/page/widget.mojom.h"
#include "third_party/blink/public/platform/web_string.h" #include "third_party/blink/public/platform/web_string.h"
#include "third_party/blink/public/platform/web_url_response.h" #include "third_party/blink/public/platform/web_url_response.h"
#include "third_party/blink/public/web/blink.h" #include "third_party/blink/public/web/blink.h"
...@@ -154,7 +157,8 @@ class WebViewPlugin : public blink::WebPlugin, ...@@ -154,7 +157,8 @@ class WebViewPlugin : public blink::WebPlugin,
// A helper that handles interaction from WebViewPlugin's internal WebView. // A helper that handles interaction from WebViewPlugin's internal WebView.
class WebViewHelper : public blink::WebViewClient, class WebViewHelper : public blink::WebViewClient,
public blink::WebWidgetClient, public blink::WebWidgetClient,
public blink::WebLocalFrameClient { public blink::WebLocalFrameClient,
public blink::mojom::WidgetHost {
public: public:
WebViewHelper(WebViewPlugin* plugin, WebViewHelper(WebViewPlugin* plugin,
const content::WebPreferences& preferences); const content::WebPreferences& preferences);
...@@ -171,8 +175,6 @@ class WebViewPlugin : public blink::WebPlugin, ...@@ -171,8 +175,6 @@ class WebViewPlugin : public blink::WebPlugin,
void DidInvalidateRect(const blink::WebRect&) override; void DidInvalidateRect(const blink::WebRect&) override;
// WebWidgetClient methods: // WebWidgetClient methods:
void SetToolTipText(const blink::WebString&,
base::i18n::TextDirection) override;
void StartDragging(network::mojom::ReferrerPolicy, void StartDragging(network::mojom::ReferrerPolicy,
const blink::WebDragData&, const blink::WebDragData&,
blink::WebDragOperationsMask, blink::WebDragOperationsMask,
...@@ -188,12 +190,21 @@ class WebViewPlugin : public blink::WebPlugin, ...@@ -188,12 +190,21 @@ class WebViewPlugin : public blink::WebPlugin,
std::unique_ptr<blink::WebURLLoaderFactory> CreateURLLoaderFactory() std::unique_ptr<blink::WebURLLoaderFactory> CreateURLLoaderFactory()
override; override;
// blink::mojom::WidgetHost implementation.
void SetCursor(const ui::Cursor& cursor) override {}
void SetToolTipText(const base::string16& tooltip_text,
base::i18n::TextDirection hint) override;
private: private:
WebViewPlugin* plugin_; WebViewPlugin* plugin_;
blink::WebNavigationControl* frame_ = nullptr; blink::WebNavigationControl* frame_ = nullptr;
// Owned by us, deleted via |close()|. // Owned by us, deleted via |close()|.
blink::WebView* web_view_; blink::WebView* web_view_;
mojo::AssociatedReceiver<blink::mojom::WidgetHost>
blink_widget_host_receiver_{this};
mojo::AssociatedRemote<blink::mojom::Widget> blink_widget_;
}; };
WebViewHelper web_view_helper_; WebViewHelper web_view_helper_;
......
...@@ -1752,9 +1752,11 @@ std::string TestRunnerBindings::PlatformName() { ...@@ -1752,9 +1752,11 @@ std::string TestRunnerBindings::PlatformName() {
} }
std::string TestRunnerBindings::TooltipText() { std::string TestRunnerBindings::TooltipText() {
if (runner_) blink::WebString tooltip_text = static_cast<RenderFrameImpl*>(frame_)
return runner_->tooltip_text_; ->GetLocalRootRenderWidget()
return std::string(); ->GetWebWidget()
->GetLastToolTipTextForTesting();
return tooltip_text.Utf8();
} }
int TestRunnerBindings::WebHistoryItemCount() { int TestRunnerBindings::WebHistoryItemCount() {
...@@ -1912,7 +1914,6 @@ void TestRunner::Reset() { ...@@ -1912,7 +1914,6 @@ void TestRunner::Reset() {
clear_referrer_ = false; clear_referrer_ = false;
platform_name_ = "chromium"; platform_name_ = "chromium";
tooltip_text_ = std::string();
web_history_item_count_ = 0; web_history_item_count_ = 0;
weak_factory_.InvalidateWeakPtrs(); weak_factory_.InvalidateWeakPtrs();
...@@ -2259,10 +2260,6 @@ bool TestRunner::PolicyDelegateShouldNotifyDone() const { ...@@ -2259,10 +2260,6 @@ bool TestRunner::PolicyDelegateShouldNotifyDone() const {
return web_test_runtime_flags_.policy_delegate_should_notify_done(); return web_test_runtime_flags_.policy_delegate_should_notify_done();
} }
void TestRunner::SetToolTipText(const blink::WebString& text) {
tooltip_text_ = text.Utf8();
}
void TestRunner::SetDragImage(const SkBitmap& drag_image) { void TestRunner::SetDragImage(const SkBitmap& drag_image) {
if (web_test_runtime_flags_.dump_drag_image()) { if (web_test_runtime_flags_.dump_drag_image()) {
if (drag_image_.isNull()) if (drag_image_.isNull())
......
...@@ -559,9 +559,6 @@ class TestRunner { ...@@ -559,9 +559,6 @@ class TestRunner {
// Bound variable to return the name of this platform (chromium). // Bound variable to return the name of this platform (chromium).
std::string platform_name_; std::string platform_name_;
// Bound variable to store the last tooltip text
std::string tooltip_text_;
// Bound variable counting the number of top URLs visited. // Bound variable counting the number of top URLs visited.
int web_history_item_count_ = 0; int web_history_item_count_ = 0;
......
...@@ -114,12 +114,6 @@ bool WebWidgetTestProxy::IsPointerLocked() { ...@@ -114,12 +114,6 @@ bool WebWidgetTestProxy::IsPointerLocked() {
return GetViewTestRunner()->isPointerLocked(); return GetViewTestRunner()->isPointerLocked();
} }
void WebWidgetTestProxy::SetToolTipText(const blink::WebString& text,
base::i18n::TextDirection hint) {
RenderWidget::SetToolTipText(text, hint);
GetTestRunner()->SetToolTipText(text);
}
void WebWidgetTestProxy::StartDragging(network::mojom::ReferrerPolicy policy, void WebWidgetTestProxy::StartDragging(network::mojom::ReferrerPolicy policy,
const blink::WebDragData& data, const blink::WebDragData& data,
blink::WebDragOperationsMask mask, blink::WebDragOperationsMask mask,
......
...@@ -17,7 +17,6 @@ ...@@ -17,7 +17,6 @@
namespace blink { namespace blink {
class WebLocalFrame; class WebLocalFrame;
class WebString;
} // namespace blink } // namespace blink
namespace content { namespace content {
...@@ -70,8 +69,6 @@ class WebWidgetTestProxy : public RenderWidget { ...@@ -70,8 +69,6 @@ class WebWidgetTestProxy : public RenderWidget {
bool request_unajusted_movement) override; bool request_unajusted_movement) override;
void RequestPointerUnlock() override; void RequestPointerUnlock() override;
bool IsPointerLocked() override; bool IsPointerLocked() override;
void SetToolTipText(const blink::WebString& text,
base::i18n::TextDirection hint) override;
void StartDragging(network::mojom::ReferrerPolicy policy, void StartDragging(network::mojom::ReferrerPolicy policy,
const blink::WebDragData& data, const blink::WebDragData& data,
blink::WebDragOperationsMask mask, blink::WebDragOperationsMask mask,
......
...@@ -186,6 +186,9 @@ class WebWidget { ...@@ -186,6 +186,9 @@ class WebWidget {
virtual void SetCursor(const ui::Cursor& cursor) = 0; virtual void SetCursor(const ui::Cursor& cursor) = 0;
// Get the current tooltip text.
virtual WebString GetLastToolTipTextForTesting() const { return WebString(); }
protected: protected:
~WebWidget() = default; ~WebWidget() = default;
}; };
......
...@@ -69,7 +69,6 @@ namespace blink { ...@@ -69,7 +69,6 @@ namespace blink {
class WebDragData; class WebDragData;
class WebGestureEvent; class WebGestureEvent;
struct WebFloatRect; struct WebFloatRect;
class WebString;
class WebWidget; class WebWidget;
class WebLocalFrame; class WebLocalFrame;
...@@ -107,10 +106,6 @@ class WebWidgetClient { ...@@ -107,10 +106,6 @@ class WebWidgetClient {
// content view area, i.e. doesn't include any window decorations. // content view area, i.e. doesn't include any window decorations.
virtual WebRect ViewRect() { return WebRect(); } virtual WebRect ViewRect() { return WebRect(); }
// Called when a tooltip should be shown at the current cursor position.
virtual void SetToolTipText(const WebString&,
base::i18n::TextDirection hint) {}
// Requests to lock the mouse cursor for the |requester_frame| in the // Requests to lock the mouse cursor for the |requester_frame| in the
// widget. If true is returned, the success result will be asynchronously // widget. If true is returned, the success result will be asynchronously
// returned via a single call to WebWidget::didAcquirePointerLock() or // returned via a single call to WebWidget::didAcquirePointerLock() or
......
...@@ -217,10 +217,7 @@ class PagePopupChromeClient final : public EmptyChromeClient { ...@@ -217,10 +217,7 @@ class PagePopupChromeClient final : public EmptyChromeClient {
void SetToolTip(LocalFrame&, void SetToolTip(LocalFrame&,
const String& tooltip_text, const String& tooltip_text,
TextDirection dir) override { TextDirection dir) override {
if (popup_->WidgetClient()) { popup_->widget_base_->SetToolTipText(tooltip_text, dir);
popup_->WidgetClient()->SetToolTipText(tooltip_text,
ToBaseTextDirection(dir));
}
} }
void InjectGestureScrollEvent(LocalFrame& local_frame, void InjectGestureScrollEvent(LocalFrame& local_frame,
......
...@@ -921,13 +921,7 @@ WebFrameWidgetBase::RendererWidgetSchedulingState() { ...@@ -921,13 +921,7 @@ WebFrameWidgetBase::RendererWidgetSchedulingState() {
void WebFrameWidgetBase::SetToolTipText(const String& tooltip_text, void WebFrameWidgetBase::SetToolTipText(const String& tooltip_text,
TextDirection dir) { TextDirection dir) {
widget_base_->GetWidgetHostRemote()->SetToolTipText( widget_base_->SetToolTipText(tooltip_text, dir);
tooltip_text.IsEmpty() ? "" : tooltip_text,
dir == TextDirection::kLtr
? mojo_base::mojom::blink::TextDirection::LEFT_TO_RIGHT
: mojo_base::mojom::blink::TextDirection::RIGHT_TO_LEFT);
Client()->SetToolTipText(tooltip_text, ToBaseTextDirection(dir));
} }
} // namespace blink } // namespace blink
...@@ -183,6 +183,10 @@ WebURL WebViewFrameWidget::GetURLForDebugTrace() { ...@@ -183,6 +183,10 @@ WebURL WebViewFrameWidget::GetURLForDebugTrace() {
return web_view_->GetURLForDebugTrace(); return web_view_->GetURLForDebugTrace();
} }
WebString WebViewFrameWidget::GetLastToolTipTextForTesting() const {
return GetPage()->GetChromeClient().GetLastToolTipTextForTesting();
}
void WebViewFrameWidget::DidDetachLocalFrameTree() { void WebViewFrameWidget::DidDetachLocalFrameTree() {
web_view_->DidDetachLocalMainFrame(); web_view_->DidDetachLocalMainFrame();
} }
......
...@@ -70,6 +70,7 @@ class CORE_EXPORT WebViewFrameWidget : public WebFrameWidgetBase { ...@@ -70,6 +70,7 @@ class CORE_EXPORT WebViewFrameWidget : public WebFrameWidgetBase {
void SetFocus(bool) override; void SetFocus(bool) override;
bool SelectionBounds(WebRect& anchor, WebRect& focus) const override; bool SelectionBounds(WebRect& anchor, WebRect& focus) const override;
WebURL GetURLForDebugTrace() override; WebURL GetURLForDebugTrace() override;
WebString GetLastToolTipTextForTesting() const override;
// WebFrameWidget overrides: // WebFrameWidget overrides:
void DidDetachLocalFrameTree() override; void DidDetachLocalFrameTree() override;
......
...@@ -261,11 +261,13 @@ void ChromeClient::SetToolTip(LocalFrame& frame, ...@@ -261,11 +261,13 @@ void ChromeClient::SetToolTip(LocalFrame& frame,
last_tool_tip_point_ = location.Point(); last_tool_tip_point_ = location.Point();
last_tool_tip_text_ = tool_tip; last_tool_tip_text_ = tool_tip;
last_mouse_over_node_ = result.InnerNodeOrImageMapImage(); last_mouse_over_node_ = result.InnerNodeOrImageMapImage();
current_tool_tip_text_for_test_ = last_tool_tip_text_;
SetToolTip(frame, tool_tip, tool_tip_direction); SetToolTip(frame, tool_tip, tool_tip_direction);
} }
void ChromeClient::ClearToolTip(LocalFrame& frame) { void ChromeClient::ClearToolTip(LocalFrame& frame) {
// Do not check m_lastToolTip* and do not update them intentionally. current_tool_tip_text_for_test_ = String();
// Do not check last_tool_tip_* and do not update them intentionally.
// We don't want to show tooltips with same content after clearToolTip(). // We don't want to show tooltips with same content after clearToolTip().
SetToolTip(frame, String(), TextDirection::kLtr); SetToolTip(frame, String(), TextDirection::kLtr);
} }
......
...@@ -313,6 +313,9 @@ class CORE_EXPORT ChromeClient : public GarbageCollected<ChromeClient> { ...@@ -313,6 +313,9 @@ class CORE_EXPORT ChromeClient : public GarbageCollected<ChromeClient> {
const HitTestResult&); const HitTestResult&);
virtual void SetToolTip(LocalFrame&, const String&, TextDirection) = 0; virtual void SetToolTip(LocalFrame&, const String&, TextDirection) = 0;
void ClearToolTip(LocalFrame&); void ClearToolTip(LocalFrame&);
String GetLastToolTipTextForTesting() {
return current_tool_tip_text_for_test_;
}
bool Print(LocalFrame*); bool Print(LocalFrame*);
...@@ -537,6 +540,9 @@ class CORE_EXPORT ChromeClient : public GarbageCollected<ChromeClient> { ...@@ -537,6 +540,9 @@ class CORE_EXPORT ChromeClient : public GarbageCollected<ChromeClient> {
WeakMember<Node> last_mouse_over_node_; WeakMember<Node> last_mouse_over_node_;
PhysicalOffset last_tool_tip_point_; PhysicalOffset last_tool_tip_point_;
String last_tool_tip_text_; String last_tool_tip_text_;
// |last_tool_tip_text_| is kept even if ClearToolTip is called. This is for
// the tooltip text that is cleared when ClearToolTip is called.
String current_tool_tip_text_for_test_;
FRIEND_TEST_ALL_PREFIXES(ChromeClientTest, SetToolTipFlood); FRIEND_TEST_ALL_PREFIXES(ChromeClientTest, SetToolTipFlood);
FRIEND_TEST_ALL_PREFIXES(ChromeClientTest, SetToolTipEmptyString); FRIEND_TEST_ALL_PREFIXES(ChromeClientTest, SetToolTipEmptyString);
......
...@@ -236,4 +236,12 @@ void WidgetBase::SetCursor(const ui::Cursor& cursor) { ...@@ -236,4 +236,12 @@ void WidgetBase::SetCursor(const ui::Cursor& cursor) {
widget_host_->SetCursor(cursor); widget_host_->SetCursor(cursor);
} }
void WidgetBase::SetToolTipText(const String& tooltip_text, TextDirection dir) {
widget_host_->SetToolTipText(
tooltip_text.IsEmpty() ? "" : tooltip_text,
dir == TextDirection::kLtr
? mojo_base::mojom::blink::TextDirection::LEFT_TO_RIGHT
: mojo_base::mojom::blink::TextDirection::RIGHT_TO_LEFT);
}
} // namespace blink } // namespace blink
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "third_party/blink/public/platform/cross_variant_mojo_util.h" #include "third_party/blink/public/platform/cross_variant_mojo_util.h"
#include "third_party/blink/public/web/web_widget.h" #include "third_party/blink/public/web/web_widget.h"
#include "third_party/blink/renderer/platform/platform_export.h" #include "third_party/blink/renderer/platform/platform_export.h"
#include "third_party/blink/renderer/platform/text/text_direction.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h" #include "third_party/blink/renderer/platform/weborigin/kurl.h"
#include "third_party/blink/renderer/platform/widget/compositing/layer_tree_view_delegate.h" #include "third_party/blink/renderer/platform/widget/compositing/layer_tree_view_delegate.h"
...@@ -115,6 +116,8 @@ class PLATFORM_EXPORT WidgetBase : public mojom::blink::Widget, ...@@ -115,6 +116,8 @@ class PLATFORM_EXPORT WidgetBase : public mojom::blink::Widget,
void SetCursor(const ui::Cursor& cursor); void SetCursor(const ui::Cursor& cursor);
void SetToolTipText(const String& tooltip_text, TextDirection dir);
private: private:
std::unique_ptr<LayerTreeView> layer_tree_view_; std::unique_ptr<LayerTreeView> layer_tree_view_;
WidgetBaseClient* client_; WidgetBaseClient* client_;
......
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