Commit ad824435 authored by lfg's avatar lfg Committed by Commit bot

Fix crash when destroying a RenderWidgetHost that holds the

pointer lock.

This CL fixes two separate issues. One is that when a
CrossProcessFrameConnector is being destroyed, if the
RenderWidgetHostViewChildFrame holds the mouse lock it must
unlock the mouse.

The second issue is that when flash holds a fullscreen
pointer lock and releases it, the WebContents is notified
of the lost mouse lock, even though it was never notified
that the mouse lock was acquired (because flash has extra
privileges to acquire the lock).

BUG=619571
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2112923002
Cr-Commit-Position: refs/heads/master@{#405195}
parent 1e0384f8
...@@ -741,3 +741,32 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessInteractiveBrowserTest, ...@@ -741,3 +741,32 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessInteractiveBrowserTest,
EXPECT_FALSE(ElementHasFullscreenStyle(c_middle, "child-0")); EXPECT_FALSE(ElementHasFullscreenStyle(c_middle, "child-0"));
} }
// Test that deleting a RenderWidgetHost that holds the mouse lock won't cause a
// crash. https://crbug.com/619571.
IN_PROC_BROWSER_TEST_F(SitePerProcessInteractiveBrowserTest,
RenderWidgetHostDeletedWhileMouseLocked) {
GURL main_url(embedded_test_server()->GetURL(
"a.com", "/cross_site_iframe_factory.html?a(b)"));
ui_test_utils::NavigateToURL(browser(), main_url);
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
content::RenderFrameHost* main_frame = web_contents->GetMainFrame();
content::RenderFrameHost* child = ChildFrameAt(main_frame, 0);
EXPECT_TRUE(ExecuteScript(child, "document.body.requestPointerLock()"));
bool mouse_locked = false;
EXPECT_TRUE(ExecuteScriptAndExtractBool(child,
"window.domAutomationController.send("
"document.body == "
"document.pointerLockElement)",
&mouse_locked));
EXPECT_TRUE(mouse_locked);
EXPECT_TRUE(main_frame->GetView()->IsMouseLocked());
EXPECT_TRUE(ExecuteScript(main_frame,
"document.querySelector('iframe').parentNode."
"removeChild(document.querySelector('iframe'))"));
EXPECT_FALSE(main_frame->GetView()->IsMouseLocked());
}
...@@ -203,6 +203,12 @@ bool CrossProcessFrameConnector::LockMouse() { ...@@ -203,6 +203,12 @@ bool CrossProcessFrameConnector::LockMouse() {
return false; return false;
} }
void CrossProcessFrameConnector::UnlockMouse() {
RenderWidgetHostViewBase* root_view = GetRootRenderWidgetHostView();
if (root_view)
root_view->UnlockMouse();
}
void CrossProcessFrameConnector::OnForwardInputEvent( void CrossProcessFrameConnector::OnForwardInputEvent(
const blink::WebInputEvent* event) { const blink::WebInputEvent* event) {
if (!view_) if (!view_)
......
...@@ -114,6 +114,9 @@ class CONTENT_EXPORT CrossProcessFrameConnector { ...@@ -114,6 +114,9 @@ class CONTENT_EXPORT CrossProcessFrameConnector {
// Locks the mouse. Returns true if mouse is locked. // Locks the mouse. Returns true if mouse is locked.
bool LockMouse(); bool LockMouse();
// Unlocks the mouse if the mouse is locked.
void UnlockMouse();
// Returns the parent RenderWidgetHostView or nullptr it it doesn't have one. // Returns the parent RenderWidgetHostView or nullptr it it doesn't have one.
virtual RenderWidgetHostViewBase* GetParentRenderWidgetHostView(); virtual RenderWidgetHostViewBase* GetParentRenderWidgetHostView();
......
...@@ -77,6 +77,13 @@ void RenderWidgetHostViewChildFrame::SetCrossProcessFrameConnector( ...@@ -77,6 +77,13 @@ void RenderWidgetHostViewChildFrame::SetCrossProcessFrameConnector(
id_allocator_->client_id()); id_allocator_->client_id());
parent_surface_client_id_ = 0; parent_surface_client_id_ = 0;
// After the RenderWidgetHostViewChildFrame loses the frame_connector, it
// won't be able to walk up the frame tree anymore. Clean up anything that
// needs to be done through the CrossProcessFrameConnector before it's gone.
// Unlocks the mouse if this RenderWidgetHostView holds the lock.
UnlockMouse();
} }
frame_connector_ = frame_connector; frame_connector_ = frame_connector;
if (frame_connector_) { if (frame_connector_) {
...@@ -458,19 +465,16 @@ bool RenderWidgetHostViewChildFrame::LockMouse() { ...@@ -458,19 +465,16 @@ bool RenderWidgetHostViewChildFrame::LockMouse() {
} }
void RenderWidgetHostViewChildFrame::UnlockMouse() { void RenderWidgetHostViewChildFrame::UnlockMouse() {
if (host_->delegate() && host_->delegate()->HasMouseLock(host_) &&
frame_connector_)
frame_connector_->UnlockMouse();
} }
bool RenderWidgetHostViewChildFrame::IsMouseLocked() { bool RenderWidgetHostViewChildFrame::IsMouseLocked() {
if (!frame_connector_) if (!host_->delegate())
return false; return false;
RenderWidgetHostViewBase* root_view = return host_->delegate()->HasMouseLock(host_);
frame_connector_->GetRootRenderWidgetHostView();
if (root_view)
return root_view->IsMouseLocked();
return false;
} }
uint32_t RenderWidgetHostViewChildFrame::GetSurfaceClientId() { uint32_t RenderWidgetHostViewChildFrame::GetSurfaceClientId() {
......
...@@ -62,6 +62,11 @@ blink::WebDisplayMode RenderWidgetHostDelegate::GetDisplayMode( ...@@ -62,6 +62,11 @@ blink::WebDisplayMode RenderWidgetHostDelegate::GetDisplayMode(
return blink::WebDisplayModeBrowser; return blink::WebDisplayModeBrowser;
} }
bool RenderWidgetHostDelegate::HasMouseLock(
RenderWidgetHostImpl* render_widget_host) {
return false;
}
TextInputManager* RenderWidgetHostDelegate::GetTextInputManager() { TextInputManager* RenderWidgetHostDelegate::GetTextInputManager() {
return nullptr; return nullptr;
} }
......
...@@ -144,10 +144,12 @@ class CONTENT_EXPORT RenderWidgetHostDelegate { ...@@ -144,10 +144,12 @@ class CONTENT_EXPORT RenderWidgetHostDelegate {
// Requests to lock the mouse. Once the request is approved or rejected, // Requests to lock the mouse. Once the request is approved or rejected,
// GotResponseToLockMouseRequest() will be called on the requesting render // GotResponseToLockMouseRequest() will be called on the requesting render
// widget host. // widget host. |privileged| means that the request is always granted, used
// for Pepper Flash.
virtual void RequestToLockMouse(RenderWidgetHostImpl* render_widget_host, virtual void RequestToLockMouse(RenderWidgetHostImpl* render_widget_host,
bool user_gesture, bool user_gesture,
bool last_unlocked_by_target) {} bool last_unlocked_by_target,
bool privileged) {}
// Return the rect where to display the resize corner, if any, otherwise // Return the rect where to display the resize corner, if any, otherwise
// an empty rect. // an empty rect.
...@@ -167,6 +169,9 @@ class CONTENT_EXPORT RenderWidgetHostDelegate { ...@@ -167,6 +169,9 @@ class CONTENT_EXPORT RenderWidgetHostDelegate {
// Notification that the widget has lost the mouse lock. // Notification that the widget has lost the mouse lock.
virtual void LostMouseLock(RenderWidgetHostImpl* render_widget_host) {} virtual void LostMouseLock(RenderWidgetHostImpl* render_widget_host) {}
// Returns true if |render_widget_host| holds the mouse lock.
virtual bool HasMouseLock(RenderWidgetHostImpl* render_widget_host);
// Called when the widget has sent a compositor proto. This is used in Btlimp // Called when the widget has sent a compositor proto. This is used in Btlimp
// mode with the RemoteChannel compositor. // mode with the RemoteChannel compositor.
virtual void ForwardCompositorProto(RenderWidgetHostImpl* render_widget_host, virtual void ForwardCompositorProto(RenderWidgetHostImpl* render_widget_host,
......
...@@ -1731,16 +1731,17 @@ void RenderWidgetHostImpl::OnLockMouse(bool user_gesture, ...@@ -1731,16 +1731,17 @@ void RenderWidgetHostImpl::OnLockMouse(bool user_gesture,
} }
pending_mouse_lock_request_ = true; pending_mouse_lock_request_ = true;
if (delegate_) {
delegate_->RequestToLockMouse(this, user_gesture, last_unlocked_by_target,
privileged && allow_privileged_mouse_lock_);
return;
}
if (privileged && allow_privileged_mouse_lock_) { if (privileged && allow_privileged_mouse_lock_) {
// Directly approve to lock the mouse. // Directly approve to lock the mouse.
GotResponseToLockMouseRequest(true); GotResponseToLockMouseRequest(true);
} else { } else {
if (delegate_) { // Otherwise, just reject it.
delegate_->RequestToLockMouse(this, user_gesture,
last_unlocked_by_target);
return;
}
// If there's no delegate, just reject it.
GotResponseToLockMouseRequest(false); GotResponseToLockMouseRequest(false);
} }
} }
......
...@@ -1707,8 +1707,7 @@ void WebContentsImpl::RenderWidgetDeleted( ...@@ -1707,8 +1707,7 @@ void WebContentsImpl::RenderWidgetDeleted(
view_->RestoreFocus(); view_->RestoreFocus();
} }
if (mouse_lock_widget_ == render_widget_host) CHECK(mouse_lock_widget_ != render_widget_host);
mouse_lock_widget_ = nullptr;
} }
void WebContentsImpl::RenderWidgetGotFocus( void WebContentsImpl::RenderWidgetGotFocus(
...@@ -1897,12 +1896,19 @@ blink::WebDisplayMode WebContentsImpl::GetDisplayMode( ...@@ -1897,12 +1896,19 @@ blink::WebDisplayMode WebContentsImpl::GetDisplayMode(
void WebContentsImpl::RequestToLockMouse( void WebContentsImpl::RequestToLockMouse(
RenderWidgetHostImpl* render_widget_host, RenderWidgetHostImpl* render_widget_host,
bool user_gesture, bool user_gesture,
bool last_unlocked_by_target) { bool last_unlocked_by_target,
bool privileged) {
if (mouse_lock_widget_) { if (mouse_lock_widget_) {
render_widget_host->GotResponseToLockMouseRequest(false); render_widget_host->GotResponseToLockMouseRequest(false);
return; return;
} }
if (privileged) {
mouse_lock_widget_ = render_widget_host;
render_widget_host->GotResponseToLockMouseRequest(true);
return;
}
bool widget_in_frame_tree = false; bool widget_in_frame_tree = false;
for (FrameTreeNode* node : frame_tree_.Nodes()) { for (FrameTreeNode* node : frame_tree_.Nodes()) {
if (node->current_frame_host()->GetRenderWidgetHost() == if (node->current_frame_host()->GetRenderWidgetHost() ==
...@@ -1929,6 +1935,14 @@ void WebContentsImpl::LostMouseLock(RenderWidgetHostImpl* render_widget_host) { ...@@ -1929,6 +1935,14 @@ void WebContentsImpl::LostMouseLock(RenderWidgetHostImpl* render_widget_host) {
delegate_->LostMouseLock(); delegate_->LostMouseLock();
} }
bool WebContentsImpl::HasMouseLock(RenderWidgetHostImpl* render_widget_host) {
// To verify if the mouse is locked, the mouse_lock_widget_ needs to be
// assigned to the widget that requested the mouse lock, and the top-level
// platform RenderWidgetHostView needs to hold the mouse lock from the OS.
return mouse_lock_widget_ == render_widget_host &&
GetTopLevelRenderWidgetHostView()->IsMouseLocked();
}
void WebContentsImpl::ForwardCompositorProto( void WebContentsImpl::ForwardCompositorProto(
RenderWidgetHostImpl* render_widget_host, RenderWidgetHostImpl* render_widget_host,
const std::vector<uint8_t>& proto) { const std::vector<uint8_t>& proto) {
...@@ -2986,8 +3000,11 @@ bool WebContentsImpl::GotResponseToLockMouseRequest(bool allowed) { ...@@ -2986,8 +3000,11 @@ bool WebContentsImpl::GotResponseToLockMouseRequest(bool allowed) {
if (GetBrowserPluginGuest()) if (GetBrowserPluginGuest())
return GetBrowserPluginGuest()->LockMouse(allowed); return GetBrowserPluginGuest()->LockMouse(allowed);
if (mouse_lock_widget_) if (mouse_lock_widget_ &&
return mouse_lock_widget_->GotResponseToLockMouseRequest(allowed); mouse_lock_widget_->GotResponseToLockMouseRequest(allowed))
return true;
mouse_lock_widget_ = nullptr;
return false; return false;
} }
......
...@@ -618,7 +618,8 @@ class CONTENT_EXPORT WebContentsImpl ...@@ -618,7 +618,8 @@ class CONTENT_EXPORT WebContentsImpl
void RendererResponsive(RenderWidgetHostImpl* render_widget_host) override; void RendererResponsive(RenderWidgetHostImpl* render_widget_host) override;
void RequestToLockMouse(RenderWidgetHostImpl* render_widget_host, void RequestToLockMouse(RenderWidgetHostImpl* render_widget_host,
bool user_gesture, bool user_gesture,
bool last_unlocked_by_target) override; bool last_unlocked_by_target,
bool privileged) override;
gfx::Rect GetRootWindowResizerRect( gfx::Rect GetRootWindowResizerRect(
RenderWidgetHostImpl* render_widget_host) const override; RenderWidgetHostImpl* render_widget_host) const override;
bool IsFullscreenForCurrentTab() const override; bool IsFullscreenForCurrentTab() const override;
...@@ -626,6 +627,7 @@ class CONTENT_EXPORT WebContentsImpl ...@@ -626,6 +627,7 @@ class CONTENT_EXPORT WebContentsImpl
RenderWidgetHostImpl* render_widget_host) const override; RenderWidgetHostImpl* render_widget_host) const override;
void LostCapture(RenderWidgetHostImpl* render_widget_host) override; void LostCapture(RenderWidgetHostImpl* render_widget_host) override;
void LostMouseLock(RenderWidgetHostImpl* render_widget_host) override; void LostMouseLock(RenderWidgetHostImpl* render_widget_host) override;
bool HasMouseLock(RenderWidgetHostImpl* render_widget_host) override;
void ForwardCompositorProto(RenderWidgetHostImpl* render_widget_host, void ForwardCompositorProto(RenderWidgetHostImpl* render_widget_host,
const std::vector<uint8_t>& proto) override; const std::vector<uint8_t>& proto) override;
void OnRenderFrameProxyVisibilityChanged(bool visible) override; void OnRenderFrameProxyVisibilityChanged(bool visible) override;
......
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