[background-sync] Merge SyncManager and BackgroundSyncProvider

[background-sync] Merge SyncManager and BackgroundSyncProvider

- Follow up to http://crrev.com/2481393002
- Move BackgroundSyncProvider code into SyncManger
- A seperate mojo IPC connection with the browser's BackgroundSyncService is now
  created per SyncManager object as opposed to per thread
- Remove BackgroundSyncProvider.(h|cpp)

BUG=662134
Committed: https://crrev.com/34efa35374cb3073a3cfb364e4a2cf9aa3273f0a
Cr-Commit-Position: refs/heads/master@{#433667}

[adithyas, 2016-11-21 17:40:55]

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

[commit-bot: I haz the power, 2016-11-21 17:41:14]

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, 2016-11-21 17:43:50]

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

[commit-bot: I haz the power, 2016-11-21 17:43:51]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[jbroman, 2016-11-21 17:48:14]

Description was changed from

==========
[background-sync] Merge SyncManager and BackgroundSyncProvider

- Follow up to http://crrev.com/2481393002
- Move BackgroundSyncProvider code into SyncManger
- A seperate mojo IPC connection with the browser's BackgroundSyncService is now
  created per SyncManger object as opposed to per thread
- Remove BackgroundSyncProvider.(h|cpp)

BUG=662134
==========

to

==========
[background-sync] Merge SyncManager and BackgroundSyncProvider

- Follow up to http://crrev.com/2481393002
- Move BackgroundSyncProvider code into SyncManger
- A seperate mojo IPC connection with the browser's BackgroundSyncService is now
  created per SyncManager object as opposed to per thread
- Remove BackgroundSyncProvider.(h|cpp)

BUG=662134
==========

[adithyas, 2016-11-21 18:16:56]

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

[commit-bot: I haz the power, 2016-11-21 18:17:38]

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

[adithyas, 2016-11-21 19:09:26]

adithyas@chromium.org changed reviewers:
+ iclelland@chromium.org, jbroman@chromium.org

[jbroman, 2016-11-21 19:25:51]

lgtm

Yay, another -93 LOC. Only two of the most trivial nits.

https://codereview.chromium.org/2515353002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/background_sync/SyncManager.cpp (right):

https://codereview.chromium.org/2515353002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/background_sync/SyncManager.cpp:24: :
m_registration(registration), m_backgroundSyncService(nullptr) {
nit: mojo::InterfacePtr (like BackgroundSyncServicePtr) is initialized to null
by default; it doesn't need to be explicitly initialized.

https://codereview.chromium.org/2515353002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/background_sync/SyncManager.cpp:142:
mojo::InterfaceRequest<mojom::blink::BackgroundSyncService> request =
super-nit: we usually just call mojo::GetProxy directly in the getInterface
call, but it's up to you.

Platform::current()->interfaceProvider()->getInterface(
    mojo::GetProxy(&m_backgroundSyncService));

[iclelland, 2016-11-21 20:17:15]

lgtm

https://codereview.chromium.org/2515353002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/background_sync/SyncManager.cpp (right):

https://codereview.chromium.org/2515353002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/background_sync/SyncManager.cpp:58: void
SyncManager::registerCallback(ScriptPromiseResolver* resolver,
nit: Move this below getBackgroundSyncServicePtr to match the declaration order
in the header file.

https://codereview.chromium.org/2515353002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/background_sync/SyncManager.cpp:107: void
SyncManager::getRegistrationsCallback(
Ditto

[adithyas, 2016-11-21 20:27:54]

Fixed jbroman@ and iclelland@ nits

https://codereview.chromium.org/2515353002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/background_sync/SyncManager.cpp (right):

https://codereview.chromium.org/2515353002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/background_sync/SyncManager.cpp:24: :
m_registration(registration), m_backgroundSyncService(nullptr) {
On 2016/11/21 at 19:25:51, jbroman wrote:
> nit: mojo::InterfacePtr (like BackgroundSyncServicePtr) is initialized to null
by default; it doesn't need to be explicitly initialized.

Fixed!

https://codereview.chromium.org/2515353002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/background_sync/SyncManager.cpp:58: void
SyncManager::registerCallback(ScriptPromiseResolver* resolver,
On 2016/11/21 at 20:17:15, iclelland wrote:
> nit: Move this below getBackgroundSyncServicePtr to match the declaration
order in the header file.

Moved this and getRegistrationsCallback below getBackgroundSyncServicePtr.

https://codereview.chromium.org/2515353002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/background_sync/SyncManager.cpp:142:
mojo::InterfaceRequest<mojom::blink::BackgroundSyncService> request =
On 2016/11/21 at 19:25:51, jbroman wrote:
> super-nit: we usually just call mojo::GetProxy directly in the getInterface
call, but it's up to you.
> 
> Platform::current()->interfaceProvider()->getInterface(
>     mojo::GetProxy(&m_backgroundSyncService));

Done.

[adithyas, 2016-11-21 20:28:19]

The CQ bit was checked by adithyas@chromium.org

[adithyas, 2016-11-21 20:28:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbroman@chromium.org, iclelland@chromium.org
Link to the patchset: https://codereview.chromium.org/2515353002/#ps60001
(title: "Reorder methods")

[commit-bot: I haz the power, 2016-11-21 20:29:33]

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-11-21 22:07:19]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1479760099503690,
"parent_rev": "7708c4995f792c531c262bb4de0766aac26ce036", "commit_rev":
"4cff60794460795c5722b859c610e2058016919c"}

[commit-bot: I haz the power, 2016-11-21 22:07:46]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-21 22:11:22]

Description was changed from

==========
[background-sync] Merge SyncManager and BackgroundSyncProvider

- Follow up to http://crrev.com/2481393002
- Move BackgroundSyncProvider code into SyncManger
- A seperate mojo IPC connection with the browser's BackgroundSyncService is now
  created per SyncManager object as opposed to per thread
- Remove BackgroundSyncProvider.(h|cpp)

BUG=662134
==========

to

==========
[background-sync] Merge SyncManager and BackgroundSyncProvider

- Follow up to http://crrev.com/2481393002
- Move BackgroundSyncProvider code into SyncManger
- A seperate mojo IPC connection with the browser's BackgroundSyncService is now
  created per SyncManager object as opposed to per thread
- Remove BackgroundSyncProvider.(h|cpp)

BUG=662134

Committed: https://crrev.com/34efa35374cb3073a3cfb364e4a2cf9aa3273f0a
Cr-Commit-Position: refs/heads/master@{#433667}
==========

[commit-bot: I haz the power, 2016-11-21 22:11:23]

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