Silent mouse lock after unlock by target

- Added test for silent mouse lock after unlocked by target, added argument to mouse lock request in tests for after_target_unlock_request.

- mouse lock dispatcher keeps track of voluntary unlocks by the target, and adds this as a parameter for the lock request.  Fullscreen controller will silently lock the mouse when the mouse was previously voluntarily unlocked by the target.

Contributed by dmurph@chromium.com
BUG=113460
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=139934

[jochen (gone - plz use gerrit), 2012-05-25 21:53:46]

drive-by content review

https://chromiumcodereview.appspot.com/10443045/diff/1/content/browser/render...
File content/browser/renderer_host/render_view_host_delegate.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/1/content/browser/render...
content/browser/renderer_host/render_view_host_delegate.h:331: bool
after_unlocked_by_target) {
align arguments, here and everywhere else

https://chromiumcodereview.appspot.com/10443045/diff/1/content/browser/render...
File content/browser/renderer_host/render_widget_host_impl.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/1/content/browser/render...
content/browser/renderer_host/render_widget_host_impl.h:410: virtual void
RequestToLockMouse(bool user_gesture,
the fact that this requires the presence of a RVH means that pepper fullscreen
can't use this code path. Is this intended?

[scheib, 2012-05-26 01:12:34]

jochen, a reply for you included.

https://chromiumcodereview.appspot.com/10443045/diff/1/chrome/browser/ui/full...
File chrome/browser/ui/fullscreen_controller.cc (right):

https://chromiumcodereview.appspot.com/10443045/diff/1/chrome/browser/ui/full...
chrome/browser/ui/fullscreen_controller.cc:75: // Must have a user gesture, or
we must already be in tab fullscreen.
Update comment, e.g.: 
Must have a user gesture to prevent misbehaving sites from constantly re-locking
the mouse. Exceptions are when the page has unlocked (i.e. not the user), or if
we're in tab fullscreen (user gesture required for that).

https://chromiumcodereview.appspot.com/10443045/diff/1/chrome/browser/ui/full...
File chrome/browser/ui/fullscreen_controller.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/1/chrome/browser/ui/full...
chrome/browser/ui/fullscreen_controller.h:92: // Mouse lock has been silently
accepted, no notification to user
nit: Add a period.

https://chromiumcodereview.appspot.com/10443045/diff/1/chrome/browser/ui/full...
chrome/browser/ui/fullscreen_controller.h:93: MOUSELOCK_ACCEPTED_SILENT
Sorry to change name suggestion, but please rename to ..._SILENTLY

https://chromiumcodereview.appspot.com/10443045/diff/1/content/browser/render...
File content/browser/renderer_host/render_widget_host_impl.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/1/content/browser/render...
content/browser/renderer_host/render_widget_host_impl.h:410: virtual void
RequestToLockMouse(bool user_gesture,
On 2012/05/25 21:53:46, jochen wrote:
> the fact that this requires the presence of a RVH means that pepper fullscreen
> can't use this code path. Is this intended?

Currently this is only used by web pages containing pepper/NaCl or javascript
applications.

We do have a pepper example that uses fullscreen and mouse lock:
src/ppapi/examples/mouse_lock/mouse_lock.cc

yzshen initially placed the mouse lock etc code at the RenderWidget level
because the implementation is done in the platform specific RenderWidgetHostView
objects, and related IPC messages already pass through the Widget. yzshen also
intends to add additional support for non RenderViewHost instances using mouse
lock, and will add implementation when needed.

https://chromiumcodereview.appspot.com/10443045/diff/1/content/renderer/mouse...
File content/renderer/mouse_lock_dispatcher.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/1/content/renderer/mouse...
content/renderer/mouse_lock_dispatcher.h:79: bool unlocked_by_target_;
We should comment. e.g. Used when locking to indicate when a target application
has voluntarily unlocked and desires to relock the mouse. If The mouse is
unlocked due to ESC being pressed by the user, this will not be true.

[dmurph, 2012-05-30 00:17:50]

I changed to booleans to enums, and added them in the mouse_lock.h file in
content/common.

https://chromiumcodereview.appspot.com/10443045/diff/1/chrome/browser/ui/full...
File chrome/browser/ui/fullscreen_controller.cc (right):

https://chromiumcodereview.appspot.com/10443045/diff/1/chrome/browser/ui/full...
chrome/browser/ui/fullscreen_controller.cc:75: // Must have a user gesture, or
we must already be in tab fullscreen.
On 2012/05/26 01:12:34, scheib wrote:
> Update comment, e.g.: 
> Must have a user gesture to prevent misbehaving sites from constantly
re-locking
> the mouse. Exceptions are when the page has unlocked (i.e. not the user), or
if
> we're in tab fullscreen (user gesture required for that).

ok

https://chromiumcodereview.appspot.com/10443045/diff/1/chrome/browser/ui/full...
File chrome/browser/ui/fullscreen_controller.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/1/chrome/browser/ui/full...
chrome/browser/ui/fullscreen_controller.h:92: // Mouse lock has been silently
accepted, no notification to user
On 2012/05/26 01:12:34, scheib wrote:
> nit: Add a period.

Done.

https://chromiumcodereview.appspot.com/10443045/diff/1/chrome/browser/ui/full...
chrome/browser/ui/fullscreen_controller.h:93: MOUSELOCK_ACCEPTED_SILENT
On 2012/05/26 01:12:34, scheib wrote:
> Sorry to change name suggestion, but please rename to ..._SILENTLY

Done.

https://chromiumcodereview.appspot.com/10443045/diff/1/content/browser/render...
File content/browser/renderer_host/render_view_host_delegate.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/1/content/browser/render...
content/browser/renderer_host/render_view_host_delegate.h:331: bool
after_unlocked_by_target) {
On 2012/05/25 21:53:46, jochen wrote:
> align arguments, here and everywhere else

ok

https://chromiumcodereview.appspot.com/10443045/diff/1/content/renderer/mouse...
File content/renderer/mouse_lock_dispatcher.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/1/content/renderer/mouse...
content/renderer/mouse_lock_dispatcher.h:79: bool unlocked_by_target_;
On 2012/05/26 01:12:34, scheib wrote:
> We should comment. e.g. Used when locking to indicate when a target
application
> has voluntarily unlocked and desires to relock the mouse. If The mouse is
> unlocked due to ESC being pressed by the user, this will not be true.

Done.

[dmurph, 2012-05-30 00:36:44]

Also, I could move the serialization definitions to a .cc file, that might be
cleaner

[scheib, 2012-05-30 16:41:21]

Comments below, but at a high level I'm getting a bad sense that the amount of
code complexity needed to include these binary state enums is not worth it.

https://chromiumcodereview.appspot.com/10443045/diff/9001/chrome/browser/ui/b...
File chrome/browser/ui/browser.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/9001/chrome/browser/ui/b...
chrome/browser/ui/browser.h:992: content::MouseLockSource lock_source,
Comment: This naming has become unfortunately abstract now. Previously it was
quite clear, user_gesture == true or false. Now it's the mouse lock source... is
that the source UI element, or the requesting source, or... oh, after finding
the file with enum values I see it's user gesture related. Essentially we trade
the readability of the single point of value specification vs the readability
through all the methods that pass/receive the parameter. Off hand I haven't
thought of a better enum name that indicates it's user gesture related, but give
it a shot.

https://chromiumcodereview.appspot.com/10443045/diff/9001/chrome/browser/ui/b...
File chrome/browser/ui/browser_browsertest.cc (right):

https://chromiumcodereview.appspot.com/10443045/diff/9001/chrome/browser/ui/b...
chrome/browser/ui/browser_browsertest.cc:267: void
RequestToLockMouse(content::WebContents* tab, bool user_gesture,
If we go with enums, we should plumb them all the way through to the call sites.

https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
File content/common/mouse_lock.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
content/common/mouse_lock.h:1: // Copyright (c) 2010 The Chromium Authors. All
rights reserved.
2012

https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
content/common/mouse_lock.h:15: // The source of a mouse lock request
Punctuation:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Punctu...

https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
content/common/mouse_lock.h:18: USER_GESTURE,
These enum names are too generic for the content namespace, e.g.
content::USER_GESTURE. They should have name prefixes that give them context,
e.g. as in view_type.h, page_zoom.h p2p_sockets.h; or should be enclosed in a
class or additional namespace. (I prefer former).

Some ideas:
MOUSE_LOCK_VIA_USER_GESTURE,
MOUSE_LOCK_NO_USER_GESTURE

MOUSE_UNLOCKED_BY_LOCK_TARGET,
MOUSE_UNLOCKED_BY_BROWSER

https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
content/common/mouse_lock.h:33: // param traits for serialization over IPC
As you mention, the implementation should be placed in a CC file. You'll need to
add this .h and the .cc to src/content/content_common.gypi

https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
content/common/mouse_lock.h:37: typedef content::MouseLockSource param_type;
IMHO the typedef param_type makes this code less readable. I see it's use in
some other bits of code, but I also see examples of direct use of types and
prefer those.

https://chromiumcodereview.appspot.com/10443045/diff/9001/content/renderer/mo...
File content/renderer/mouse_lock_dispatcher.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/9001/content/renderer/mo...
content/renderer/mouse_lock_dispatcher.h:82: // to ESC being pressed by the
user, this will NOT_TARGET
s/will/will be/, and add a period.

[dmurph, 2012-05-30 17:44:37]

On 2012/05/30 16:41:21, scheib wrote:
> Comments below, but at a high level I'm getting a bad sense that the amount of
> code complexity needed to include these binary state enums is not worth it.
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/chrome/browser/ui/b...
> File chrome/browser/ui/browser.h (right):
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/chrome/browser/ui/b...
> chrome/browser/ui/browser.h:992: content::MouseLockSource lock_source,
> Comment: This naming has become unfortunately abstract now. Previously it was
> quite clear, user_gesture == true or false. Now it's the mouse lock source...
is
> that the source UI element, or the requesting source, or... oh, after finding
> the file with enum values I see it's user gesture related. Essentially we
trade
> the readability of the single point of value specification vs the readability
> through all the methods that pass/receive the parameter. Off hand I haven't
> thought of a better enum name that indicates it's user gesture related, but
give
> it a shot.
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/chrome/browser/ui/b...
> File chrome/browser/ui/browser_browsertest.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/chrome/browser/ui/b...
> chrome/browser/ui/browser_browsertest.cc:267: void
> RequestToLockMouse(content::WebContents* tab, bool user_gesture,
> If we go with enums, we should plumb them all the way through to the call
sites.
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
> File content/common/mouse_lock.h (right):
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
> content/common/mouse_lock.h:1: // Copyright (c) 2010 The Chromium Authors. All
> rights reserved.
> 2012
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
> content/common/mouse_lock.h:15: // The source of a mouse lock request
> Punctuation:
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Punctu...
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
> content/common/mouse_lock.h:18: USER_GESTURE,
> These enum names are too generic for the content namespace, e.g.
> content::USER_GESTURE. They should have name prefixes that give them context,
> e.g. as in view_type.h, page_zoom.h p2p_sockets.h; or should be enclosed in a
> class or additional namespace. (I prefer former).
> 
> Some ideas:
> MOUSE_LOCK_VIA_USER_GESTURE,
> MOUSE_LOCK_NO_USER_GESTURE
> 
> MOUSE_UNLOCKED_BY_LOCK_TARGET,
> MOUSE_UNLOCKED_BY_BROWSER
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
> content/common/mouse_lock.h:33: // param traits for serialization over IPC
> As you mention, the implementation should be placed in a CC file. You'll need
to
> add this .h and the .cc to src/content/content_common.gypi
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/content/common/mous...
> content/common/mouse_lock.h:37: typedef content::MouseLockSource param_type;
> IMHO the typedef param_type makes this code less readable. I see it's use in
> some other bits of code, but I also see examples of direct use of types and
> prefer those.
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/content/renderer/mo...
> File content/renderer/mouse_lock_dispatcher.h (right):
> 
>
https://chromiumcodereview.appspot.com/10443045/diff/9001/content/renderer/mo...
> content/renderer/mouse_lock_dispatcher.h:82: // to ESC being pressed by the
> user, this will NOT_TARGET
> s/will/will be/, and add a period.


I completely agree, the overhead with IPC and namespace issues definitely make
it not worth the enums.  I'll change back, fix comments, and re-upload

[scheib, 2012-05-30 20:10:05]

lgtm, time to find OWNER(s) to add to this review. 

You alternate between after_unlocked_by_target vs last_unlocked_by_target in
different files, pick one.

http://codereview.chromium.org/10443045/diff/16001/content/renderer/mouse_loc...
File content/renderer/mouse_lock_dispatcher.cc (right):

http://codereview.chromium.org/10443045/diff/16001/content/renderer/mouse_loc...
content/renderer/mouse_lock_dispatcher.cc:37: Send(
Too trivial to bother modifying this file.

[dmurph, 2012-05-30 20:54:30]

Naming and syntax all fixed.  Test passes.

https://chromiumcodereview.appspot.com/10443045/diff/16001/content/renderer/m...
File content/renderer/mouse_lock_dispatcher.cc (right):

https://chromiumcodereview.appspot.com/10443045/diff/16001/content/renderer/m...
content/renderer/mouse_lock_dispatcher.cc:37: Send(
On 2012/05/30 20:10:06, scheib wrote:
> Too trivial to bother modifying this file.

Done.

[sky, 2012-05-30 22:48:26]

LGTM

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/b...
File chrome/browser/ui/browser_browsertest.cc (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/b...
chrome/browser/ui/browser_browsertest.cc:267: void
RequestToLockMouse(content::WebContents* tab, bool user_gesture,
nit: when you wrap, wrap each param on its own line.

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/f...
File chrome/browser/ui/fullscreen_controller.cc (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/f...
chrome/browser/ui/fullscreen_controller.cc:67: || mouse_lock_state_ ==
MOUSELOCK_ACCEPTED_SILENTLY;
|| on previous line

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/public/brow...
File content/public/browser/web_contents_delegate.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/public/brow...
content/public/browser/web_contents_delegate.h:395: bool /* user_gesture */,
Don't comment out parameters like this (the rest of the class doesn't do this).

Two boolean parameters like this becomes a bit unreadable. Did you consider a
bitmask?

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/renderer/mo...
File content/renderer/mouse_lock_dispatcher.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/renderer/mo...
content/renderer/mouse_lock_dispatcher.h:81: // to ESC being pressed by the
user, this will false
'will false' -> 'will be false.'

[jam, 2012-05-31 06:05:23]

lgtm

http://codereview.chromium.org/10443045/diff/7038/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

http://codereview.chromium.org/10443045/diff/7038/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:3651:
fullscreen_controller_->RequestToLockMouse(tab, user_gesture,
nit: this tabbing of parameters is against the style guide. the first parameter
on a line has to start at the same indentation as the line before

http://codereview.chromium.org/10443045/diff/7038/chrome/browser/ui/fullscree...
File chrome/browser/ui/fullscreen_controller.cc (right):

http://codereview.chromium.org/10443045/diff/7038/chrome/browser/ui/fullscree...
chrome/browser/ui/fullscreen_controller.cc:81: &&
!IsFullscreenForTabOrPending(tab)) {
nit: && on previous line

http://codereview.chromium.org/10443045/diff/7038/content/browser/renderer_ho...
File content/browser/renderer_host/render_view_host_delegate.h (right):

http://codereview.chromium.org/10443045/diff/7038/content/browser/renderer_ho...
content/browser/renderer_host/render_view_host_delegate.h:332: }
nit: follow the same style in this class and put the brace brackets together

http://codereview.chromium.org/10443045/diff/7038/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

http://codereview.chromium.org/10443045/diff/7038/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.cc:1044: bool /*
user_gesture */,
nit: no need to put these in comments, we don't use that style for unused
parameters in this file

[dmurph, 2012-05-31 21:13:49]

Fixed nit changes.  Ready for submit.

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/b...
File chrome/browser/ui/browser.cc (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/b...
chrome/browser/ui/browser.cc:3651:
fullscreen_controller_->RequestToLockMouse(tab, user_gesture,
On 2012/05/31 06:05:23, John Abd-El-Malek wrote:
> nit: this tabbing of parameters is against the style guide. the first
parameter
> on a line has to start at the same indentation as the line before

Done.

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/b...
File chrome/browser/ui/browser_browsertest.cc (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/b...
chrome/browser/ui/browser_browsertest.cc:267: void
RequestToLockMouse(content::WebContents* tab, bool user_gesture,
On 2012/05/30 22:48:27, sky wrote:
> nit: when you wrap, wrap each param on its own line.

Done.

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/f...
File chrome/browser/ui/fullscreen_controller.cc (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/f...
chrome/browser/ui/fullscreen_controller.cc:67: || mouse_lock_state_ ==
MOUSELOCK_ACCEPTED_SILENTLY;
On 2012/05/30 22:48:27, sky wrote:
> || on previous line

Done.

https://chromiumcodereview.appspot.com/10443045/diff/7038/chrome/browser/ui/f...
chrome/browser/ui/fullscreen_controller.cc:81: &&
!IsFullscreenForTabOrPending(tab)) {
On 2012/05/31 06:05:23, John Abd-El-Malek wrote:
> nit: && on previous line

Done.

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/browser/ren...
File content/browser/renderer_host/render_view_host_delegate.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/browser/ren...
content/browser/renderer_host/render_view_host_delegate.h:332: }
On 2012/05/31 06:05:23, John Abd-El-Malek wrote:
> nit: follow the same style in this class and put the brace brackets together

Done.

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/browser/ren...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/browser/ren...
content/browser/renderer_host/render_widget_host_impl.cc:1044: bool /*
user_gesture */,
On 2012/05/31 06:05:23, John Abd-El-Malek wrote:
> nit: no need to put these in comments, we don't use that style for unused
> parameters in this file

Done.

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/public/brow...
File content/public/browser/web_contents_delegate.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/public/brow...
content/public/browser/web_contents_delegate.h:395: bool /* user_gesture */,
We tried enums, we considered bitmasks, and we concluded that booleans maximize
readability for this case.

Done.

On 2012/05/30 22:48:27, sky wrote:
> Don't comment out parameters like this (the rest of the class doesn't do
this).
> 
> Two boolean parameters like this becomes a bit unreadable. Did you consider a
> bitmask?

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/renderer/mo...
File content/renderer/mouse_lock_dispatcher.h (right):

https://chromiumcodereview.appspot.com/10443045/diff/7038/content/renderer/mo...
content/renderer/mouse_lock_dispatcher.h:81: // to ESC being pressed by the
user, this will false
On 2012/05/30 22:48:27, sky wrote:
> 'will false' -> 'will be false.'

Done.

[commit-bot: I haz the power, 2012-05-31 21:35:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmurph@chromium.org/10443045/6005

[commit-bot: I haz the power, 2012-06-01 00:15:52]

Change committed as 139934

[yzshen1, 2012-06-03 08:44:47]

On 2012/06/01 00:15:52, I haz the power (commit-bot) wrote:
> Change committed as 139934

I just saw this CL.
WRT the change in mouse_lock_dispatcher, is the following sequence possible?
- target sends a lock mouse request
- before the request is completed, target sends an unlock request. As a result,
unlocked_by_target_ is set to true
- now the target can silently lock mouse.

[scheib, 2012-06-04 17:09:24]

This is a problem, I've got a patch in progress to fix it. Note, this isn't
exposed to all web sites, only sites that have previously been approved by the
user and saved to content settings. Then, those sites must send a lock and
unlock request before the browser process can permit the lock.

[dmurph1, 2012-06-04 17:19:03]

On 2012/06/03 08:44:47, yzshen1 wrote:
> On 2012/06/01 00:15:52, I haz the power (commit-bot) wrote:
> > Change committed as 139934
> 
> I just saw this CL.
> WRT the change in mouse_lock_dispatcher, is the following sequence possible?
> - target sends a lock mouse request
> - before the request is completed, target sends an unlock request. As a
result,
> unlocked_by_target_ is set to true
> - now the target can silently lock mouse.

Not quite, but there is a small bug.  So even if unlocked_by_target_ is true, we
can only silently lock the mouse if the user has previously authorized the page
to lock the mouse (this is handled by the full screen controller)

So worst case scenario here is that the user has pressed 'esc' to unlock the
mouse, and then the webpage can do what you described to re-lock the mouse
without having a notification pop up.  So definitely a bug, not as bad though

[dmurph1, 2012-06-04 17:19:09]

On 2012/06/03 08:44:47, yzshen1 wrote:
> On 2012/06/01 00:15:52, I haz the power (commit-bot) wrote:
> > Change committed as 139934
> 
> I just saw this CL.
> WRT the change in mouse_lock_dispatcher, is the following sequence possible?
> - target sends a lock mouse request
> - before the request is completed, target sends an unlock request. As a
result,
> unlocked_by_target_ is set to true
> - now the target can silently lock mouse.

Not quite, but there is a small bug.  So even if unlocked_by_target_ is true, we
can only silently lock the mouse if the user has previously authorized the page
to lock the mouse (this is handled by the full screen controller)

So worst case scenario here is that the user has pressed 'esc' to unlock the
mouse, and then the webpage can do what you described to re-lock the mouse
without having a notification pop up.  So definitely a bug, not as bad though

[scheib, 2012-06-04 17:36:28]

On 2012/06/04 17:09:24, scheib wrote:
> This is a problem, I've got a patch in progress to fix it. Note, this isn't
> exposed to all web sites, only sites that have previously been approved by the
> user and saved to content settings. Then, those sites must send a lock and
> unlock request before the browser process can permit the lock.

http://codereview.chromium.org/10512011/