Introduce layout tests for SWR.getNotifications()

Introduce layout tests for SWR.getNotifications()

This CL introduces a series of layout tests for the getNotifications()
function, and exercises the function both in Document and Service Worker
contexts with and without a tag filter.

This CL is part of a two-sided patch:
  [1] https://codereview.chromium.org/1057573002/
  [2] This CL.

BUG=442143
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193991

[Peter Beverloo, 2015-04-01 16:31:48]

peter@chromium.org changed reviewers:
+ johnme@chromium.org

[Peter Beverloo, 2015-04-01 16:31:48]

+johnme

[johnme, 2015-04-09 10:29:50]

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
File
LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js
(right):

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js:4:
function serializeNotification(notification) {
Optional nit: cloneNotification?

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
File
LayoutTests/http/tests/notifications/serviceworkerregistration-get-from-service-worker.html
(right):

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/serviceworkerregistration-get-from-service-worker.html:35:
info.port.addEventListener('message', function(event) {
Nit: Technically it seems possible for the SW to reply before you've added the
event listener.

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
File
LayoutTests/http/tests/notifications/serviceworkerregistration-service-worker-get.html
(right):

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/serviceworkerregistration-service-worker-get.html:39:
info.port.addEventListener('message', function(event) {
Ditto

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/serviceworkerregistration-service-worker-get.html:64:
test.done();
Perhaps you could also test that registration.getNotifications() called from the
document is assert_array_equivalent to event.data.notifications?

[Peter Beverloo, 2015-04-17 13:03:17]

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
File
LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js
(right):

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js:4:
function serializeNotification(notification) {
On 2015/04/09 10:29:50, johnme wrote:
> Optional nit: cloneNotification?

It doesn't clone the notification (cloning would imply that we'd get an
identical object back), rather, it serializes it to textual form (JSON).

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
File
LayoutTests/http/tests/notifications/serviceworkerregistration-get-from-service-worker.html
(right):

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/serviceworkerregistration-get-from-service-worker.html:35:
info.port.addEventListener('message', function(event) {
On 2015/04/09 10:29:50, johnme wrote:
> Nit: Technically it seems possible for the SW to reply before you've added the
> event listener.

Executing a very minor amount of jitted JavaScript versus a cross-process
asynchronous operation (which also executes JavaScript) seems like a safe bet. I
haven't seen any flakiness in any of the other tests using this paradigm. The
benefit of keeping it like this is that the reading order of the patch matches
execution order.

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
File
LayoutTests/http/tests/notifications/serviceworkerregistration-service-worker-get.html
(right):

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/serviceworkerregistration-service-worker-get.html:39:
info.port.addEventListener('message', function(event) {
On 2015/04/09 10:29:50, johnme wrote:
> Ditto

Acknowledged.

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/serviceworkerregistration-service-worker-get.html:64:
test.done();
On 2015/04/09 10:29:50, johnme wrote:
> Perhaps you could also test that registration.getNotifications() called from
the
> document is assert_array_equivalent to event.data.notifications?

We have tests covering the correctness of both properties - without one of these
tests failing it seems impossible for that condition to be broken?

[johnme, 2015-04-17 16:29:25]

lgtm with nit

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
File
LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js
(right):

https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js:4:
function serializeNotification(notification) {
On 2015/04/17 13:03:17, Peter Beverloo wrote:
> On 2015/04/09 10:29:50, johnme wrote:
> > Optional nit: cloneNotification?
> 
> It doesn't clone the notification (cloning would imply that we'd get an
> identical object back), rather, it serializes it to textual form (JSON).

Yeah, but after serializing it to a string, it then deserializes it back to an
object. So it's more similar to cloning or deep copying.

[Peter Beverloo, 2015-04-17 16:30:04]

On 2015/04/17 16:29:25, johnme wrote:
> lgtm with nit
> 
>
https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
> File
>
LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js
> (right):
> 
>
https://codereview.chromium.org/1056573002/diff/1/LayoutTests/http/tests/noti...
>
LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js:4:
> function serializeNotification(notification) {
> On 2015/04/17 13:03:17, Peter Beverloo wrote:
> > On 2015/04/09 10:29:50, johnme wrote:
> > > Optional nit: cloneNotification?
> > 
> > It doesn't clone the notification (cloning would imply that we'd get an
> > identical object back), rather, it serializes it to textual form (JSON).
> 
> Yeah, but after serializing it to a string, it then deserializes it back to an
> object. So it's more similar to cloning or deep copying.

Done!

I'll submit this over the weekend as Blink cools down a little bit.

[Peter Beverloo, 2015-04-19 14:15:28]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2015-04-19 14:15:28]

The patchset sent to the CQ was uploaded after l-g-t-m from johnme@chromium.org
Link to the patchset: https://codereview.chromium.org/1056573002/#ps20001
(title: " ")

[commit-bot: I haz the power, 2015-04-19 14:15:34]

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

[commit-bot: I haz the power, 2015-04-19 15:56:51]

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