[webkit-reviews] review granted: [Bug 130059] Refactor Vibration algorithm to use only one timer. : [Attachment 226477] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jinwoo Jeong
<jw00.jeong at samsung.com>'s request for review:
Bug 130059: Refactor Vibration algorithm to use only one timer.
https://bugs.webkit.org/show_bug.cgi?id=130059

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=226477&action=review


> Source/WebCore/Modules/vibration/Vibration.cpp:72
> +    // Pre-exsiting instance need to be canceled when vibrate() is called.

Typo: "exsiting".

> Source/WebCore/Modules/vibration/Vibration.h:43
> +    // FIXME : When visibilitychange event is dispatched, if a vibration is
working, it should be canceled.

No space before the ":" in "FIXME:".

I don’t understand the meaning of the phrase “a vibration is working”.

> Source/WebCore/Modules/vibration/Vibration.h:58
> +    enum class State {
> +	   Idle,
> +	   Vibrating,
> +	   Waiting
> +    };

I’d suggest putting this on a single line instead of 5 lines.