[Webkit-unassigned] [Bug 130059] Refactor Vibration algorithm to use only 1 timer.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 10 19:33:03 PDT 2014


https://bugs.webkit.org/show_bug.cgi?id=130059





--- Comment #2 from Jinwoo Song <jinwoo7.song at samsung.com>  2014-03-10 19:29:59 PST ---
(From update of attachment 226363)
View in context: https://bugs.webkit.org/attachment.cgi?id=226363&action=review

> Source/WebCore/ChangeLog:3
> +        Refactor Vibration algorithm to use only 1 timer.

nit: s/1/one

> Source/WebCore/ChangeLog:8
> +        Currently Vibration has used 2 timer,

Writing suggestion: 'Currently Vibration is using two timers,'

> Source/WebCore/ChangeLog:13
> +        Indeed this patch applies the algorithm and restrictions defined on specification.

Writing suggestion: 'Also, this patch implement the missing part of the algorithm, which check the maximum length of the vibration pattern and the maximum duration of the vibration.

> Source/WebCore/Modules/vibration/Vibration.cpp:32
> +const unsigned MaxVibrationDuration = 10000;

Where these magic values come from?

> Source/WebCore/Modules/vibration/Vibration.cpp:87
>          return true;

These lines can be removed as it is already checked.

> Source/WebCore/Modules/vibration/Vibration.cpp:110
> +        m_state = Idle;

You may return here.

> Source/WebCore/Modules/vibration/Vibration.cpp:111
> +    else {

Why don't you use switch-case statement here?

> Source/WebCore/Modules/vibration/Vibration.h:-45
> -    // FIXME : Add suspendVibration() and resumeVibration() to the page visibility feature, when the document.hidden attribute is changed.

It's okay to remove these unused methods but It would be better to comment 'FIXME' for implementing page visibility feature.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list