Bug 76410

Created attachment 122693 [details] Patch

Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.

Created attachment 122930 [details] Same Patch; Retry Mac EWS

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 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 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"?

Created attachment 123620 [details] Patch

Created attachment 123700 [details] Patch for landing

Comment on attachment 123700 [details] Patch for landing Clearing flags on attachment: 123700 Committed r105720: <http://trac.webkit.org/changeset/105720>

All reviewed patches have been landed. Closing bug.