Implement the ServiceWorkerRegistration.getNotifications() API.

Implement the ServiceWorkerRegistration.getNotifications() API.

Now that the embedder implements the new Blink APIs, implement the Web
exposed API and hook it up with the API. This currently provides a
full renderer -> browser -> renderer round-trip, but no implementation
on the browser side yet. As such, only add a test to ensure that the
path works for the no-notifications-at-all case.

When the browser implements the ability to retrieve open notifications,
layout tests will be added accordingly. Until then, add a test which
gets notifications for a page that has none, to make sure we do
exercise the code path.

This feature is part of the following Intent to Implement:
  https://groups.google.com/a/chromium.org/d/msg/blink-dev/gb8QoBKJnGU/jKcPVOtnEVoJ

This CL is part of a three-sided patch:
  [1] https://codereview.chromium.org/1018613003/
  [2] https://codereview.chromium.org/1014073002/
  [3] This CL.

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

[Peter Beverloo, 2015-03-17 17:43:11]

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

[Peter Beverloo, 2015-03-17 17:43:12]

+johnme for notifications
+mkwst FYI

[johnme, 2015-03-17 19:39:35]

non-owner lgtm with optional nit (though I'd encourage Mike to double-check
NotificationArray, as I'm not so familiar with oilpan)

https://codereview.chromium.org/1016843002/diff/1/Source/modules/notification...
File Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp
(right):

https://codereview.chromium.org/1016843002/diff/1/Source/modules/notification...
Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp:41:
const WebPersistentNotificationInfo& notificationInfo = (*notificationInfos)[i];
Nit - how about:

for (const WebPersistentNotificationInfo& notificationInfo : *notificationInfos)

You'll have to wait for https://codereview.chromium.org/1014933002 to land first
though!

[Mike West, 2015-03-17 20:09:39]

mkwst@chromium.org changed reviewers:
+ oilpan-reviews@chromium.org

[Mike West, 2015-03-17 20:09:40]

The code change LGTM. Tagging oilpan-reviews@ for someone to give it a second
look from that perspective.

[haraken, 2015-03-18 01:32:54]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2015-03-18 01:32:54]

Oilpan bits LGTM.

https://codereview.chromium.org/1016843002/diff/1/Source/modules/notification...
File Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp
(right):

https://codereview.chromium.org/1016843002/diff/1/Source/modules/notification...
Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp:50:
delete notificationInfosRaw;

I don't fully understand why we need to take a raw pointer and explicitly call
delete. It would be better if we can delegate the work to the normal OwnPtr
mechanism.

[Peter Beverloo, 2015-03-19 15:43:07]

https://codereview.chromium.org/1016843002/diff/1/Source/modules/notification...
File Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp
(right):

https://codereview.chromium.org/1016843002/diff/1/Source/modules/notification...
Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp:41:
const WebPersistentNotificationInfo& notificationInfo = (*notificationInfos)[i];
On 2015/03/17 19:39:35, johnme wrote:
> Nit - how about:
> 
> for (const WebPersistentNotificationInfo& notificationInfo :
*notificationInfos)
> 
> You'll have to wait for https://codereview.chromium.org/1014933002 to land
first
> though!

I waited and applied it :-). Thanks!

https://codereview.chromium.org/1016843002/diff/1/Source/modules/notification...
Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp:50:
delete notificationInfosRaw;
On 2015/03/18 01:32:54, haraken wrote:
> 
> I don't fully understand why we need to take a raw pointer and explicitly call
> delete. It would be better if we can delegate the work to the normal OwnPtr
> mechanism.

In some cases the CallbackPromiseAdapter is used to pass a boolean, in which
case the memory exists on-stack by the caller (the embedder). The dispose()
method will be empty for those cases.

In this case, Blink takes ownership the data passed in to the callback, here
|notificationInfosRaw|. This is a more common paradigm for
CallbackPromiseAdapter.

[Peter Beverloo, 2015-03-20 14:35:28]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2015-03-20 14:35:29]

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

[commit-bot: I haz the power, 2015-03-20 14:35:37]

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

[commit-bot: I haz the power, 2015-03-20 16:48:18]

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

[commit-bot: I haz the power, 2015-03-20 16:48:19]

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

[Peter Beverloo, 2015-03-20 19:28:52]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2015-03-20 19:28:53]

The patchset sent to the CQ was uploaded after l-g-t-m from johnme@chromium.org,
haraken@chromium.org, mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1016843002/#ps40001
(title: "update the webexposed test")

[commit-bot: I haz the power, 2015-03-20 19:29:07]

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

[commit-bot: I haz the power, 2015-03-20 22:13:08]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192290