Implement pointer lock across outer/inner WebContents.

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index c8b38722cbd096c893ec16e258aae8768681b16b..03fa5d5aee10f1f32d234d9d48007c7289849200 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -1916,12 +1916,16 @@ void WebContentsImpl::RequestToLockMouse(
     bool user_gesture,
     bool last_unlocked_by_target,
     bool privileged) {
-  if (mouse_lock_widget_) {
-    render_widget_host->GotResponseToLockMouseRequest(false);
-    return;
+  for (WebContentsImpl* current = this; current;
+       current = current->GetOuterWebContents()) {
+    if (current->mouse_lock_widget_) {

[ncarter (slow), 2016/12/22 00:01:07]

I'm wondering if we should contemplate having some refptr'ed context object
(like a focus manager or something) that is shared among the inner/outer
WebContentses, or something else that deduplicates this state.

It is fine to proceed with things as you've implemented it, since it looks
correct to me. But if this pattern is something that will happen again with
other data members, then I'd hope we could shift it to a shared context object.

[lfg, 2016/12/22 19:23:55]

I thought about that a bit, but for a single pointer I didn't think this was
worth the refactoring. Something like that would also introduce complicated
lifetime management.

+      render_widget_host->GotResponseToLockMouseRequest(false);
+      return;
+    }
   }
 
   if (privileged) {
+    DCHECK(!GetOuterWebContents());
     mouse_lock_widget_ = render_widget_host;
     render_widget_host->GotResponseToLockMouseRequest(true);
     return;
@@ -1937,7 +1941,11 @@ void WebContentsImpl::RequestToLockMouse(
   }
 
   if (widget_in_frame_tree && delegate_) {
-    mouse_lock_widget_ = render_widget_host;
+    for (WebContentsImpl* current = this; current;
+         current = current->GetOuterWebContents()) {
+      current->mouse_lock_widget_ = render_widget_host;
+    }
+
     delegate_->RequestToLockMouse(this, user_gesture, last_unlocked_by_target);
   } else {
     render_widget_host->GotResponseToLockMouseRequest(false);
@@ -1946,8 +1954,15 @@ void WebContentsImpl::RequestToLockMouse(
 
 void WebContentsImpl::LostMouseLock(RenderWidgetHostImpl* render_widget_host) {
   CHECK(mouse_lock_widget_);
+
+  if (mouse_lock_widget_->delegate()->GetAsWebContents() != this)
+    return mouse_lock_widget_->delegate()->LostMouseLock(render_widget_host);
+
   mouse_lock_widget_->SendMouseLockLost();
-  mouse_lock_widget_ = nullptr;
+  for (WebContentsImpl* current = this; current;
+       current = current->GetOuterWebContents()) {
+    current->mouse_lock_widget_ = nullptr;
+  }
 
   if (delegate_)
     delegate_->LostMouseLock();
@@ -3063,14 +3078,26 @@ gfx::Size WebContentsImpl::GetPreferredSize() const {
 }
 
 bool WebContentsImpl::GotResponseToLockMouseRequest(bool allowed) {
-  if (GetBrowserPluginGuest())
+  if (!GuestMode::IsCrossProcessFrameGuest(GetWebContents()) &&
+      GetBrowserPluginGuest())
     return GetBrowserPluginGuest()->LockMouse(allowed);
 
-  if (mouse_lock_widget_ &&
-      mouse_lock_widget_->GotResponseToLockMouseRequest(allowed))
-    return true;
+  if (mouse_lock_widget_) {
+    if (mouse_lock_widget_->delegate()->GetAsWebContents() != this) {

[ncarter (slow), 2016/12/22 00:01:07]

When does this case happen?

From what I can tell, GotResponseToLockMouseRequest() is a response method,
meant to be called by implementations of
WebContentsDelegate::RequestToLockMouse(). And it looks like
GotResponseToLockMouseRequest is always called on the WebContents passed as the
first argument to RequestToLockMouse().

But the only call I see to delegate_->RequestToLockMouse() seems to originate
from the inner WebContents that's actually wanting the mouse lock (i.e. the
lowest one for which mouse_lock_widget_ is non-null), so it's not clear why we
need to descend further down the tree here.

If it turns out that we don't need this case, then we probably don't need the
GetAsWebContents() method either -- the other usage of GetAsWebContents() can
just be replaced by casting the WebContentsImpl to a RenderWidgetHostDelegate
and doing the equality comparison between the two Delegate* pointers.

[lfg, 2016/12/22 19:23:55]

GotResponseToLockMouseRequest is a poor name for the function, as it can also
used to cancel the mouse lock (see the implementation in RenderWidgetHostImpl,
that calls RejectMouseLockOrUnlockIfNecessary). And it's the only function
exposed in the content/public interface, as LostMouseLock is not.

+      return mouse_lock_widget_->delegate()
+          ->GetAsWebContents()
+          ->GotResponseToLockMouseRequest(allowed);
+    }
+
+    if (mouse_lock_widget_->GotResponseToLockMouseRequest(allowed))
+      return true;
+  }
+
+  for (WebContentsImpl* current = this; current;
+       current = current->GetOuterWebContents()) {
+    current->mouse_lock_widget_ = nullptr;
+  }
 
-  mouse_lock_widget_ = nullptr;
   return false;
 }