Push API: Implement and ship PushSubscription.options

Push API: Implement and ship PushSubscription.options

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ysEXU21Dn1Y/discussion

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
Committed: https://crrev.com/8882a829851f9bfe6e1857c9d8b0a7c679d89c00
Cr-Commit-Position: refs/heads/master@{#405752}

[johnme, 2016-07-08 10:35:47]

Description was changed from

==========
Push API: Expose PushSubscriptionOptions on PushSubscription

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

to

==========
Push API: Expose PushSubscriptionOptions on PushSubscription

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

[johnme, 2016-07-08 10:36:00]

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

[johnme, 2016-07-08 10:36:35]

Description was changed from

==========
Push API: Expose PushSubscriptionOptions on PushSubscription

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

to

==========
Push API: Implement PushSubscription.options

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

[johnme, 2016-07-08 13:28:07]

Description was changed from

==========
Push API: Implement PushSubscription.options

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

to

==========
Push API: Implement PushSubscription.options

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ysEXU21Dn1Y/discus...

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

[johnme, 2016-07-08 13:28:33]

Description was changed from

==========
Push API: Implement PushSubscription.options

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ysEXU21Dn1Y/discus...

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

to

==========
Push API: Implement and ship PushSubscription.options

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ysEXU21Dn1Y/discus...

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

[Peter Beverloo, 2016-07-08 18:30:13]

lgtm % comments

https://chromiumcodereview.appspot.com/2133673002/diff/1/content/browser/push...
File content/browser/push_messaging/push_messaging_message_filter.cc (right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/content/browser/push...
content/browser/push_messaging/push_messaging_message_filter.cc:759: const GURL
origin = registration->pattern().GetOrigin();
nit: move to :770?

https://chromiumcodereview.appspot.com/2133673002/diff/1/content/browser/push...
content/browser/push_messaging/push_messaging_message_filter.cc:833: // We
reject subscription requests with userVisibleOnly false, so it must have
nit: don't use "we" in comments. Probably want to avoid "Chrome" too.

https://chromiumcodereview.appspot.com/2133673002/diff/1/content/child/push_m...
File content/child/push_messaging/push_provider.cc (right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/content/child/push_m...
content/child/push_messaging/push_provider.cc:234:
blink::WebString::fromLatin1(options.sender_info), p256dh, auth)));
When moving to Mojo, we'd probably want Blink to think about this as a uint8_t
vector (as that's the natural type there), and Chrome to think about this as an
std::string (as both SW user data and the GCM Driver require that)?

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/L...
File
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html
(right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/L...
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html:20:
assert_true(actual_subscription instanceof window.PushSubscription,
nit: why do you prefix these types with `window`? That's the global, so there's
no need to do so.

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/L...
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html:48:
description + ": applicationServerKey instanceof window.ArrayBuffer");
nit: alignment

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/L...
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html:59:
// for bad (https://github.com/w3c/push-api/issues/198).
no need to imply judgement calls in our code :)

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/L...
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html:74:
var VAPID_OPTIONS = {
const as you use on :66? Same on :78 and :83

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/L...
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html:146:
</html>
nit: -2 indent

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/L...
File
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
(right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/L...
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt:4558:
method constructor
What does the constructor do? Throw?

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
File third_party/WebKit/Source/modules/push_messaging/PushManager.cpp (right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
third_party/WebKit/Source/modules/push_messaging/PushManager.cpp:96: new
PushPermissionStatusCallbacks(resolver));
micro nit: not a fan of the wrapping here, since we can't align the lines :(.

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
File third_party/WebKit/Source/modules/push_messaging/PushSubscription.h
(right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
third_party/WebKit/Source/modules/push_messaging/PushSubscription.h:36:
PushSubscriptionOptions* options() const { return m_options; }
nit: looking throughout the code, it's *much* more common to return
m_options.get() than it is to rely on the conversion operator.

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
File third_party/WebKit/Source/modules/push_messaging/PushSubscription.idl
(right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
third_party/WebKit/Source/modules/push_messaging/PushSubscription.idl:22:
serializer;
Should the `options` member influence the serializer?

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
File
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.cpp
(right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.cpp:19:
const int kMaxApplicationServerKeyLength = 255;
We don't indent code in namespaces.

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
File third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.h
(right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.h:23:
static WebPushSubscriptionOptions toWeb(const PushSubscriptionOptionsInit&,
ExceptionState&);
++docs (notably because it takes an ExceptionState)

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.h:36:
PushSubscriptionOptions(const WebPushSubscriptionOptions&);
explicit

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
File
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.idl
(right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.idl:12:
// Mutable by web developer. See https://github.com/w3c/push-api/issues/198.
Please move this to the source code, IDL is not the right place for these docs.
(Ideally it would be copy/paste from the spec.)

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
File
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptionsInit.idl
(right):

https://chromiumcodereview.appspot.com/2133673002/diff/1/third_party/WebKit/S...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptionsInit.idl:1:
// Copyright 2015 The Chromium Authors. All rights reserved.
2016

[johnme, 2016-07-13 17:51:58]

Addressed nits - thanks!

https://codereview.chromium.org/2133673002/diff/1/content/browser/push_messag...
File content/browser/push_messaging/push_messaging_message_filter.cc (right):

https://codereview.chromium.org/2133673002/diff/1/content/browser/push_messag...
content/browser/push_messaging/push_messaging_message_filter.cc:759: const GURL
origin = registration->pattern().GetOrigin();
On 2016/07/08 18:30:11, Peter Beverloo wrote:
> nit: move to :770?

Done.

https://codereview.chromium.org/2133673002/diff/1/content/browser/push_messag...
content/browser/push_messaging/push_messaging_message_filter.cc:833: // We
reject subscription requests with userVisibleOnly false, so it must have
On 2016/07/08 18:30:11, Peter Beverloo wrote:
> nit: don't use "we" in comments. Probably want to avoid "Chrome" too.

Ended up putting Chrome, since I'm relying on an implementation detail of
Chromium, and
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md
says to write Chrome not Chromium in comments.

https://codereview.chromium.org/2133673002/diff/1/content/child/push_messagin...
File content/child/push_messaging/push_provider.cc (right):

https://codereview.chromium.org/2133673002/diff/1/content/child/push_messagin...
content/child/push_messaging/push_provider.cc:234:
blink::WebString::fromLatin1(options.sender_info), p256dh, auth)));
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> When moving to Mojo, we'd probably want Blink to think about this as a uint8_t
> vector (as that's the natural type there), and Chrome to think about this as
an
> std::string (as both SW user data and the GCM Driver require that)?

Acknowledged. Added a TODO in WebPushSubscriptionOptions.h

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html
(right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html:20:
assert_true(actual_subscription instanceof window.PushSubscription,
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> nit: why do you prefix these types with `window`? That's the global, so
there's
> no need to do so.

Done.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html:48:
description + ": applicationServerKey instanceof window.ArrayBuffer");
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> nit: alignment

Done.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html:59:
// for bad (https://github.com/w3c/push-api/issues/198).
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> no need to imply judgement calls in our code :)

Done.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html:74:
var VAPID_OPTIONS = {
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> const as you use on :66? Same on :78 and :83

Done.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html:146:
</html>
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> nit: -2 indent

Done.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
(right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt:4558:
method constructor
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> What does the constructor do? Throw?

Uncaught TypeError: Illegal constructor

Same as if you try to construct a PushSubscription, or indeed many of the things
in this list.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/push_messaging/PushManager.cpp (right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/push_messaging/PushManager.cpp:96: new
PushPermissionStatusCallbacks(resolver));
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> micro nit: not a fan of the wrapping here, since we can't align the lines :(.

Done.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/push_messaging/PushSubscription.h
(right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/push_messaging/PushSubscription.h:36:
PushSubscriptionOptions* options() const { return m_options; }
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> nit: looking throughout the code, it's *much* more common to return
> m_options.get() than it is to rely on the conversion operator.

Done.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/push_messaging/PushSubscription.idl
(right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/push_messaging/PushSubscription.idl:22:
serializer;
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> Should the `options` member influence the serializer?

The spec explicitly says "the options to a PushSubscription are not
serialized"[1], and I find no strong reason to object (most servers will always
pass the same options, so it's redundant). Perhaps PushSubscriptionOptions
should have its own separate serializer though?

[1]: https://w3c.github.io/push-api/#pushsubscription-interface

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.cpp
(right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.cpp:19:
const int kMaxApplicationServerKeyLength = 255;
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> We don't indent code in namespaces.

We *do* indent code in nested namespaces:
https://www.chromium.org/blink/coding-style#TOC-Indentation

`git cl format` actually enforces this too.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.h
(right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.h:23:
static WebPushSubscriptionOptions toWeb(const PushSubscriptionOptionsInit&,
ExceptionState&);
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> ++docs (notably because it takes an ExceptionState)

Done.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.h:36:
PushSubscriptionOptions(const WebPushSubscriptionOptions&);
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> explicit

Done (good catch)

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.idl
(right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.idl:12:
// Mutable by web developer. See https://github.com/w3c/push-api/issues/198.
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> Please move this to the source code, IDL is not the right place for these
docs.
> (Ideally it would be copy/paste from the spec.)

Done.

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptionsInit.idl
(right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptionsInit.idl:1:
// Copyright 2015 The Chromium Authors. All rights reserved.
On 2016/07/08 18:30:12, Peter Beverloo wrote:
> 2016

Done.

[Peter Beverloo, 2016-07-14 14:40:02]

still lgtm

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.cpp
(right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.cpp:19:
const int kMaxApplicationServerKeyLength = 255;
On 2016/07/13 17:51:57, johnme wrote:
> On 2016/07/08 18:30:12, Peter Beverloo wrote:
> > We don't indent code in namespaces.
> 
> We *do* indent code in nested namespaces:
> https://www.chromium.org/blink/coding-style#TOC-Indentation
> 
> `git cl format` actually enforces this too.

That does not seem to apply to anonymous namespaces (3 out of the first 400
results do this):

https://cs.chromium.org/search/?q=f:third_party/webkit+pcre:yes+namespace%5Cs...

This would also be inconsistent with the other three uses in our module. Please
override `git cl format` and remove the indentation?

[johnme, 2016-07-14 17:01:22]

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.cpp
(right):

https://codereview.chromium.org/2133673002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.cpp:19:
const int kMaxApplicationServerKeyLength = 255;
On 2016/07/14 14:40:01, Peter Beverloo wrote:
> On 2016/07/13 17:51:57, johnme wrote:
> > On 2016/07/08 18:30:12, Peter Beverloo wrote:
> > > We don't indent code in namespaces.
> > 
> > We *do* indent code in nested namespaces:
> > https://www.chromium.org/blink/coding-style#TOC-Indentation
> > 
> > `git cl format` actually enforces this too.
> 
> That does not seem to apply to anonymous namespaces (3 out of the first 400
> results do this):
> 
>
https://cs.chromium.org/search/?q=f:third_party/webkit+pcre:yes+namespace%5Cs...
> 
> This would also be inconsistent with the other three uses in our module.
Please
> override `git cl format` and remove the indentation?

Done.

[johnme, 2016-07-14 18:00:03]

The CQ bit was checked by johnme@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-14 18:00:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[johnme, 2016-07-14 18:05:01]

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

[johnme, 2016-07-14 18:05:02]

mkwst: please review content/common/push_messaging_messages.h

rbyers: please review webexposed/ test changes as per intent to ship

[commit-bot: I haz the power, 2016-07-14 19:11:16]

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

[commit-bot: I haz the power, 2016-07-14 19:11:17]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Rick Byers, 2016-07-14 19:33:10]

webexposed LGTM

[Mike West, 2016-07-15 09:00:30]

IPC LGTM (will y'all be mojoifying this at some point?)

[johnme, 2016-07-15 11:12:14]

On 2016/07/15 09:00:30, Mike West wrote:
> IPC LGTM (will y'all be mojoifying this at some point?)

That's in the pipeline, though it's been deprioritized somewhat after seeing how
many rough edges Peter ran into mojoifying Notifications :-|

[johnme, 2016-07-15 11:12:17]

The CQ bit was checked by johnme@chromium.org

[johnme, 2016-07-15 11:12:17]

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/2133673002/#ps60001
(title: "Add MODULES_EXPORT")

[johnme, 2016-07-15 11:12:23]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2016-07-15 11:12:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[johnme, 2016-07-15 12:59:11]

The CQ bit was unchecked by johnme@chromium.org

[johnme, 2016-07-15 12:59:12]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2016-07-15 12:59:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 13:14:00]

Description was changed from

==========
Push API: Implement and ship PushSubscription.options

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ysEXU21Dn1Y/discus...

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

to

==========
Push API: Implement and ship PushSubscription.options

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ysEXU21Dn1Y/discus...

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

[commit-bot: I haz the power, 2016-07-15 13:14:01]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-15 13:14:06]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-15 13:15:22]

Description was changed from

==========
Push API: Implement and ship PushSubscription.options

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ysEXU21Dn1Y/discus...

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627
==========

to

==========
Push API: Implement and ship PushSubscription.options

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ysEXU21Dn1Y/discus...

Recent updates to the Push API renamed the PushSubscriptionOptions
dictionary to PushSubscriptionOptionsInit, and instead introduced a
PushSubscriptionOptions interface which should be exposed as an attribute
on PushSubscription objects, reflecting the options that were passed in
when subscribing.

See commits:
https://github.com/w3c/push-api/commit/3f7aaed45680818d6c472e6b6bce8854e0c9bff2
https://github.com/w3c/push-api/commit/5c25fee2dcaecb140d78566ac26207ee602a6844
https://github.com/w3c/push-api/commit/428f4c5c8993fc6af82f1ea9194e1bb1cbec2feb

This patch implements those changes.

BUG=626627

Committed: https://crrev.com/8882a829851f9bfe6e1857c9d8b0a7c679d89c00
Cr-Commit-Position: refs/heads/master@{#405752}
==========

[commit-bot: I haz the power, 2016-07-15 13:15:24]

Patchset 4 (id:??) landed as
https://crrev.com/8882a829851f9bfe6e1857c9d8b0a7c679d89c00
Cr-Commit-Position: refs/heads/master@{#405752}