Implement some follow-ups after mojofiction of Push-messaging.

Implement some follow-ups after mojofiction of Push-messaging.

- In push_messaging_manager.cc, store |callback| in |data| (RegisterData)
  instead of passing it everywhere needs it.
- Naming the functions like DidFoo as opposed to FooCallback in
  Push_Provider{.h|.cc}. which conveys information about the fact that something
  happened.
- In PushProvider::SubscribeCallback(), to replace "if (!callbacks) return;" by
  DCHECK(callbacks).

BUG=693366
Review-Url: https://codereview.chromium.org/2712693002
Cr-Commit-Position: refs/heads/master@{#452765}
Committed: https://chromium.googlesource.com/chromium/src/+/f667030fb4c6bb84a55332a5335d451e541d2c02

[ke.he, 2017-02-22 13:09:28]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 13:09:43]

Dry run: 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, 2017-02-22 13:57:41]

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

[commit-bot: I haz the power, 2017-02-22 13:57:41]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-02-22 15:11:43]

ke.he@intel.com changed reviewers:
+ peter@chromium.org

[ke.he, 2017-02-22 15:11:44]

Hi, Peter, PTAL, Thanks:)

[Peter Beverloo, 2017-02-23 13:37:40]

lgtm, thanks!

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

https://codereview.chromium.org/2712693002/diff/1/content/browser/push_messag...
content/browser/push_messaging/push_messaging_manager.cc:118: SubscribeCallback
callback;
This will be copied, but since the callback's bind state is ref counted that's
fine.

https://codereview.chromium.org/2712693002/diff/1/content/browser/push_messag...
content/browser/push_messaging/push_messaging_manager.cc:513:
DCHECK(data.callback);
nit: I wouldn't add this DCHECK, it's really uncommon to DCHECK that a callback
is non-null. I think we can rely on it? (The Run() method will also check this.)

[ke.he, 2017-02-24 03:24:30]

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

https://codereview.chromium.org/2712693002/diff/1/content/browser/push_messag...
content/browser/push_messaging/push_messaging_manager.cc:118: SubscribeCallback
callback;
On 2017/02/23 13:37:39, Peter Beverloo wrote:
> This will be copied, but since the callback's bind state is ref counted that's
> fine.

Thanks!

https://codereview.chromium.org/2712693002/diff/1/content/browser/push_messag...
content/browser/push_messaging/push_messaging_manager.cc:513:
DCHECK(data.callback);
On 2017/02/23 13:37:39, Peter Beverloo wrote:
> nit: I wouldn't add this DCHECK, it's really uncommon to DCHECK that a
callback
> is non-null. I think we can rely on it? (The Run() method will also check
this.)

Done. (and elsewhere)

[ke.he, 2017-02-24 03:24:38]

The CQ bit was checked by ke.he@intel.com

[ke.he, 2017-02-24 03:24:38]

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/2712693002/#ps20001
(title: "Implement some follow-ups after mojofiction of Push-messaging.")

[commit-bot: I haz the power, 2017-02-24 03:25:10]

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, 2017-02-24 03:27:42]

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

[commit-bot: I haz the power, 2017-02-24 03:27:44]

Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[ke.he, 2017-02-24 03:45:17]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 03:45:33]

Dry run: 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, 2017-02-24 04:34:30]

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

[commit-bot: I haz the power, 2017-02-24 04:34:31]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-02-24 05:35:07]

The CQ bit was checked by ke.he@intel.com

[ke.he, 2017-02-24 05:35:08]

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/2712693002/#ps40001
(title: "code rebase")

[commit-bot: I haz the power, 2017-02-24 05:35:21]

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, 2017-02-24 05:40:44]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487914507596260,
"parent_rev": "350936ff217789a657ef44417c374c6a0423f06f", "commit_rev":
"f667030fb4c6bb84a55332a5335d451e541d2c02"}

[commit-bot: I haz the power, 2017-02-24 05:41:26]

Description was changed from

==========
Implement some follow-ups after mojofiction of Push-messaging.

- In push_messaging_manager.cc, store |callback| in |data| (RegisterData)
  instead of passing it everywhere needs it.
- Naming the functions like DidFoo as opposed to FooCallback in
  Push_Provider{.h|.cc}. which conveys information about the fact that something
  happened.
- In PushProvider::SubscribeCallback(), to replace "if (!callbacks) return;" by
  DCHECK(callbacks).

BUG=693366
==========

to

==========
Implement some follow-ups after mojofiction of Push-messaging.

- In push_messaging_manager.cc, store |callback| in |data| (RegisterData)
  instead of passing it everywhere needs it.
- Naming the functions like DidFoo as opposed to FooCallback in
  Push_Provider{.h|.cc}. which conveys information about the fact that something
  happened.
- In PushProvider::SubscribeCallback(), to replace "if (!callbacks) return;" by
  DCHECK(callbacks).

BUG=693366

Review-Url: https://codereview.chromium.org/2712693002
Cr-Commit-Position: refs/heads/master@{#452765}
Committed:
https://chromium.googlesource.com/chromium/src/+/f667030fb4c6bb84a55332a5335d...
==========

[commit-bot: I haz the power, 2017-02-24 05:41:27]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/f667030fb4c6bb84a55332a5335d...