Clear pattern to prevent timing problem between cancelVibration and vibrate

• Component: DOM
• Status: RESOLVED FIXED

[Kihong Kwon]

If there is javascript codes like below,

navigator.vibrate(1000);
navigator.vibrate(0);

cancelVibrate() can be called before calling vibrate().(It's timer timing issue)
In this case, cancelVibrate() is not working at all.

We need to fix this.

[Kihong Kwon]

Created attachment 158301 [details]
Patch

[Kihong Kwon]

CC haraken and morrita.

[Kentaro Hara]

Comment on attachment 158301 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        There is a timing issue in the cancelVibration.
> +        Since vibrate works based on timer, cancelVibration might be called
> +        eariler than vibrate when cancelVibration is called just after vibrate call.
> +        It can be prevented from clearing m_pattern in the cancelVibration.

Can't you write a layout test for this?

[Kihong Kwon]

(In reply to comment #3)
> (From update of attachment 158301 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=158301&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        There is a timing issue in the cancelVibration.
> > +        Since vibrate works based on timer, cancelVibration might be called
> > +        eariler than vibrate when cancelVibration is called just after vibrate call.
> > +        It can be prevented from clearing m_pattern in the cancelVibration.
> 
> Can't you write a layout test for this?

There is no return value in the Vibration API.
Therefore, if we add layout test, I think we have only way to use printf like legacy notification tests.(LayoutTests/fast/notifications/)
But IMHO it's not good to us.
I would like to get your opinion about this please. :)

[Kentaro Hara]

Comment on attachment 158301 [details]
Patch

Thanks, I understood that writing the test is difficult. The change looks reasonable, let's land it.

[WebKit Review Bot]

Comment on attachment 158301 [details]
Patch

Clearing flags on attachment: 158301

Committed r125624: <http://trac.webkit.org/changeset/125624>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.