Add layouttest for notification properties of persistent notification

Add layouttest for notification of persistent notification

Tests that the persistent notification object exposes the data and vibrate
properties per the semantics defined by the specification.

BUG=442129, 442132, 467812
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=195403

[Sanghyun Park, 2015-04-21 08:44:00]

sh919.park@samsung.com changed reviewers:
+ peter@chromium.org

[Sanghyun Park, 2015-04-21 08:49:01]

Patchset #1 (id:1) has been deleted

[Peter Beverloo, 2015-04-21 12:26:37]

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-reflection.html
(right):

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-reflection.html:56:
assert_equals(event.data.notification.data, options.data, 'The data field must
be the same.');
Does this test pass? The chronology is rather odd - the tests need to be run in
sequence in order for this to work as expected, but right now they're running in
parallel...

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-throw.html
(right):

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-throw.html:4:
<title>Notifications: Verifying the exception throwing behavior, when data set
invalid value such as method.</title>
micro nit: as *a* method.

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-throw.html:29:
}).then(function() {
then(unreached_fulfillment(test))

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-throw.html:37:
}, 'showNotification() must reject If data is invalid value.');
micro nit: s/If/if/

[Sanghyun Park, 2015-04-21 13:38:34]

Thanks for your comments :).

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-reflection.html
(right):

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-reflection.html:56:
assert_equals(event.data.notification.data, options.data, 'The data field must
be the same.');
On 2015/04/21 12:26:36, Peter Beverloo wrote:
> Does this test pass? The chronology is rather odd - the tests need to be run
in
> sequence in order for this to work as expected, but right now they're running
in
> parallel...

Oops, I had mistake.
I'll re-make this.

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-throw.html
(right):

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-throw.html:4:
<title>Notifications: Verifying the exception throwing behavior, when data set
invalid value such as method.</title>
On 2015/04/21 12:26:36, Peter Beverloo wrote:
> micro nit: as *a* method.

Done.

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-throw.html:29:
}).then(function() {
On 2015/04/21 12:26:36, Peter Beverloo wrote:
> then(unreached_fulfillment(test))

Done.

https://codereview.chromium.org/1095093002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/notifications/serviceworkerregistration-document-data-throw.html:37:
}, 'showNotification() must reject If data is invalid value.');
On 2015/04/21 12:26:36, Peter Beverloo wrote:
> micro nit: s/If/if/

Done.

[Sanghyun Park, 2015-05-13 16:18:22]

Patchset #2 (id:40001) has been deleted

[Sanghyun Park, 2015-05-13 16:21:32]

Dear Peter.
Please take another look.

Thanks

[Peter Beverloo, 2015-05-14 16:13:36]

One question, the rest lg. Thanks!!

https://codereview.chromium.org/1095093002/diff/60001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-data-reflection.html
(right):

https://codereview.chromium.org/1095093002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-data-reflection.html:75:
assertNotificationDataReflects(0);
How long does it take to run this test?

I'm a bit hesitant to adding this, because we'd end up testing the serialization
behavior which is covered by other bits. If it's reasonably fast then we could
have it for additional coverage, but if it takes long (which is hard to define)
it may not be worth it.

[Sanghyun Park, 2015-05-15 08:52:35]

Thanks for your review :) 

Please refer below comments

https://codereview.chromium.org/1095093002/diff/60001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-data-reflection.html
(right):

https://codereview.chromium.org/1095093002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-data-reflection.html:75:
assertNotificationDataReflects(0);
On 2015/05/14 16:13:36, Peter Beverloo wrote:
> How long does it take to run this test?
> 
> I'm a bit hesitant to adding this, because we'd end up testing the
serialization
> behavior which is covered by other bits. If it's reasonably fast then we could
> have it for additional coverage, but if it takes long (which is hard to
define)
> it may not be worth it.

Results of the tests done, it takes less than one second.
It seems no problem. :)

[Peter Beverloo, 2015-05-15 09:03:59]

OK, thank you for checking!

You really should consider sending out an Intent to Ship for
Notification.vibrate. It's in good shape to be included for Chrome 45.

lgtm

[Sanghyun Park, 2015-05-15 09:06:47]

On 2015/05/15 09:03:59, Peter Beverloo wrote:
> OK, thank you for checking!
> 
> You really should consider sending out an Intent to Ship for
> Notification.vibrate. It's in good shape to be included for Chrome 45.
> 
> lgtm

Thanks so much!.

I'll send intent to Ship. :)

[Sanghyun Park, 2015-05-15 09:07:06]

The CQ bit was checked by sh919.park@samsung.com

[commit-bot: I haz the power, 2015-05-15 09:07:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1095093002/60001

[commit-bot: I haz the power, 2015-05-15 10:40:20]

Committed patchset #2 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195403