Implement pointer lock API for out-of-process iframes.

Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

TEST=Manual test please:
Navigate to http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html
and verify that toggling pointer lock from all iframes work
as well as exiting* pointer lock. Verify that the mouse
movement continues to be tracked in the frame that locked
the pointer.
* Exiting can be done by ESC key, ALT-TAB (on Mac Command
Tab), Windows key to bring up start menu etc.

BUG=601926, 419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/bee1e0ad56ce6b9def191f6ec6ef5a71a2f5d83e
Cr-Commit-Position: refs/heads/master@{#398679}

[lfg, 2016-05-16 20:00:07]

Description was changed from

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

BUG=601926,419087
==========

to

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[lfg, 2016-05-16 20:19:20]

lfg@chromium.org changed reviewers:
+ dcheng@chromium.org

[lfg, 2016-05-16 20:19:20]

Daniel, please take a look.

[dcheng, 2016-05-16 20:24:40]

On 2016/05/16 at 20:19:20, lfg wrote:
> Daniel, please take a look.

Is there anyone who's more familiar with the pointer lock API (other than you
now, presumably =)?

[lfg, 2016-05-16 20:46:19]

Description was changed from

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[lfg, 2016-05-16 20:46:19]

lfg@chromium.org changed reviewers:
+ scheib@chromium.org

[lfg, 2016-05-16 20:47:05]

On 2016/05/16 20:24:40, dcheng wrote:
> On 2016/05/16 at 20:19:20, lfg wrote:
> > Daniel, please take a look.
> 
> Is there anyone who's more familiar with the pointer lock API (other than you
> now, presumably =)?

I've looked through the blame, scheib@ implemented this back in 2012, and not
much changed since.

+scheib@, can you take a look?

[scheib, 2016-05-17 20:26:44]

I'll look.

[scheib, 2016-05-19 00:38:58]

Thanks, looking forward to this working. There's quite an array of components
involved, so inspection alone isn't great to know this works. For testing there
is the mouse_lock_dispatcher_browsertest.cc and layout tests with iframes. Is
there a story for out of process iframes, can we gain covereage for them in
these systems?

Many pointer lock bugs are systematic, needing integration testing. The browser
test gets some of that, but not all. In lieu of building out a testing API
spanning that whole range manual TEST= instructions were included in early
patches. I think this change needs a manual test plan.

https://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html is one example
testing page.

https://codereview.chromium.org/1980133002/diff/40001/content/renderer/render...
File content/renderer/render_widget.h (right):

https://codereview.chromium.org/1980133002/diff/40001/content/renderer/render...
content/renderer/render_widget.h:411: bool should_add_route);
Is this part of the mouse lock move? I didn't follow this part of the change.

https://codereview.chromium.org/1980133002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/PointerLockController.cpp (right):

https://codereview.chromium.org/1980133002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/PointerLockController.cpp:88: // Set element
null immediately to block any future interaction with it
This comment should follow the clearElement call.

https://codereview.chromium.org/1980133002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1980133002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebViewImpl.cpp:4430: return
mainFrameImpl()->frameWidget() && mainFrameImpl()->frameWidget()->client() &&
mainFrameImpl()->frameWidget()->client()->requestPointerLock();
These && chains are too long.. let's do:

if (widget = mainFrameImpl()->frameWidget()) {
  if (client = ...) {
    client->requestPointerLock();
  }
}

[lfg, 2016-05-19 23:53:56]

On 2016/05/19 00:38:58, scheib wrote:
> Thanks, looking forward to this working. There's quite an array of components
> involved, so inspection alone isn't great to know this works. For testing
there
> is the mouse_lock_dispatcher_browsertest.cc and layout tests with iframes. Is
> there a story for out of process iframes, can we gain covereage for them in
> these systems?

Since there's a single codepath for out-of-process iframes and non-oopif chrome,
the same tests that apply today exercise the same codepath used for
out-of-process iframes (I updated all the tests to be executed on the widget,
instead of the view).

> Many pointer lock bugs are systematic, needing integration testing. The
browser
> test gets some of that, but not all. In lieu of building out a testing API
> spanning that whole range manual TEST= instructions were included in early
> patches. I think this change needs a manual test plan.

That's a good point, it's definitely a good idea. Do you have some pointers or
design docs that cover the manual testing instructions? I tried to look through
the commit logs, but couldn't find much.

[lfg, 2016-05-19 23:54:07]

https://codereview.chromium.org/1980133002/diff/40001/content/renderer/render...
File content/renderer/render_widget.h (right):

https://codereview.chromium.org/1980133002/diff/40001/content/renderer/render...
content/renderer/render_widget.h:411: bool should_add_route);
On 2016/05/19 00:38:58, scheib wrote:
> Is this part of the mouse lock move? I didn't follow this part of the change.

It's not strictly necessary, but since I've moved the mouse_lock_dispatcher to
RenderWidget, we still need to initialize it during the RenderView
initialization. Instead of adding a new function that does the same as
RenderWidget::DoInit without adding the route, I've just added a parameter here.
I feel this is a bit cleaner.

In the future, we'd like to have RenderView be just a renderer object holding
the state of the page, and not having a RenderViewHost counterpart (so no route
anymore).

https://codereview.chromium.org/1980133002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/PointerLockController.cpp (right):

https://codereview.chromium.org/1980133002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/PointerLockController.cpp:88: // Set element
null immediately to block any future interaction with it
On 2016/05/19 00:38:58, scheib wrote:
> This comment should follow the clearElement call.

Done.

https://codereview.chromium.org/1980133002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1980133002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebViewImpl.cpp:4430: return
mainFrameImpl()->frameWidget() && mainFrameImpl()->frameWidget()->client() &&
mainFrameImpl()->frameWidget()->client()->requestPointerLock();
On 2016/05/19 00:38:58, scheib wrote:
> These && chains are too long.. let's do:
> 
> if (widget = mainFrameImpl()->frameWidget()) {
>   if (client = ...) {
>     client->requestPointerLock();
>   }
> }

Done.

[scheib, 2016-05-21 05:14:52]

LGTM with a TEST= plan (response follows)

[scheib, 2016-05-21 05:27:15]

> > Many pointer lock bugs are systematic, needing integration testing. The
> browser
> > test gets some of that, but not all. In lieu of building out a testing API
> > spanning that whole range manual TEST= instructions were included in early
> > patches. I think this change needs a manual test plan.
> 
> That's a good point, it's definitely a good idea. Do you have some pointers or
> design docs that cover the manual testing instructions? I tried to look
through
> the commit logs, but couldn't find much.

We should have something better. It just hasn't been a priority and work here is
minimal. /shame on me/.

https://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html exposes many
functions. I think something like:

TEST=Manual test please:
 From http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html enter and
exit* pointer lock from the main page, and the iframe, several times verifying
that the mouse movement continues to have an effect and that state can be
entered and exited.
 * Exiting can be done by ESC key, ALT-TAB (on Mac Command Tab), Windows key to
bring up start menu etc.


Is there anything we can to to force an iframe in / out of being out-of-process?

[lfg, 2016-05-25 18:52:33]

On 2016/05/21 05:27:15, scheib wrote:
> We should have something better. It just hasn't been a priority and work here
is
> minimal. /shame on me/.
> 
> https://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html exposes many
> functions. I think something like:
> 
> TEST=Manual test please:
>  From http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html enter and
> exit* pointer lock from the main page, and the iframe, several times verifying
> that the mouse movement continues to have an effect and that state can be
> entered and exited.
>  * Exiting can be done by ESC key, ALT-TAB (on Mac Command Tab), Windows key
to
> bring up start menu etc.
> 
> 
> Is there anything we can to to force an iframe in / out of being
out-of-process?

Yes, the easiest way to do it on github is to mix http and https (since they
will be different origins). Do you mind adding that you your test page, so I can
use it writing a manual test plan? Basically, you already have an iframe there,
you could just add a new iframe with src=http://... (insttead of https, since
then they will be different origins). You can make sure that an out-of-process
iframe is created by running chrome with --site-per-process and looking at the
task manager, you'll get a process tagged as Subframe.

[lfg, 2016-05-25 18:53:39]

lfg@chromium.org changed reviewers:
+ nasko@chromium.org

[lfg, 2016-05-25 18:53:40]

+nasko@ for content/ review.

[nasko, 2016-05-25 21:12:01]

Mostly nits.

https://codereview.chromium.org/1980133002/diff/80001/content/browser/frame_h...
File content/browser/frame_host/render_widget_host_view_child_frame.cc (right):

https://codereview.chromium.org/1980133002/diff/80001/content/browser/frame_h...
content/browser/frame_host/render_widget_host_view_child_frame.cc:489: if
(frame_connector_) {
Why not early return false if !frame_connector_? It removes a level of
indentation.

https://codereview.chromium.org/1980133002/diff/80001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_impl.h (right):

https://codereview.chromium.org/1980133002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_impl.h:247: // Notifies the
renderer that the widget lost the mouse lock.
nit: Either "renderer process" or "RenderWidget", so we are precise in comments.

https://codereview.chromium.org/1980133002/diff/80001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/1980133002/diff/80001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:1912: } else
The else clause also needs {} if the if clause has them.

https://codereview.chromium.org/1980133002/diff/80001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.h (right):

https://codereview.chromium.org/1980133002/diff/80001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.h:1378: RenderWidgetHostImpl*
mouse_lock_widget_;
Put a small comment clarifying this member.

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
content/renderer/render_widget.cc:158: // The WebWidget handles mouse lock in
WebKit's handleInputEvent().
nit: s/WebKit/Blink/

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
content/renderer/render_widget.cc:273: mouse_lock_dispatcher_(nullptr),
unique_ptr doesn't need explicit initialization to null, does it?

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
File content/renderer/render_widget_mouse_lock_dispatcher.cc (right):

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
content/renderer/render_widget_mouse_lock_dispatcher.cc:1: // Copyright (c) 2012
The Chromium Authors. All rights reserved.
Update this line too.

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
File content/renderer/render_widget_mouse_lock_dispatcher.h (right):

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
content/renderer/render_widget_mouse_lock_dispatcher.h:1: // Copyright (c) 2012
The Chromium Authors. All rights reserved.
No (c) and we are now 2016 ;)

[scheib, 2016-05-26 20:49:27]

On 2016/05/25 18:52:33, lfg wrote:
> On 2016/05/21 05:27:15, scheib wrote:
> > We should have something better. It just hasn't been a priority and work
here
> is
> > minimal. /shame on me/.
> > 
> > https://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html exposes many
> > functions. I think something like:
> > 
> > TEST=Manual test please:
> >  From http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html enter
and
> > exit* pointer lock from the main page, and the iframe, several times
verifying
> > that the mouse movement continues to have an effect and that state can be
> > entered and exited.
> >  * Exiting can be done by ESC key, ALT-TAB (on Mac Command Tab), Windows key
> to
> > bring up start menu etc.
> > 
> > 
> > Is there anything we can to to force an iframe in / out of being
> out-of-process?
> 
> Yes, the easiest way to do it on github is to mix http and https (since they
> will be different origins). Do you mind adding that you your test page, so I
can
> use it writing a manual test plan? Basically, you already have an iframe
there,
> you could just add a new iframe with src=http://... (insttead of https, since
> then they will be different origins). You can make sure that an out-of-process
> iframe is created by running chrome with --site-per-process and looking at the
> task manager, you'll get a process tagged as Subframe.

I've created
https://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html

If that's not right, please fix up and pull request.

[scheib, 2016-05-26 21:33:11]

On 2016/05/26 20:49:27, scheib wrote:
> On 2016/05/25 18:52:33, lfg wrote:
> > On 2016/05/21 05:27:15, scheib wrote:
> > > We should have something better. It just hasn't been a priority and work
> here
> > is
> > > minimal. /shame on me/.
> > > 
> > > https://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html exposes
many
> > > functions. I think something like:
> > > 
> > > TEST=Manual test please:
> > >  From http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html enter
> and
> > > exit* pointer lock from the main page, and the iframe, several times
> verifying
> > > that the mouse movement continues to have an effect and that state can be
> > > entered and exited.
> > >  * Exiting can be done by ESC key, ALT-TAB (on Mac Command Tab), Windows
key
> > to
> > > bring up start menu etc.
> > > 
> > > 
> > > Is there anything we can to to force an iframe in / out of being
> > out-of-process?
> > 
> > Yes, the easiest way to do it on github is to mix http and https (since they
> > will be different origins). Do you mind adding that you your test page, so I
> can
> > use it writing a manual test plan? Basically, you already have an iframe
> there,
> > you could just add a new iframe with src=http://... (insttead of https,
since
> > then they will be different origins). You can make sure that an
out-of-process
> > iframe is created by running chrome with --site-per-process and looking at
the
> > task manager, you'll get a process tagged as Subframe.
> 
> I've created
> https://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html

http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html
too

[lfg, 2016-05-31 21:12:07]

Description was changed from

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

TEST=Manual test please:
Navigate to
http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html and
verify that toggling pointer lock from all iframes work as well as exiting*
pointer lock. Verify that the mouse movement continues to be tracked in the
frame that locked the pointer.
* Exiting can be done by ESC key, ALT-TAB (on Mac Command Tab), Windows key to
bring up start menu etc.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[lfg, 2016-05-31 21:12:32]

Description was changed from

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

TEST=Manual test please:
Navigate to
http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html and
verify that toggling pointer lock from all iframes work as well as exiting*
pointer lock. Verify that the mouse movement continues to be tracked in the
frame that locked the pointer.
* Exiting can be done by ESC key, ALT-TAB (on Mac Command Tab), Windows key to
bring up start menu etc.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

TEST=Manual test please:
Navigate to
http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html
and verify that toggling pointer lock from all iframes work
as well as exiting* pointer lock. Verify that the mouse
movement continues to be tracked in the frame that locked
the pointer.
* Exiting can be done by ESC key, ALT-TAB (on Mac Command
Tab), Windows key to bring up start menu etc.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[lfg, 2016-05-31 21:13:10]

On 2016/05/26 21:33:11, scheib wrote:
> On 2016/05/26 20:49:27, scheib wrote:
> > On 2016/05/25 18:52:33, lfg wrote:
> > > On 2016/05/21 05:27:15, scheib wrote:
> > > > We should have something better. It just hasn't been a priority and work
> > here
> > > is
> > > > minimal. /shame on me/.
> > > > 
> > > > https://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html exposes
> many
> > > > functions. I think something like:
> > > > 
> > > > TEST=Manual test please:
> > > >  From http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen.html
enter
> > and
> > > > exit* pointer lock from the main page, and the iframe, several times
> > verifying
> > > > that the mouse movement continues to have an effect and that state can
be
> > > > entered and exited.
> > > >  * Exiting can be done by ESC key, ALT-TAB (on Mac Command Tab), Windows
> key
> > > to
> > > > bring up start menu etc.
> > > > 
> > > > 
> > > > Is there anything we can to to force an iframe in / out of being
> > > out-of-process?
> > > 
> > > Yes, the easiest way to do it on github is to mix http and https (since
they
> > > will be different origins). Do you mind adding that you your test page, so
I
> > can
> > > use it writing a manual test plan? Basically, you already have an iframe
> > there,
> > > you could just add a new iframe with src=http://... (insttead of https,
> since
> > > then they will be different origins). You can make sure that an
> out-of-process
> > > iframe is created by running chrome with --site-per-process and looking at
> the
> > > task manager, you'll get a process tagged as Subframe.
> > 
> > I've created
> > https://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html
> 
> http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html
> too

Thanks, that's perfect, I've added test instructions to the CL.

[lfg, 2016-05-31 21:13:20]

PTAL.

https://codereview.chromium.org/1980133002/diff/80001/content/browser/frame_h...
File content/browser/frame_host/render_widget_host_view_child_frame.cc (right):

https://codereview.chromium.org/1980133002/diff/80001/content/browser/frame_h...
content/browser/frame_host/render_widget_host_view_child_frame.cc:489: if
(frame_connector_) {
On 2016/05/25 21:12:01, nasko (slow) wrote:
> Why not early return false if !frame_connector_? It removes a level of
> indentation.

In this case I need two 'return false' since the root_view may be null (though
root_view can only be null in tests).

https://codereview.chromium.org/1980133002/diff/80001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_impl.h (right):

https://codereview.chromium.org/1980133002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_impl.h:247: // Notifies the
renderer that the widget lost the mouse lock.
On 2016/05/25 21:12:01, nasko (slow) wrote:
> nit: Either "renderer process" or "RenderWidget", so we are precise in
comments.

Done.

https://codereview.chromium.org/1980133002/diff/80001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/1980133002/diff/80001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:1912: } else
On 2016/05/25 21:12:01, nasko (slow) wrote:
> The else clause also needs {} if the if clause has them.

Done.

https://codereview.chromium.org/1980133002/diff/80001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.h (right):

https://codereview.chromium.org/1980133002/diff/80001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.h:1378: RenderWidgetHostImpl*
mouse_lock_widget_;
On 2016/05/25 21:12:01, nasko (slow) wrote:
> Put a small comment clarifying this member.

Done.

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
content/renderer/render_widget.cc:158: // The WebWidget handles mouse lock in
WebKit's handleInputEvent().
On 2016/05/25 21:12:01, nasko (slow) wrote:
> nit: s/WebKit/Blink/

Done.

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
content/renderer/render_widget.cc:273: mouse_lock_dispatcher_(nullptr),
On 2016/05/25 21:12:01, nasko (slow) wrote:
> unique_ptr doesn't need explicit initialization to null, does it?

Right, it used to be a raw pointer, I've changed it to a unique_ptr since it has
the same lifetime as the widget. Removed the initialization.

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
File content/renderer/render_widget_mouse_lock_dispatcher.cc (right):

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
content/renderer/render_widget_mouse_lock_dispatcher.cc:1: // Copyright (c) 2012
The Chromium Authors. All rights reserved.
On 2016/05/25 21:12:01, nasko (slow) wrote:
> Update this line too.

Should we do this for renames? This was renamed from
render_view_mouse_lock_dispatcher.

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
File content/renderer/render_widget_mouse_lock_dispatcher.h (right):

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
content/renderer/render_widget_mouse_lock_dispatcher.h:1: // Copyright (c) 2012
The Chromium Authors. All rights reserved.
On 2016/05/25 21:12:01, nasko (slow) wrote:
> No (c) and we are now 2016 ;)

Same question as the other file.

[scheib, 2016-06-01 00:57:00]

Thanks, TEST= LGTM

[nasko, 2016-06-01 16:31:03]

LGTM

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
File content/renderer/render_widget_mouse_lock_dispatcher.cc (right):

https://codereview.chromium.org/1980133002/diff/80001/content/renderer/render...
content/renderer/render_widget_mouse_lock_dispatcher.cc:1: // Copyright (c) 2012
The Chromium Authors. All rights reserved.
On 2016/05/31 21:13:20, lfg wrote:
> On 2016/05/25 21:12:01, nasko (slow) wrote:
> > Update this line too.
> 
> Should we do this for renames? This was renamed from
> render_view_mouse_lock_dispatcher.

Ah, good point!

[lfg, 2016-06-01 18:31:14]

+dcheng: Daniel, can you do a blink owners review?

[dcheng, 2016-06-04 05:28:56]

https://codereview.chromium.org/1980133002/diff/140001/content/renderer/rende...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/1980133002/diff/140001/content/renderer/rende...
content/renderer/render_widget.cc:393: bool should_add_route) {
Can we just make RenderView::Initialize() not add the route itself instead of
adding the bool?

https://codereview.chromium.org/1980133002/diff/140001/content/renderer/rende...
content/renderer/render_widget.cc:402: base::WrapUnique(new
RenderWidgetMouseLockDispatcher(this));
Nit: base::MakeUnique, or be consistent with line 400 and use reset.

https://codereview.chromium.org/1980133002/diff/140001/content/renderer/rende...
File content/renderer/render_widget_mouse_lock_dispatcher.h (right):

https://codereview.chromium.org/1980133002/diff/140001/content/renderer/rende...
content/renderer/render_widget_mouse_lock_dispatcher.h:20: //
RenderViewMouseLockDispatcher is owned by RenderViewImpl.
This comment needs to be updated, I think.

https://codereview.chromium.org/1980133002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (right):

https://codereview.chromium.org/1980133002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:651: if (page())
Is it actually possible to call this when there's no page()?  I would have
thought that things would have been torn down and callbacks cancelled.

https://codereview.chromium.org/1980133002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1980133002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebViewImpl.cpp:4449: if (WebFrameWidget* widget =
mainFrameImpl()->frameWidget()) {
It doesn't seem like anyone still needs to call this, so maybe we can just
remove it.

[lfg, 2016-06-06 20:09:43]

https://codereview.chromium.org/1980133002/diff/140001/content/renderer/rende...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/1980133002/diff/140001/content/renderer/rende...
content/renderer/render_widget.cc:393: bool should_add_route) {
On 2016/06/04 05:28:56, dcheng wrote:
> Can we just make RenderView::Initialize() not add the route itself instead of
> adding the bool?

Done.

https://codereview.chromium.org/1980133002/diff/140001/content/renderer/rende...
content/renderer/render_widget.cc:402: base::WrapUnique(new
RenderWidgetMouseLockDispatcher(this));
On 2016/06/04 05:28:56, dcheng wrote:
> Nit: base::MakeUnique, or be consistent with line 400 and use reset.

Done.

https://codereview.chromium.org/1980133002/diff/140001/content/renderer/rende...
File content/renderer/render_widget_mouse_lock_dispatcher.h (right):

https://codereview.chromium.org/1980133002/diff/140001/content/renderer/rende...
content/renderer/render_widget_mouse_lock_dispatcher.h:20: //
RenderViewMouseLockDispatcher is owned by RenderViewImpl.
On 2016/06/04 05:28:56, dcheng wrote:
> This comment needs to be updated, I think.

Done.

https://codereview.chromium.org/1980133002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (right):

https://codereview.chromium.org/1980133002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:651: if (page())
On 2016/06/04 05:28:56, dcheng wrote:
> Is it actually possible to call this when there's no page()?  I would have
> thought that things would have been torn down and callbacks cancelled.

No, I don't think it's possible. This check is unnecessary, removed.

https://codereview.chromium.org/1980133002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1980133002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebViewImpl.cpp:4449: if (WebFrameWidget* widget =
mainFrameImpl()->frameWidget()) {
On 2016/06/04 05:28:56, dcheng wrote:
> It doesn't seem like anyone still needs to call this, so maybe we can just
> remove it.

Done.

[dcheng, 2016-06-07 05:07:55]

LGTM

https://codereview.chromium.org/1980133002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1980133002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebViewImpl.cpp:2174: bool isPointerLocked =
false;
Sigh. One day we won't need this I guess =)

[lfg, 2016-06-07 19:00:59]

The CQ bit was checked by lfg@chromium.org

[lfg, 2016-06-07 19:01:00]

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org,
nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/1980133002/#ps180001
(title: "addressing comments")

[commit-bot: I haz the power, 2016-06-07 19:01:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1980133002/180001

[commit-bot: I haz the power, 2016-06-08 00:44:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-08 00:44:03]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[lfg, 2016-06-08 20:00:54]

The CQ bit was checked by lfg@chromium.org

[lfg, 2016-06-08 20:00:55]

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org,
dcheng@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/1980133002/#ps200001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-08 20:01:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1980133002/200001

[commit-bot: I haz the power, 2016-06-08 21:24:46]

Description was changed from

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

TEST=Manual test please:
Navigate to
http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html
and verify that toggling pointer lock from all iframes work
as well as exiting* pointer lock. Verify that the mouse
movement continues to be tracked in the frame that locked
the pointer.
* Exiting can be done by ESC key, ALT-TAB (on Mac Command
Tab), Windows key to bring up start menu etc.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

TEST=Manual test please:
Navigate to
http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html
and verify that toggling pointer lock from all iframes work
as well as exiting* pointer lock. Verify that the mouse
movement continues to be tracked in the frame that locked
the pointer.
* Exiting can be done by ESC key, ALT-TAB (on Mac Command
Tab), Windows key to bring up start menu etc.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-06-08 21:24:47]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-06-08 21:26:01]

Description was changed from

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

TEST=Manual test please:
Navigate to
http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html
and verify that toggling pointer lock from all iframes work
as well as exiting* pointer lock. Verify that the mouse
movement continues to be tracked in the frame that locked
the pointer.
* Exiting can be done by ESC key, ALT-TAB (on Mac Command
Tab), Windows key to bring up start menu etc.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Implement pointer lock API for out-of-process iframes.

This patch moves the MouseLockDispatcher from RenderView to RenderWidget and
implements the necessary plumbing to allow pointer locking from out-of-process
iframes.

TEST=Manual test please:
Navigate to
http://scheib.github.io/HTMLMisc/PointerLockAndFullscreen-iframes.html
and verify that toggling pointer lock from all iframes work
as well as exiting* pointer lock. Verify that the mouse
movement continues to be tracked in the frame that locked
the pointer.
* Exiting can be done by ESC key, ALT-TAB (on Mac Command
Tab), Windows key to bring up start menu etc.

BUG=601926,419087
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/bee1e0ad56ce6b9def191f6ec6ef5a71a2f5d83e
Cr-Commit-Position: refs/heads/master@{#398679}
==========

[commit-bot: I haz the power, 2016-06-08 21:26:02]

Patchset 11 (id:??) landed as
https://crrev.com/bee1e0ad56ce6b9def191f6ec6ef5a71a2f5d83e
Cr-Commit-Position: refs/heads/master@{#398679}