Implement a RenderFrame-less path for Web Notifications.

Implement a RenderFrame-less path for Web Notifications.

This implements the WebNotificationManager Blink API in //content,
together with a dispatcher to communicate with the browser process and
for the browser process to communicate with the appropriate Notification
Manager. This code path is not being used yet, but will be once the
Blink Web Notification API switches over.

BUG=392187
Committed: https://crrev.com/4099729cc49bf6a045c36677e98a38215979ab37
Cr-Commit-Position: refs/heads/master@{#301398}

[Peter Beverloo, 2014-10-23 18:44:51]

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

[Peter Beverloo, 2014-10-23 18:44:51]

PTAL. While the line-count is high, the functionality should be rather
straightforward and is mostly equal to what we already have. Let me know if you
want me to split it up further?

Full patches which I'm splitting up can be seen here:
  Blink: https://codereview.chromium.org/624033003/
  Chromium: https://codereview.chromium.org/626973002/

[Mike West, 2014-10-23 19:06:18]

On 2014/10/23 18:44:51, Peter Beverloo wrote:
> PTAL. While the line-count is high, the functionality should be rather
> straightforward and is mostly equal to what we already have. Let me know if
you
> want me to split it up further?
> 
> Full patches which I'm splitting up can be seen here:
>   Blink: https://codereview.chromium.org/624033003/
>   Chromium: https://codereview.chromium.org/626973002/

I'll take a look at this in the morning. Thanks for sending it over!

[Mike West, 2014-10-24 13:20:18]

LGTM % nits. This seems like a pretty reasonable way to migrate the existing
functionality off RenderFrame.

You'll need someone in content/{browser,child}'s OWNERS file to agree with me,
of course.

https://codereview.chromium.org/644643005/diff/40001/content/browser/notifica...
File content/browser/notification_message_filter.h (right):

https://codereview.chromium.org/644643005/diff/40001/content/browser/notifica...
content/browser/notification_message_filter.h:9: #include
"third_party/WebKit/public/platform/WebNotificationPermission.h"
I think you can forward-declare this.

https://codereview.chromium.org/644643005/diff/40001/content/child/blink_plat...
File content/child/blink_platform_impl.cc (right):

https://codereview.chromium.org/644643005/diff/40001/content/child/blink_plat...
content/child/blink_platform_impl.cc:1040: thread_safe_sender_.get(),
Do you need to store the ThreadSafeSender and NotificationDispatcher in the
constructor? Can't you grab them here and drop the object properties?

https://codereview.chromium.org/644643005/diff/40001/content/child/blink_plat...
File content/child/blink_platform_impl.h (right):

https://codereview.chromium.org/644643005/diff/40001/content/child/blink_plat...
content/child/blink_platform_impl.h:159: virtual blink::WebGeofencingProvider*
geofencingProvider() override;
Not-your-nit: We should drop these 'override' declarations for platform APIs
defined in Blink.

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
File content/child/notifications/notification_dispatcher.h (right):

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
content/child/notifications/notification_dispatcher.h:44: typedef std::map<int,
int> NotificationIdToThreadId;
Nit: `using NotificationIdToThreadId = ...`.

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
File content/child/notifications/notification_manager.cc (right):

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
content/child/notifications/notification_manager.cc:22: namespace {
Nit: Blank line.

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
content/child/notifications/notification_manager.cc:28: static
base::LazyInstance<base::ThreadLocalPointer<NotificationManager> >::Leaky
Nit: >>

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
content/child/notifications/notification_manager.cc:40:
g_notification_manager_tls.Pointer()->Set(NULL);
Nit: nullptr

[Peter Beverloo, 2014-10-24 13:57:34]

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

[Peter Beverloo, 2014-10-24 13:57:35]

Thanks for the quick review! All addressed.

jam: PTAL

https://codereview.chromium.org/644643005/diff/40001/content/browser/notifica...
File content/browser/notification_message_filter.h (right):

https://codereview.chromium.org/644643005/diff/40001/content/browser/notifica...
content/browser/notification_message_filter.h:9: #include
"third_party/WebKit/public/platform/WebNotificationPermission.h"
On 2014/10/24 13:20:18, Mike West wrote:
> I think you can forward-declare this.

Nope, it's an enum and we can't forward declare (non-class) enumerations.

https://codereview.chromium.org/644643005/diff/40001/content/child/blink_plat...
File content/child/blink_platform_impl.cc (right):

https://codereview.chromium.org/644643005/diff/40001/content/child/blink_plat...
content/child/blink_platform_impl.cc:1040: thread_safe_sender_.get(),
On 2014/10/24 13:20:18, Mike West wrote:
> Do you need to store the ThreadSafeSender and NotificationDispatcher in the
> constructor? Can't you grab them here and drop the object properties?

ChildThread::current() is stored in a TLS and is not available here when called
from a worker. This is the common practice, and also how it's implemented in
renderer_blink_platform_impl.

https://codereview.chromium.org/644643005/diff/40001/content/child/blink_plat...
File content/child/blink_platform_impl.h (right):

https://codereview.chromium.org/644643005/diff/40001/content/child/blink_plat...
content/child/blink_platform_impl.h:159: virtual blink::WebGeofencingProvider*
geofencingProvider() override;
On 2014/10/24 13:20:18, Mike West wrote:
> Not-your-nit: We should drop these 'override' declarations for platform APIs
> defined in Blink.

https://codereview.chromium.org/679523004/

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
File content/child/notifications/notification_dispatcher.h (right):

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
content/child/notifications/notification_dispatcher.h:44: typedef std::map<int,
int> NotificationIdToThreadId;
On 2014/10/24 13:20:18, Mike West wrote:
> Nit: `using NotificationIdToThreadId = ...`.

Done.

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
File content/child/notifications/notification_manager.cc (right):

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
content/child/notifications/notification_manager.cc:22: namespace {
On 2014/10/24 13:20:18, Mike West wrote:
> Nit: Blank line.

Done.

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
content/child/notifications/notification_manager.cc:28: static
base::LazyInstance<base::ThreadLocalPointer<NotificationManager> >::Leaky
On 2014/10/24 13:20:18, Mike West wrote:
> Nit: >>

Done.

https://codereview.chromium.org/644643005/diff/40001/content/child/notificati...
content/child/notifications/notification_manager.cc:40:
g_notification_manager_tls.Pointer()->Set(NULL);
On 2014/10/24 13:20:18, Mike West wrote:
> Nit: nullptr

Done.

[jam, 2014-10-24 23:44:57]

lgtm, add yourself to the new directory

[Peter Beverloo, 2014-10-27 11:40:44]

On 2014/10/24 23:44:57, jam wrote:
> lgtm, add yourself to the new directory

Done. Thanks!

[Peter Beverloo, 2014-10-27 11:41:28]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2014-10-27 11:42:08]

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

[commit-bot: I haz the power, 2014-10-27 17:42:42]

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

[commit-bot: I haz the power, 2014-10-27 17:42:43]

Exceeded time limit waiting for builds to trigger.

[Peter Beverloo, 2014-10-27 17:43:34]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2014-10-27 17:44:50]

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

[commit-bot: I haz the power, 2014-10-27 18:02:04]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2014-10-27 18:02:56]

Patchset 6 (id:??) landed as
https://crrev.com/4099729cc49bf6a045c36677e98a38215979ab37
Cr-Commit-Position: refs/heads/master@{#301398}