Add requester frame param to mouse-lock calls.
Without a specific frame, we were using the local root of caller's
widget, which was showing wrong activation state in certain cases.
(cherry picked from commit 32a0b159fdace8433876da922b04bba4063f1533)
Bug: 981597
Change-Id: I45a5dd88ea4c487ff4bf3a722e72bb6923fb8319
TBR: [email protected]
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1721186
Commit-Queue: Navid Zolghadr <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Reviewed-by: Alex Moshchuk <[email protected]>
Reviewed-by: Navid Zolghadr <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#684359}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1741297
Cr-Commit-Position: refs/branch-heads/3865@{#283}
Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094}
diff --git a/content/browser/pointer_lock_browsertest.cc b/content/browser/pointer_lock_browsertest.cc
index 2f639b7..c87bf023 100644
--- a/content/browser/pointer_lock_browsertest.cc
+++ b/content/browser/pointer_lock_browsertest.cc
@@ -35,7 +35,7 @@
void RequestToLockMouse(WebContents* web_contents,
bool user_gesture,
bool last_unlocked_by_target) override {
- web_contents->GotResponseToLockMouseRequest(true);
+ web_contents->GotResponseToLockMouseRequest(user_gesture);
}
void LostMouseLock() override {}
@@ -115,7 +115,7 @@
MockPointerLockWebContentsDelegate web_contents_delegate_;
};
-IN_PROC_BROWSER_TEST_F(PointerLockBrowserTest, PointerLock) {
+IN_PROC_BROWSER_TEST_F(PointerLockBrowserTest, PointerLockBasic) {
GURL main_url(embedded_test_server()->GetURL(
"a.com", "/cross_site_iframe_factory.html?a(b)"));
EXPECT_TRUE(NavigateToURL(shell(), main_url));
@@ -147,6 +147,46 @@
EvalJs(child, "document.pointerLockElement == document.body"));
}
+IN_PROC_BROWSER_TEST_F(PointerLockBrowserTest, PointerLockAndUserActivation) {
+ GURL main_url(embedded_test_server()->GetURL(
+ "a.com", "/cross_site_iframe_factory.html?a(b(b))"));
+ EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+ FrameTreeNode* root = web_contents()->GetFrameTree()->root();
+ FrameTreeNode* child = root->child_at(0);
+ FrameTreeNode* grand_child = child->child_at(0);
+
+ // Without user activation, pointer lock request from any (child or
+ // grand_child) frame fails.
+ EXPECT_TRUE(ExecJs(child, "document.body.requestPointerLock()",
+ EXECUTE_SCRIPT_NO_USER_GESTURE));
+ EXPECT_EQ(false, EvalJs(child, "document.pointerLockElement == document.body",
+ EXECUTE_SCRIPT_NO_USER_GESTURE));
+ EXPECT_TRUE(ExecJs(grand_child, "document.body.requestPointerLock()",
+ EXECUTE_SCRIPT_NO_USER_GESTURE));
+ EXPECT_EQ(false,
+ EvalJs(grand_child, "document.pointerLockElement == document.body",
+ EXECUTE_SCRIPT_NO_USER_GESTURE));
+
+ // Execute a empty (dummy) JS to activate the child frame.
+ EXPECT_TRUE(ExecJs(child, ""));
+
+ // With user activation in the child frame, pointer lock from the same frame
+ // succeeds.
+ EXPECT_TRUE(ExecJs(child, "document.body.requestPointerLock()",
+ EXECUTE_SCRIPT_NO_USER_GESTURE));
+ EXPECT_EQ(true, EvalJs(child, "document.pointerLockElement == document.body",
+ EXECUTE_SCRIPT_NO_USER_GESTURE));
+
+ // But with user activation in the child frame, pointer lock from the
+ // grand_child frame fails.
+ EXPECT_TRUE(ExecJs(grand_child, "document.body.requestPointerLock()",
+ EXECUTE_SCRIPT_NO_USER_GESTURE));
+ EXPECT_EQ(false,
+ EvalJs(grand_child, "document.pointerLockElement == document.body",
+ EXECUTE_SCRIPT_NO_USER_GESTURE));
+}
+
IN_PROC_BROWSER_TEST_F(PointerLockBrowserTest, PointerLockEventRouting) {
GURL main_url(embedded_test_server()->GetURL(
"a.com", "/cross_site_iframe_factory.html?a(b)"));
diff --git a/content/renderer/browser_plugin/browser_plugin.cc b/content/renderer/browser_plugin/browser_plugin.cc
index 6f66951..8f9051c 100644
--- a/content/renderer/browser_plugin/browser_plugin.cc
+++ b/content/renderer/browser_plugin/browser_plugin.cc
@@ -341,7 +341,9 @@
if (enable) {
if (mouse_locked_ || !render_widget)
return;
- render_widget->mouse_lock_dispatcher()->LockMouse(this);
+ blink::WebLocalFrame* requester_frame =
+ Container()->GetDocument().GetFrame();
+ render_widget->mouse_lock_dispatcher()->LockMouse(this, requester_frame);
} else {
if (!mouse_locked_) {
OnLockMouseACK(false);
diff --git a/content/renderer/mouse_lock_dispatcher.cc b/content/renderer/mouse_lock_dispatcher.cc
index 0749c9c..84c06db5 100644
--- a/content/renderer/mouse_lock_dispatcher.cc
+++ b/content/renderer/mouse_lock_dispatcher.cc
@@ -18,14 +18,15 @@
MouseLockDispatcher::~MouseLockDispatcher() {
}
-bool MouseLockDispatcher::LockMouse(LockTarget* target) {
+bool MouseLockDispatcher::LockMouse(LockTarget* target,
+ blink::WebLocalFrame* requester_frame) {
if (MouseLockedOrPendingAction())
return false;
pending_lock_request_ = true;
target_ = target;
- SendLockMouseRequest();
+ SendLockMouseRequest(requester_frame);
return true;
}
diff --git a/content/renderer/mouse_lock_dispatcher.h b/content/renderer/mouse_lock_dispatcher.h
index d5c3be8..0e76aa6 100644
--- a/content/renderer/mouse_lock_dispatcher.h
+++ b/content/renderer/mouse_lock_dispatcher.h
@@ -10,6 +10,7 @@
namespace blink {
class WebMouseEvent;
+class WebLocalFrame;
} // namespace blink
namespace content {
@@ -31,9 +32,10 @@
const blink::WebMouseEvent& event) = 0;
};
- // Locks the mouse to the |target|. If true is returned, an asynchronous
- // response to target->OnLockMouseACK() will follow.
- bool LockMouse(LockTarget* target);
+ // Locks the mouse to |target| if |requester_frame| has transient user
+ // activation. If true is returned, an asynchronous response to
+ // target->OnLockMouseACK() will follow.
+ bool LockMouse(LockTarget* target, blink::WebLocalFrame* requester_frame);
// Request to unlock the mouse. An asynchronous response to
// target->OnMouseLockLost() will follow.
void UnlockMouse(LockTarget* target);
@@ -55,7 +57,7 @@
protected:
// Subclasses must implement these methods to send mouse lock requests to the
// browser.
- virtual void SendLockMouseRequest() = 0;
+ virtual void SendLockMouseRequest(blink::WebLocalFrame* requester_frame) = 0;
virtual void SendUnlockMouseRequest() = 0;
private:
diff --git a/content/renderer/mouse_lock_dispatcher_browsertest.cc b/content/renderer/mouse_lock_dispatcher_browsertest.cc
index 48936a8..059d7f0 100644
--- a/content/renderer/mouse_lock_dispatcher_browsertest.cc
+++ b/content/renderer/mouse_lock_dispatcher_browsertest.cc
@@ -55,13 +55,14 @@
} // namespace
-// Test simple use of RenderViewImpl interface to WebKit for pointer lock.
+// Test simple use of RenderViewImpl interface for pointer lock.
TEST_F(MouseLockDispatcherTest, BasicWebWidget) {
// Start unlocked.
EXPECT_FALSE(widget()->IsPointerLocked());
// Lock.
- EXPECT_TRUE(widget()->RequestPointerLock());
+ EXPECT_TRUE(widget()->RequestPointerLock(
+ view()->GetMainRenderFrame()->GetWebFrame()));
widget()->OnMessageReceived(WidgetMsg_LockMouse_ACK(route_id_, true));
EXPECT_TRUE(widget()->IsPointerLocked());
@@ -71,7 +72,8 @@
EXPECT_FALSE(widget()->IsPointerLocked());
// Attempt a lock, and have it fail.
- EXPECT_TRUE(widget()->RequestPointerLock());
+ EXPECT_TRUE(widget()->RequestPointerLock(
+ view()->GetMainRenderFrame()->GetWebFrame()));
widget()->OnMessageReceived(WidgetMsg_LockMouse_ACK(route_id_, false));
EXPECT_FALSE(widget()->IsPointerLocked());
}
@@ -89,7 +91,8 @@
EXPECT_FALSE(dispatcher()->IsMouseLockedTo(target_));
// Lock.
- EXPECT_TRUE(dispatcher()->LockMouse(target_));
+ EXPECT_TRUE(dispatcher()->LockMouse(
+ target_, view()->GetMainRenderFrame()->GetWebFrame()));
widget()->OnMessageReceived(WidgetMsg_LockMouse_ACK(route_id_, true));
EXPECT_TRUE(dispatcher()->IsMouseLockedTo(target_));
@@ -102,7 +105,8 @@
EXPECT_FALSE(dispatcher()->IsMouseLockedTo(target_));
// Attempt a lock, and have it fail.
- EXPECT_TRUE(dispatcher()->LockMouse(target_));
+ EXPECT_TRUE(dispatcher()->LockMouse(
+ target_, view()->GetMainRenderFrame()->GetWebFrame()));
widget()->OnMessageReceived(WidgetMsg_LockMouse_ACK(route_id_, false));
EXPECT_FALSE(dispatcher()->IsMouseLockedTo(target_));
}
@@ -115,7 +119,8 @@
EXPECT_CALL(*target_, OnMouseLockLost()).Times(0);
// Lock.
- EXPECT_TRUE(dispatcher()->LockMouse(target_));
+ EXPECT_TRUE(dispatcher()->LockMouse(
+ target_, view()->GetMainRenderFrame()->GetWebFrame()));
widget()->OnMessageReceived(WidgetMsg_LockMouse_ACK(route_id_, true));
EXPECT_TRUE(dispatcher()->IsMouseLockedTo(target_));
@@ -136,7 +141,8 @@
EXPECT_CALL(*target_, OnMouseLockLost()).Times(0);
// Lock request.
- EXPECT_TRUE(dispatcher()->LockMouse(target_));
+ EXPECT_TRUE(dispatcher()->LockMouse(
+ target_, view()->GetMainRenderFrame()->GetWebFrame()));
// Before receiving response delete the target.
dispatcher()->OnLockTargetDestroyed(target_);
@@ -154,7 +160,8 @@
EXPECT_CALL(*target_, OnMouseLockLost()).Times(0);
// Lock request.
- EXPECT_TRUE(dispatcher()->LockMouse(target_));
+ EXPECT_TRUE(dispatcher()->LockMouse(
+ target_, view()->GetMainRenderFrame()->GetWebFrame()));
// Before receiving response delete the target.
dispatcher()->OnLockTargetDestroyed(target_);
@@ -178,7 +185,8 @@
dispatcher()->WillHandleMouseEvent(blink::WebMouseEvent());
// Lock.
- EXPECT_TRUE(dispatcher()->LockMouse(target_));
+ EXPECT_TRUE(dispatcher()->LockMouse(
+ target_, view()->GetMainRenderFrame()->GetWebFrame()));
widget()->OnMessageReceived(WidgetMsg_LockMouse_ACK(route_id_, true));
EXPECT_TRUE(dispatcher()->IsMouseLockedTo(target_));
@@ -205,11 +213,13 @@
EXPECT_CALL(*target_, OnMouseLockLost());
// Lock request for target.
- EXPECT_TRUE(dispatcher()->LockMouse(target_));
+ EXPECT_TRUE(dispatcher()->LockMouse(
+ target_, view()->GetMainRenderFrame()->GetWebFrame()));
// Fail attempt to lock alternate.
EXPECT_FALSE(dispatcher()->IsMouseLockedTo(alternate_target_));
- EXPECT_FALSE(dispatcher()->LockMouse(alternate_target_));
+ EXPECT_FALSE(dispatcher()->LockMouse(
+ alternate_target_, view()->GetMainRenderFrame()->GetWebFrame()));
// Lock completion for target.
widget()->OnMessageReceived(WidgetMsg_LockMouse_ACK(route_id_, true));
@@ -217,7 +227,8 @@
// Fail attempt to lock alternate.
EXPECT_FALSE(dispatcher()->IsMouseLockedTo(alternate_target_));
- EXPECT_FALSE(dispatcher()->LockMouse(alternate_target_));
+ EXPECT_FALSE(dispatcher()->LockMouse(
+ alternate_target_, view()->GetMainRenderFrame()->GetWebFrame()));
// Receive mouse event to only one target.
dispatcher()->WillHandleMouseEvent(blink::WebMouseEvent());
diff --git a/content/renderer/pepper/pepper_plugin_instance_impl.cc b/content/renderer/pepper/pepper_plugin_instance_impl.cc
index 7a5cd52..5b5a840 100644
--- a/content/renderer/pepper/pepper_plugin_instance_impl.cc
+++ b/content/renderer/pepper/pepper_plugin_instance_impl.cc
@@ -3268,7 +3268,9 @@
}
bool PepperPluginInstanceImpl::LockMouse() {
- return GetMouseLockDispatcher()->LockMouse(GetOrCreateLockTargetAdapter());
+ WebLocalFrame* requester_frame = container_->GetDocument().GetFrame();
+ return GetMouseLockDispatcher()->LockMouse(GetOrCreateLockTargetAdapter(),
+ requester_frame);
}
MouseLockDispatcher::LockTarget*
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index 734c561..9640295 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -3650,8 +3650,9 @@
return gfx::ToRoundedPoint(ConvertWindowPointToViewport(gfx::PointF(point)));
}
-bool RenderWidget::RequestPointerLock() {
- return mouse_lock_dispatcher_->LockMouse(webwidget_mouse_lock_target_.get());
+bool RenderWidget::RequestPointerLock(WebLocalFrame* requester_frame) {
+ return mouse_lock_dispatcher_->LockMouse(webwidget_mouse_lock_target_.get(),
+ requester_frame);
}
void RenderWidget::RequestPointerUnlock() {
diff --git a/content/renderer/render_widget.h b/content/renderer/render_widget.h
index 20f02c0b..62588c09 100644
--- a/content/renderer/render_widget.h
+++ b/content/renderer/render_widget.h
@@ -420,7 +420,7 @@
void ConvertViewportToWindow(blink::WebRect* rect) override;
void ConvertViewportToWindow(blink::WebFloatRect* rect) override;
void ConvertWindowToViewport(blink::WebFloatRect* rect) override;
- bool RequestPointerLock() override;
+ bool RequestPointerLock(blink::WebLocalFrame* requester_frame) override;
void RequestPointerUnlock() override;
bool IsPointerLocked() override;
void StartDragging(network::mojom::ReferrerPolicy policy,
diff --git a/content/renderer/render_widget_fullscreen_pepper.cc b/content/renderer/render_widget_fullscreen_pepper.cc
index 06e9fd5..cc9ed31 100644
--- a/content/renderer/render_widget_fullscreen_pepper.cc
+++ b/content/renderer/render_widget_fullscreen_pepper.cc
@@ -57,7 +57,7 @@
private:
// MouseLockDispatcher implementation.
- void SendLockMouseRequest() override;
+ void SendLockMouseRequest(blink::WebLocalFrame* requester_frame) override;
void SendUnlockMouseRequest() override;
RenderWidgetFullscreenPepper* widget_;
@@ -116,7 +116,11 @@
FullscreenMouseLockDispatcher::~FullscreenMouseLockDispatcher() {
}
-void FullscreenMouseLockDispatcher::SendLockMouseRequest() {
+void FullscreenMouseLockDispatcher::SendLockMouseRequest(
+ blink::WebLocalFrame*) {
+ // TODO(mustaq): Why is it not checking user activation state at all? In
+ // particular, the last Boolean param ("privileged") in the IPC below looks
+ // scary without this check.
widget_->Send(
new WidgetHostMsg_LockMouse(widget_->routing_id(), false, true));
}
diff --git a/content/renderer/render_widget_mouse_lock_dispatcher.cc b/content/renderer/render_widget_mouse_lock_dispatcher.cc
index bac8dc5..456f5e8 100644
--- a/content/renderer/render_widget_mouse_lock_dispatcher.cc
+++ b/content/renderer/render_widget_mouse_lock_dispatcher.cc
@@ -20,15 +20,10 @@
RenderWidgetMouseLockDispatcher::~RenderWidgetMouseLockDispatcher() {}
-void RenderWidgetMouseLockDispatcher::SendLockMouseRequest() {
- blink::WebWidget* web_widget = render_widget_->GetWebWidget();
- blink::WebLocalFrame* web_local_frame =
- (web_widget && web_widget->IsWebFrameWidget())
- ? static_cast<blink::WebFrameWidget*>(web_widget)->LocalRoot()
- : nullptr;
-
+void RenderWidgetMouseLockDispatcher::SendLockMouseRequest(
+ blink::WebLocalFrame* requester_frame) {
bool user_gesture =
- blink::WebUserGestureIndicator::IsProcessingUserGesture(web_local_frame);
+ blink::WebUserGestureIndicator::IsProcessingUserGesture(requester_frame);
render_widget_->Send(new WidgetHostMsg_LockMouse(render_widget_->routing_id(),
user_gesture, false));
}
diff --git a/content/renderer/render_widget_mouse_lock_dispatcher.h b/content/renderer/render_widget_mouse_lock_dispatcher.h
index b9f5cb2d..4e81790 100644
--- a/content/renderer/render_widget_mouse_lock_dispatcher.h
+++ b/content/renderer/render_widget_mouse_lock_dispatcher.h
@@ -27,7 +27,7 @@
private:
// MouseLockDispatcher implementation.
- void SendLockMouseRequest() override;
+ void SendLockMouseRequest(blink::WebLocalFrame* requester_frame) override;
void SendUnlockMouseRequest() override;
void OnLockMouseACK(bool succeeded);
diff --git a/content/shell/test_runner/web_widget_test_proxy.cc b/content/shell/test_runner/web_widget_test_proxy.cc
index 4536afb..33c7bce 100644
--- a/content/shell/test_runner/web_widget_test_proxy.cc
+++ b/content/shell/test_runner/web_widget_test_proxy.cc
@@ -89,7 +89,7 @@
}
}
-bool WebWidgetTestProxy::RequestPointerLock() {
+bool WebWidgetTestProxy::RequestPointerLock(blink::WebLocalFrame*) {
return GetViewTestRunner()->RequestPointerLock();
}
diff --git a/content/shell/test_runner/web_widget_test_proxy.h b/content/shell/test_runner/web_widget_test_proxy.h
index 04b356f..6abcf04 100644
--- a/content/shell/test_runner/web_widget_test_proxy.h
+++ b/content/shell/test_runner/web_widget_test_proxy.h
@@ -65,7 +65,7 @@
// WebWidgetClient implementation.
void ScheduleAnimation() override;
- bool RequestPointerLock() override;
+ bool RequestPointerLock(blink::WebLocalFrame* requester_frame) override;
void RequestPointerUnlock() override;
bool IsPointerLocked() override;
void SetToolTipText(const blink::WebString& text,
diff --git a/third_party/blink/public/web/web_widget_client.h b/third_party/blink/public/web/web_widget_client.h
index 94d0749..2c4e56a 100644
--- a/third_party/blink/public/web/web_widget_client.h
+++ b/third_party/blink/public/web/web_widget_client.h
@@ -73,6 +73,7 @@
struct WebFloatPoint;
struct WebFloatRect;
struct WebFloatSize;
+class WebLocalFrame;
class WebWidgetClient {
public:
@@ -145,12 +146,14 @@
// Called when a tooltip should be shown at the current cursor position.
virtual void SetToolTipText(const WebString&, WebTextDirection hint) {}
- // Requests to lock the mouse cursor. If true is returned, the success
- // result will be asynchronously returned via a single call to
- // WebWidget::didAcquirePointerLock() or
+ // Requests to lock the mouse cursor for the |requester_frame| in the
+ // widget. If true is returned, the success result will be asynchronously
+ // returned via a single call to WebWidget::didAcquirePointerLock() or
// WebWidget::didNotAcquirePointerLock().
// If false, the request has been denied synchronously.
- virtual bool RequestPointerLock() { return false; }
+ virtual bool RequestPointerLock(WebLocalFrame* requester_frame) {
+ return false;
+ }
// Cause the pointer lock to be released. This may be called at any time,
// including when a lock is pending but not yet acquired.
diff --git a/third_party/blink/renderer/core/page/chrome_client_impl.cc b/third_party/blink/renderer/core/page/chrome_client_impl.cc
index 2df0aef..f033c08 100644
--- a/third_party/blink/renderer/core/page/chrome_client_impl.cc
+++ b/third_party/blink/renderer/core/page/chrome_client_impl.cc
@@ -1122,7 +1122,7 @@
return WebLocalFrameImpl::FromFrame(frame)
->LocalRootFrameWidget()
->Client()
- ->RequestPointerLock();
+ ->RequestPointerLock(WebLocalFrameImpl::FromFrame(frame));
}
void ChromeClientImpl::RequestPointerUnlock(LocalFrame* frame) {
