[webkit-reviews] review denied: [Bug 76411] [Chromium] Add Pointer Lock test hooks and mock implementation to DumpRenderTree : [Attachment 122695] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Vincent Scheib
<scheib at chromium.org>'s request for review:
Bug 76411: [Chromium] Add Pointer Lock test hooks and mock implementation to
DumpRenderTree
https://bugs.webkit.org/show_bug.cgi?id=76411

Attachment 122695: Patch
https://bugs.webkit.org/attachment.cgi?id=122695&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=122695&action=review


We had better enclose the code with #if ENABLE(POINTER_LOCK).

> Tools/DumpRenderTree/chromium/LayoutTestController.cpp:2178
> +void LayoutTestController::didLosePointerLock(const CppArgumentList&
arguments, CppVariant* result)

The arguments 'arguments' and 'result' are not used. We had better omit them.
void LayoutTestController::didLosePointerLock(const CppArgumentList&,
CppVariant*)

> Tools/DumpRenderTree/chromium/LayoutTestController.cpp:2184
> +void LayoutTestController::setPointerLockWillFailAsynchronously(const
CppArgumentList& arguments, CppVariant* result)

ditto.

> Tools/DumpRenderTree/chromium/LayoutTestController.cpp:2190
> +void LayoutTestController::setPointerLockWillFailSynchronously(const
CppArgumentList& arguments, CppVariant* result)

ditto.

> Tools/DumpRenderTree/chromium/WebViewHost.h:395
> +    enum {
> +	 POINTER_LOCK_WILL_SUCCEED,
> +	 POINTER_LOCK_WILL_FAIL_ASYNC,
> +	 POINTER_LOCK_WILL_FAIL_SYNC

Enum names should be
  PointerLockWillSucceed,
  PointerLockWillFailAsync,
  PointerLockWillFailSync

See "12. Enum members should user InterCaps with an initial capital letter." in
http://www.webkit.org/coding/coding-style.html