Bug 76410 - [Chromium] Add WebKit API for Pointer Lock
Summary: [Chromium] Add WebKit API for Pointer Lock
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Vincent Scheib
URL:
Keywords:
Depends on:
Blocks: 75762 76411
  Show dependency treegraph
 
Reported: 2012-01-16 16:49 PST by Vincent Scheib
Modified: 2012-01-24 03:29 PST (History)
2 users (show)

See Also:


Attachments
Patch (8.36 KB, patch)
2012-01-16 16:54 PST, Vincent Scheib
no flags Details | Formatted Diff | Diff
Same Patch; Retry Mac EWS (8.36 KB, patch)
2012-01-18 07:40 PST, Vincent Scheib
no flags Details | Formatted Diff | Diff
Patch (9.12 KB, patch)
2012-01-23 14:28 PST, Vincent Scheib
no flags Details | Formatted Diff | Diff
Patch for landing (9.12 KB, patch)
2012-01-23 22:59 PST, Vincent Scheib
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vincent Scheib 2012-01-16 16:49:51 PST
[Chromium] Add WebKit API for Pointer Lock
Comment 1 Vincent Scheib 2012-01-16 16:54:31 PST
Created attachment 122693 [details]
Patch
Comment 2 WebKit Review Bot 2012-01-16 16:57:25 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 3 Vincent Scheib 2012-01-18 07:40:19 PST
Created attachment 122930 [details]
Same Patch; Retry Mac EWS
Comment 4 Darin Fisher (:fishd, Google) 2012-01-18 21:42:01 PST
Comment on attachment 122693 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122693&action=review

> Source/WebKit/chromium/public/WebWidget.h:175
> +    virtual void didCompletePointerLock() { }

Can you describe the processing model here?  WebKit calls requestPointerLock and then expects to receive one of these calls in reply, assuming requestPointerLock returned true?  what happens if WebKit calls requestPointerUnlock?

please write down some comments that document the call sequences.

do we need to distinguish between pointer lock being lost as a result of requestPointerUnlock
versus some other UA action?  can we follow the fullscreen pattern instead?  there we can
request entering / exiting fullscreen, and there are corresponding events telling us whether
we are in fullscreen or out of fullscreen.  in other words, there is only one notification
about exiting fullscreen.

> Source/WebKit/chromium/src/WebViewImpl.cpp:1335
> +    if (isPointerLocked() && WebInputEvent::isMouseEventType(inputEvent.type)) {

why doesn't this happen at the EventHandler level?

> Source/WebKit/chromium/src/WebViewImpl.cpp:1336
> +      pointerLockMouseEvent(inputEvent);

indentation
Comment 5 Vincent Scheib 2012-01-19 00:34:26 PST
Comment on attachment 122693 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122693&action=review

>> Source/WebKit/chromium/public/WebWidget.h:175
>> +    virtual void didCompletePointerLock() { }
> 
> Can you describe the processing model here?  WebKit calls requestPointerLock and then expects to receive one of these calls in reply, assuming requestPointerLock returned true?  what happens if WebKit calls requestPointerUnlock?
> 
> please write down some comments that document the call sequences.
> 
> do we need to distinguish between pointer lock being lost as a result of requestPointerUnlock
> versus some other UA action?  can we follow the fullscreen pattern instead?  there we can
> request entering / exiting fullscreen, and there are corresponding events telling us whether
> we are in fullscreen or out of fullscreen.  in other words, there is only one notification
> about exiting fullscreen.

I can comment the call pattern, but let's make sure we're happy with it:

The mouse lock spec differs from fullscreen a bit, which is why there are different call patterns here. Spec as it stands offers a 1 to 1 response of a lock request and the result of it being carried out or not. The response is delivered by callback to the specific functions provided when the lock was requested. That is different than the event that lock state is lost, which bubbles for anyone to see.

What happens if multiple elements attempt a fullscreen transition rapidly before the client can change state? With pointer lock, one and only one will receive a success response, and the others will receive failures. Mouse lock will transition to being enabled, and dispatching a lost event because some of the requests failed would be wrong. Dispatching a success event would be confusing, though I suppose the API could expose what the current target is and everyone could diligently check to see if mouse lock enabled for them.

So, expected call patterns are
- Call pointer.lock
- Receive a callback to one and only one of the optional success or failure callbacks you provided
- If lock is lost after success, receive a pointerlocklost event indicating it is lost
- Or, if lock is no longer desired call pointer.unlock, receive an asynch pointerlocklost event indicating it is lost

>> Source/WebKit/chromium/src/WebViewImpl.cpp:1335
>> +    if (isPointerLocked() && WebInputEvent::isMouseEventType(inputEvent.type)) {
> 
> why doesn't this happen at the EventHandler level?

To avoid the logic complexity that appears just below here and in the disparate calls made by different event types. Pointer lock intends to capture mouse input data as raw as it can, we don't want dragging, capture, DOM target selection, etc.
Comment 6 Darin Fisher (:fishd, Google) 2012-01-23 12:23:55 PST
Comment on attachment 122693 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122693&action=review

> Source/WebKit/chromium/public/WebWidget.h:176
> +    virtual void didNotCompletePointerLock() { }

maybe "Complete" -> "Acquire"?
Comment 7 Vincent Scheib 2012-01-23 14:28:28 PST
Created attachment 123620 [details]
Patch
Comment 8 Vincent Scheib 2012-01-23 22:59:29 PST
Created attachment 123700 [details]
Patch for landing
Comment 9 WebKit Review Bot 2012-01-24 03:29:18 PST
Comment on attachment 123700 [details]
Patch for landing

Clearing flags on attachment: 123700

Committed r105720: <http://trac.webkit.org/changeset/105720>
Comment 10 WebKit Review Bot 2012-01-24 03:29:24 PST
All reviewed patches have been landed.  Closing bug.