Add NotificationEvent.action property in SW events

Add NotificationEvent.action property in SW events

Spec: https://github.com/whatwg/notifications/pull/48

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/6tlbJW0NjEA/LF0kPfJx7c0J

Part of a sequence of patches:
- https://codereview.chromium.org/1254423011 (blink)
- https://codereview.chromium.org/1263043002 (blink)
- https://codereview.chromium.org/1263043003 (blink)
- https://codereview.chromium.org/1268813007 (blink)
- https://codereview.chromium.org/1267673003 (chromium)
- this patch (blink)
- https://codereview.chromium.org/1269923003 (chromium)
- https://codereview.chromium.org/1262893006 (chromium)
- https://codereview.chromium.org/1264403002 (blink)

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

[johnme, 2015-08-04 15:56:54]

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

[Peter Beverloo, 2015-08-04 21:01:23]

Please update serviceworker-notification-event.js as well (which tests the event
+ the init dictionary).

lgtm % that, and two comments.

https://codereview.chromium.org/1262893005/diff/1/LayoutTests/http/tests/noti...
File
LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-reflection.html
(right):

https://codereview.chromium.org/1262893005/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-reflection.html:72:
if (key == "actions")
Single quotes. (Consistency with the file.)

https://codereview.chromium.org/1262893005/diff/1/Source/web/ServiceWorkerGlo...
File Source/web/ServiceWorkerGlobalScopeProxy.cpp (right):

https://codereview.chromium.org/1262893005/diff/1/Source/web/ServiceWorkerGlo...
Source/web/ServiceWorkerGlobalScopeProxy.cpp:147: if (actionIndex >= 0)
Make sure that it's in bounds (upper-bound as well).

https://codereview.chromium.org/1262893005/diff/1/Source/web/ServiceWorkerGlo...
File Source/web/ServiceWorkerGlobalScopeProxy.h (right):

https://codereview.chromium.org/1262893005/diff/1/Source/web/ServiceWorkerGlo...
Source/web/ServiceWorkerGlobalScopeProxy.h:80: void
dispatchNotificationClickEvent(int, int64_t notificationID, const
WebNotificationData&, int actionIndex) override;
Mm. Why did you decide to pass an int for the action index rather than passing
the action as a String? Now you need weird -1 values rather than passing the
empty string.

[johnme, 2015-08-05 11:56:22]

Addressed review comments - thanks :)

> Please update serviceworker-notification-event.js as well
> (which tests the event + the init dictionary).

Done

https://codereview.chromium.org/1262893005/diff/1/LayoutTests/http/tests/noti...
File
LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-reflection.html
(right):

https://codereview.chromium.org/1262893005/diff/1/LayoutTests/http/tests/noti...
LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-reflection.html:72:
if (key == "actions")
On 2015/08/04 21:01:23, Peter Beverloo wrote:
> Single quotes. (Consistency with the file.)

Done.

https://codereview.chromium.org/1262893005/diff/1/Source/web/ServiceWorkerGlo...
File Source/web/ServiceWorkerGlobalScopeProxy.cpp (right):

https://codereview.chromium.org/1262893005/diff/1/Source/web/ServiceWorkerGlo...
Source/web/ServiceWorkerGlobalScopeProxy.cpp:147: if (actionIndex >= 0)
On 2015/08/04 21:01:23, Peter Beverloo wrote:
> Make sure that it's in bounds (upper-bound as well).

Done.

https://codereview.chromium.org/1262893005/diff/1/Source/web/ServiceWorkerGlo...
File Source/web/ServiceWorkerGlobalScopeProxy.h (right):

https://codereview.chromium.org/1262893005/diff/1/Source/web/ServiceWorkerGlo...
Source/web/ServiceWorkerGlobalScopeProxy.h:80: void
dispatchNotificationClickEvent(int, int64_t notificationID, const
WebNotificationData&, int actionIndex) override;
On 2015/08/04 21:01:23, Peter Beverloo wrote:
> Mm. Why did you decide to pass an int for the action index rather than passing
> the action as a String? Now you need weird -1 values rather than passing the
> empty string.

In the lower layers (ui::messagecenter::ButtonInfo etc) we don't know the
|action|s of the buttons, we only store their |title|s and get back a button
index. So passing the string would require a whole lot more plumbing.

I could define a kInvalidNotificationActionIndex constant in
content/public/common/platform_notification_data.h if you think that'd help?

[johnme, 2015-08-05 12:01:27]

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

[johnme, 2015-08-05 12:01:28]

jochen@chromium.org: Please review changes in public/web/ & Source/web/. Thanks!

[jochen (gone - plz use gerrit), 2015-08-05 15:00:09]

lgtm

[johnme, 2015-08-05 16:31:16]

The CQ bit was checked by johnme@chromium.org

[johnme, 2015-08-05 16:31:16]

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

[commit-bot: I haz the power, 2015-08-05 16:31:28]

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

[commit-bot: I haz the power, 2015-08-05 21:08:55]

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

[commit-bot: I haz the power, 2015-08-05 21:08:56]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/72674)

[johnme, 2015-08-06 09:54:48]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2015-08-06 09:54:59]

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

[commit-bot: I haz the power, 2015-08-06 10:55:20]

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