Commit a5289b95 authored by Piotr Kalinowski's avatar Piotr Kalinowski Committed by Commit Bot

Route ShowContextMenuAtPoint correctly for emulated events

RWHIER::ShowContextMenutAtPoint used last_mouse_move_target_ to
determine RenderWidgetHostImpl instance to forward the call to. It
acknowledged that tracked view may have been destroyed since remembering
it, and it first checked if it was in view map.

This check used to fail for nullptr without crashing leading to an early
return. However, IsViewInMap implementation changed since then and now
if there were no mouse enter/leave events earlier, the browser would
crash trying to dereference nullptr.

This happens when devtools on the desktop connect to a mobile instance
of the browser and forward a right click event on the page.
TouchEmulator will convert it directly to the call to show context menu,
but since remote devtools do not forward mouse move events, as the
mobile does not even have a mouse, this variable will be empty.

However, TouchEmulator already has a target view associated with
emulated touch event that is being converted to context menu request. So
pass that along and use it.

Bug: 987665
Change-Id: I1bf8012827e9397f911a3c12efc984815ad37901
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1718571Reviewed-by: default avatarJames MacLean <wjmaclean@chromium.org>
Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682308}
parent 6b153fb6
...@@ -187,7 +187,7 @@ bool TouchEmulator::HandleMouseEvent(const WebMouseEvent& mouse_event, ...@@ -187,7 +187,7 @@ bool TouchEmulator::HandleMouseEvent(const WebMouseEvent& mouse_event,
client_->ShowContextMenuAtPoint( client_->ShowContextMenuAtPoint(
gfx::Point(mouse_event.PositionInWidget().x, gfx::Point(mouse_event.PositionInWidget().x,
mouse_event.PositionInWidget().y), mouse_event.PositionInWidget().y),
ui::MENU_SOURCE_MOUSE); ui::MENU_SOURCE_MOUSE, target_view);
} }
if (mouse_event.button != WebMouseEvent::Button::kLeft) if (mouse_event.button != WebMouseEvent::Button::kLeft)
......
...@@ -25,8 +25,10 @@ class CONTENT_EXPORT TouchEmulatorClient { ...@@ -25,8 +25,10 @@ class CONTENT_EXPORT TouchEmulatorClient {
virtual void ForwardEmulatedTouchEvent(const blink::WebTouchEvent& event, virtual void ForwardEmulatedTouchEvent(const blink::WebTouchEvent& event,
RenderWidgetHostViewBase* target) = 0; RenderWidgetHostViewBase* target) = 0;
virtual void SetCursor(const WebCursor& cursor) = 0; virtual void SetCursor(const WebCursor& cursor) = 0;
// |target| is the view associated with the corresponding input event.
virtual void ShowContextMenuAtPoint(const gfx::Point& point, virtual void ShowContextMenuAtPoint(const gfx::Point& point,
const ui::MenuSourceType source_type) = 0; const ui::MenuSourceType source_type,
RenderWidgetHostViewBase* target) = 0;
}; };
} // namespace content } // namespace content
......
...@@ -89,7 +89,8 @@ class TouchEmulatorTest : public testing::Test, ...@@ -89,7 +89,8 @@ class TouchEmulatorTest : public testing::Test,
} }
void ShowContextMenuAtPoint(const gfx::Point& point, void ShowContextMenuAtPoint(const gfx::Point& point,
const ui::MenuSourceType source_type) override {} const ui::MenuSourceType source_type,
RenderWidgetHostViewBase* target) override {}
protected: protected:
TouchEmulator* emulator() const { TouchEmulator* emulator() const {
......
...@@ -1272,11 +1272,6 @@ void RenderWidgetHostImpl::WaitForInputProcessed(base::OnceClosure callback) { ...@@ -1272,11 +1272,6 @@ void RenderWidgetHostImpl::WaitForInputProcessed(base::OnceClosure callback) {
input_router_->WaitForInputProcessed(std::move(callback)); input_router_->WaitForInputProcessed(std::move(callback));
} }
void RenderWidgetHostImpl::ForwardEmulatedGestureEvent(
const blink::WebGestureEvent& gesture_event) {
ForwardGestureEvent(gesture_event);
}
void RenderWidgetHostImpl::ForwardGestureEvent( void RenderWidgetHostImpl::ForwardGestureEvent(
const blink::WebGestureEvent& gesture_event) { const blink::WebGestureEvent& gesture_event) {
ForwardGestureEventWithLatencyInfo( ForwardGestureEventWithLatencyInfo(
...@@ -1390,14 +1385,6 @@ void RenderWidgetHostImpl::ForwardGestureEventWithLatencyInfo( ...@@ -1390,14 +1385,6 @@ void RenderWidgetHostImpl::ForwardGestureEventWithLatencyInfo(
} }
} }
void RenderWidgetHostImpl::ForwardEmulatedTouchEvent(
const blink::WebTouchEvent& touch_event,
RenderWidgetHostViewBase* target) {
TRACE_EVENT0("input", "RenderWidgetHostImpl::ForwardEmulatedTouchEvent");
ForwardTouchEventWithLatencyInfo(touch_event,
ui::LatencyInfo(ui::SourceEventType::TOUCH));
}
void RenderWidgetHostImpl::ForwardTouchEventWithLatencyInfo( void RenderWidgetHostImpl::ForwardTouchEventWithLatencyInfo(
const blink::WebTouchEvent& touch_event, const blink::WebTouchEvent& touch_event,
const ui::LatencyInfo& latency) { const ui::LatencyInfo& latency) {
......
...@@ -127,7 +127,6 @@ class CONTENT_EXPORT RenderWidgetHostImpl ...@@ -127,7 +127,6 @@ class CONTENT_EXPORT RenderWidgetHostImpl
public InputDispositionHandler, public InputDispositionHandler,
public RenderProcessHostImpl::PriorityClient, public RenderProcessHostImpl::PriorityClient,
public RenderProcessHostObserver, public RenderProcessHostObserver,
public TouchEmulatorClient,
public SyntheticGestureController::Delegate, public SyntheticGestureController::Delegate,
public viz::mojom::CompositorFrameSink, public viz::mojom::CompositorFrameSink,
public IPC::Listener, public IPC::Listener,
...@@ -439,14 +438,9 @@ class CONTENT_EXPORT RenderWidgetHostImpl ...@@ -439,14 +438,9 @@ class CONTENT_EXPORT RenderWidgetHostImpl
// Returns an emulator for this widget. See TouchEmulator for more details. // Returns an emulator for this widget. See TouchEmulator for more details.
TouchEmulator* GetTouchEmulator(); TouchEmulator* GetTouchEmulator();
// TouchEmulatorClient implementation. void SetCursor(const WebCursor& cursor);
void ForwardEmulatedGestureEvent(
const blink::WebGestureEvent& gesture_event) override;
void ForwardEmulatedTouchEvent(const blink::WebTouchEvent& touch_event,
RenderWidgetHostViewBase* target) override;
void SetCursor(const WebCursor& cursor) override;
void ShowContextMenuAtPoint(const gfx::Point& point, void ShowContextMenuAtPoint(const gfx::Point& point,
const ui::MenuSourceType source_type) override; const ui::MenuSourceType source_type);
// Queues a synthetic gesture for testing purposes. Invokes the on_complete // Queues a synthetic gesture for testing purposes. Invokes the on_complete
// callback when the gesture is finished running. // callback when the gesture is finished running.
......
...@@ -1933,15 +1933,11 @@ void RenderWidgetHostInputEventRouter::SetCursor(const WebCursor& cursor) { ...@@ -1933,15 +1933,11 @@ void RenderWidgetHostInputEventRouter::SetCursor(const WebCursor& cursor) {
void RenderWidgetHostInputEventRouter::ShowContextMenuAtPoint( void RenderWidgetHostInputEventRouter::ShowContextMenuAtPoint(
const gfx::Point& point, const gfx::Point& point,
const ui::MenuSourceType source_type) { const ui::MenuSourceType source_type,
// It's possible that since |last_mouse_move_target_| was set by the RenderWidgetHostViewBase* target) {
// outbound mouse event that the view may have gone away. Before dispatching DCHECK(IsViewInMap(target));
// the context menu, confirm the view is still available. auto* rwhi =
if (!IsViewInMap(last_mouse_move_target_)) static_cast<RenderWidgetHostImpl*>(target->GetRenderWidgetHost());
return;
auto* rwhi = static_cast<RenderWidgetHostImpl*>(
last_mouse_move_target_->GetRenderWidgetHost());
DCHECK(rwhi); DCHECK(rwhi);
rwhi->ShowContextMenuAtPoint(point, source_type); rwhi->ShowContextMenuAtPoint(point, source_type);
} }
......
...@@ -172,7 +172,8 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter ...@@ -172,7 +172,8 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter
RenderWidgetHostViewBase* target) override; RenderWidgetHostViewBase* target) override;
void SetCursor(const WebCursor& cursor) override; void SetCursor(const WebCursor& cursor) override;
void ShowContextMenuAtPoint(const gfx::Point& point, void ShowContextMenuAtPoint(const gfx::Point& point,
const ui::MenuSourceType source_type) override; const ui::MenuSourceType source_type,
RenderWidgetHostViewBase* target) override;
// HitTestRegionObserver // HitTestRegionObserver
void OnAggregatedHitTestRegionListUpdated( void OnAggregatedHitTestRegionListUpdated(
......
...@@ -900,4 +900,12 @@ TEST_F(RenderWidgetHostInputEventRouterTest, ...@@ -900,4 +900,12 @@ TEST_F(RenderWidgetHostInputEventRouterTest,
EXPECT_NE(view_root_.get(), bubbling_gesture_scroll_target()); EXPECT_NE(view_root_.get(), bubbling_gesture_scroll_target());
} }
// Calling ShowContextMenuAtPoint without other events will happen when desktop
// devtools connect to a browser instance running on a mobile. It should not
// crash.
TEST_F(RenderWidgetHostInputEventRouterTest, CanCallShowContextMenuAtPoint) {
rwhier()->ShowContextMenuAtPoint(gfx::Point(0, 0), ui::MENU_SOURCE_MOUSE,
view_root_.get());
}
} // namespace content } // namespace content
...@@ -297,7 +297,6 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid ...@@ -297,7 +297,6 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid
bool HasValidFrame() const; bool HasValidFrame() const;
void MoveCaret(const gfx::Point& point); void MoveCaret(const gfx::Point& point);
void ShowContextMenuAtPoint(const gfx::Point& point, ui::MenuSourceType);
void DismissTextHandles(); void DismissTextHandles();
void SetTextHandlesTemporarilyHidden(bool hide_handles); void SetTextHandlesTemporarilyHidden(bool hide_handles);
void SelectWordAroundCaretAck(bool did_select, void SelectWordAroundCaretAck(bool did_select,
......
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