[BackgroundSync] Add new registration options from the spec

[BackgroundSync] Add new registration options from the spec

This CL brings the BackgroundSyncRegistration options up-to-date with the spec: https://github.com/slightlyoff/BackgroundSync/pull/74

TODO in followup CLs:
* Rename 'name' to 'tag': http://crrev.com/1068863003.
* Give periodic and one-shot tags their own namespaces in the map so that they don't conflict.

BUG=474573
Committed: https://crrev.com/36b903805f31c455f0831eb70113d7f25396b79f
Cr-Commit-Position: refs/heads/master@{#324077}

[jkarlin, 2015-04-07 13:49:07]

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

[jkarlin, 2015-04-07 13:49:54]

johnme@: Thanks!

[johnme, 2015-04-07 17:05:12]

lgtm with nit, though I'm not sure what you mean by the pseudo-spec. It would be
nice to keep the explainer[1] or spec[2] up to date with this stuff (and ideally
link to the spec/explainer pull requests from the corresponding chromium commit
messages). For example I don't see fire_once mentioned there.

[1]: https://github.com/slightlyoff/BackgroundSync/blob/master/explainer.md
[2]: https://slightlyoff.github.io/BackgroundSync/spec/

https://codereview.chromium.org/1068863002/diff/1/content/browser/background_...
File content/browser/background_sync/background_sync_manager.h (right):

https://codereview.chromium.org/1068863002/diff/1/content/browser/background_...
content/browser/background_sync/background_sync_manager.h:56: :
id(kInvalidRegistrationId),
Nit: easier to read if you remove this constructor, and just use C++11
Non-Static Class Member Initializers*, e.g.:

    bool fire_once = true;

*: (allowed by https://chromium-cpp.appspot.com)

[jkarlin, 2015-04-07 17:55:07]

Thanks!

https://codereview.chromium.org/1068863002/diff/1/content/browser/background_...
File content/browser/background_sync/background_sync_manager.h (right):

https://codereview.chromium.org/1068863002/diff/1/content/browser/background_...
content/browser/background_sync/background_sync_manager.h:56: :
id(kInvalidRegistrationId),
On 2015/04/07 17:05:12, johnme wrote:
> Nit: easier to read if you remove this constructor, and just use C++11
> Non-Static Class Member Initializers*, e.g.:
> 
>     bool fire_once = true;
> 
> *: (allowed by https://chromium-cpp.appspot.com)

Done.

[jkarlin, 2015-04-07 17:56:57]

On 2015/04/07 17:05:12, johnme wrote:
> lgtm with nit, though I'm not sure what you mean by the pseudo-spec. It would
be
> nice to keep the explainer[1] or spec[2] up to date with this stuff (and
ideally
> link to the spec/explainer pull requests from the corresponding chromium
commit
> messages). 

Added the pull request link to the description.


> For example I don't see fire_once mentioned there.

Ah, right. That one is implementation specific.

[jkarlin, 2015-04-07 18:04:10]

The CQ bit was checked by jkarlin@chromium.org

[jkarlin, 2015-04-07 18:04:10]

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

[commit-bot: I haz the power, 2015-04-07 18:04:43]

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

[commit-bot: I haz the power, 2015-04-07 18:41:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-07 18:42:40]

Patchset 2 (id:??) landed as
https://crrev.com/36b903805f31c455f0831eb70113d7f25396b79f
Cr-Commit-Position: refs/heads/master@{#324077}