Add layout tests for ServiceWorker's notificationclose event.

Add layout tests for ServiceWorker's notificationclose event.

This patch implements a follow up test to
https://codereview.chromium.org/1619703002/

BUG=579150
TBR=sky (for //components/html_viewer/)
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html

Committed: https://crrev.com/24bd34b788d560f63aeef59942baac5eb4e58912
Cr-Commit-Position: refs/heads/master@{#374348}

[Nina, 2016-02-02 16:53:48]

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

[Nina, 2016-02-02 16:53:49]

nsatragno@chromium.org changed required reviewers:
+ peter@chromium.org

[Nina, 2016-02-02 16:53:49]

Hi Peter,

Could you give this patch a look?

Thanks!

[Peter Beverloo, 2016-02-02 23:50:10]

https://codereview.chromium.org/1656823002/diff/40002/content/shell/browser/l...
File content/shell/browser/layout_test/layout_test_notification_manager.cc
(right):

https://codereview.chromium.org/1656823002/diff/40002/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_notification_manager.cc:144:
page_iterator->second->NotificationClosed();
No need to check for in-page notifications here - those code paths are already
exercised by programmatically closing the notification.

https://codereview.chromium.org/1656823002/diff/40002/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js
(right):

https://codereview.chromium.org/1656823002/diff/40002/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js:109:
notification: notificationCopy});
micro nit: space before }

https://codereview.chromium.org/1656823002/diff/40002/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
(left):

https://codereview.chromium.org/1656823002/diff/40002/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html:57:
testRunner.simulateWebNotificationClick(scope, -1 /* action_index */);
Can we perhaps generalize the data reflection test in a function, and run it
twice? (Potentially extracting the actual testRunner call as a closure or so.)

This is effectively duplicating 70 LOC for one method call and one string of
difference.

[Nina, 2016-02-03 15:00:46]

Description was changed from

==========
[WIP] Add layout tests for ServiceWorker's notificationclose event.

BUG=
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
==========

to

==========
Add layout tests for ServiceWorker's notificationclose event.

This patch implements a follow up test to
https://codereview.chromium.org/1619703002/

BUG=579150
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
==========

[Nina, 2016-02-03 15:04:23]

nsatragno@chromium.org changed reviewers:
+ mkwst@chromium.org

[Nina, 2016-02-03 15:04:23]

nsatragno@chromium.org changed required reviewers:
+ mkwst@chromium.org

[Nina, 2016-02-03 15:04:23]

Mike, would you mind reviewing the IPC changes?

Peter, please see my updated patch.

Thanks.

https://codereview.chromium.org/1656823002/diff/40002/content/shell/browser/l...
File content/shell/browser/layout_test/layout_test_notification_manager.cc
(right):

https://codereview.chromium.org/1656823002/diff/40002/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_notification_manager.cc:144:
page_iterator->second->NotificationClosed();
On 2016/02/02 23:50:10, Peter Beverloo wrote:
> No need to check for in-page notifications here - those code paths are already
> exercised by programmatically closing the notification.

Removed.

https://codereview.chromium.org/1656823002/diff/40002/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js
(right):

https://codereview.chromium.org/1656823002/diff/40002/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/notifications/resources/instrumentation-service-worker.js:109:
notification: notificationCopy});
On 2016/02/02 23:50:10, Peter Beverloo wrote:
> micro nit: space before }

Done.

https://codereview.chromium.org/1656823002/diff/40002/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
(left):

https://codereview.chromium.org/1656823002/diff/40002/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html:57:
testRunner.simulateWebNotificationClick(scope, -1 /* action_index */);
On 2016/02/02 23:50:10, Peter Beverloo wrote:
> Can we perhaps generalize the data reflection test in a function, and run it
> twice? (Potentially extracting the actual testRunner call as a closure or so.)
> 
> This is effectively duplicating 70 LOC for one method call and one string of
> difference.

I refactored the logic into a different file and extracted the differences in an
object parameter. Please give it a look.

[Mike West, 2016-02-04 18:15:25]

the layout_test IPC changes LGTM, thanks.

[Peter Beverloo, 2016-02-08 18:22:00]

Description was changed from

==========
Add layout tests for ServiceWorker's notificationclose event.

This patch implements a follow up test to
https://codereview.chromium.org/1619703002/

BUG=579150
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
==========

to

==========
Add layout tests for ServiceWorker's notificationclose event.

This patch implements a follow up test to
https://codereview.chromium.org/1619703002/

BUG=579150
TBR=sky
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
==========

[Peter Beverloo, 2016-02-08 18:22:14]

Description was changed from

==========
Add layout tests for ServiceWorker's notificationclose event.

This patch implements a follow up test to
https://codereview.chromium.org/1619703002/

BUG=579150
TBR=sky
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
==========

to

==========
Add layout tests for ServiceWorker's notificationclose event.

This patch implements a follow up test to
https://codereview.chromium.org/1619703002/

BUG=579150
TBR=sky (for //components/html_viewer/)
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
==========

[Peter Beverloo, 2016-02-08 19:07:06]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2016-02-08 19:07:07]

lgtm

[commit-bot: I haz the power, 2016-02-08 19:09:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1656823002/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1656823002/70001

[commit-bot: I haz the power, 2016-02-08 21:57:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-08 21:57:45]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Nina, 2016-02-09 10:24:00]

The CQ bit was checked by nsatragno@chromium.org

[commit-bot: I haz the power, 2016-02-09 10:24:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1656823002/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1656823002/70001

[commit-bot: I haz the power, 2016-02-09 10:30:10]

Description was changed from

==========
Add layout tests for ServiceWorker's notificationclose event.

This patch implements a follow up test to
https://codereview.chromium.org/1619703002/

BUG=579150
TBR=sky (for //components/html_viewer/)
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
==========

to

==========
Add layout tests for ServiceWorker's notificationclose event.

This patch implements a follow up test to
https://codereview.chromium.org/1619703002/

BUG=579150
TBR=sky (for //components/html_viewer/)
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
==========

[commit-bot: I haz the power, 2016-02-09 10:30:11]

Committed patchset #5 (id:70001)

[commit-bot: I haz the power, 2016-02-09 10:31:24]

Description was changed from

==========
Add layout tests for ServiceWorker's notificationclose event.

This patch implements a follow up test to
https://codereview.chromium.org/1619703002/

BUG=579150
TBR=sky (for //components/html_viewer/)
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html
==========

to

==========
Add layout tests for ServiceWorker's notificationclose event.

This patch implements a follow up test to
https://codereview.chromium.org/1619703002/

BUG=579150
TBR=sky (for //components/html_viewer/)
TEST=http/tests/notifications/serviceworker-notificationclose-event-data-reflection.html

Committed: https://crrev.com/24bd34b788d560f63aeef59942baac5eb4e58912
Cr-Commit-Position: refs/heads/master@{#374348}
==========

[commit-bot: I haz the power, 2016-02-09 10:31:25]

Patchset 5 (id:??) landed as
https://crrev.com/24bd34b788d560f63aeef59942baac5eb4e58912
Cr-Commit-Position: refs/heads/master@{#374348}