[Sync] Remove some http-related chrome deps from SyncBackendHostImpl.

[Sync] Remove some http-related chrome deps from SyncBackendHostImpl.

Previously on ::Initialize, SBHI would get passed a NetworkResources
object, and use that combined with profile_->GetRequestContext(),
UpdateNetworkTime, and the Core's CancelationSignal to create an
HttpPostProviderFactory.

Uses of Profile need to go away, and UpdateNetworkTime depends on
chrome/browser/browser_process.h, so this change introduces the
HttpPostProviderFactoryGetter callback typedef to SBH, which just takes
a CancelationSignal and returns the HttpPostProviderFactory.

The HttpPostProviderFactoryGetter replaces NetworkResources in the
Initialize argument list.

BUG=512056
TBR=sky

Committed: https://crrev.com/d2ea9249e1b8b76a73a6af3d50db9b523926f671
Cr-Commit-Position: refs/heads/master@{#356725}

[maxbogue, 2015-10-27 16:49:31]

maxbogue@chromium.org changed reviewers:
+ zea@chromium.org

[maxbogue, 2015-10-27 16:49:32]

Hey Nicolas, here's the partially-applied-function change I mentioned yesterday.
PTAL!

[Nicolas Zea, 2015-10-27 20:41:42]

https://codereview.chromium.org/1410013008/diff/20001/chrome/browser/sync/glu...
File chrome/browser/sync/glue/sync_backend_host.h (right):

https://codereview.chromium.org/1410013008/diff/20001/chrome/browser/sync/glu...
chrome/browser/sync/glue/sync_backend_host.h:78: const
HttpPostProviderFactoryGetter& get_http_post_provider_factory,
nit: prefer calling this http_post_provider_factory_getter (we typically have
the name describe what it is, not be a verb about what it would do).

https://codereview.chromium.org/1410013008/diff/20001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_service.cc (right):

https://codereview.chromium.org/1410013008/diff/20001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_service.cc:194: void
UpdateNetworkTimeOnUIThread(base::Time network_time,
Were you planning to try to move this to the PSSFactory?

[maxbogue, 2015-10-27 23:53:08]

Moved the time update function to PSSFactory, PTAL!

https://codereview.chromium.org/1410013008/diff/20001/chrome/browser/sync/glu...
File chrome/browser/sync/glue/sync_backend_host.h (right):

https://codereview.chromium.org/1410013008/diff/20001/chrome/browser/sync/glu...
chrome/browser/sync/glue/sync_backend_host.h:78: const
HttpPostProviderFactoryGetter& get_http_post_provider_factory,
On 2015/10/27 20:41:42, Nicolas Zea wrote:
> nit: prefer calling this http_post_provider_factory_getter (we typically have
> the name describe what it is, not be a verb about what it would do).

Ah, my habit when naming function vars is verbs, but that probably makes more
sense in other languages. Done.

https://codereview.chromium.org/1410013008/diff/20001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_service.cc (right):

https://codereview.chromium.org/1410013008/diff/20001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_service.cc:194: void
UpdateNetworkTimeOnUIThread(base::Time network_time,
On 2015/10/27 20:41:42, Nicolas Zea wrote:
> Were you planning to try to move this to the PSSFactory?

Done.

[Nicolas Zea, 2015-10-28 20:33:18]

LGTM!

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:1044: "sync/profile_sync_test_util.h",
Does this imply we weren't actually using any of these methods in tests
previously? If so, maybe file a bug to clean them up?

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_service.h (right):

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_service.h:238:
syncer::NetworkTimeUpdateCallback network_time_update_callback);
nit: pass by const ref

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_test_util.cc (right):

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_test_util.cc:11: #include "base/time/time.h"
nit: is this needed? (looks like you can leave time forward declared)

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_test_util.h (right):

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_test_util.h:24: void
EmptyNetworkTimeUpdate(const base::Time&,
nit: Add a comment about what this is for (and the fact that it's a definition
of NetworkTimeCallback)

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_test_util.h:24: void
EmptyNetworkTimeUpdate(const base::Time&,
forward declare Time and TimeDelta?

[maxbogue, 2015-10-28 23:19:11]

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:1044: "sync/profile_sync_test_util.h",
On 2015/10/28 20:33:18, Nicolas Zea wrote:
> Does this imply we weren't actually using any of these methods in tests
> previously? If so, maybe file a bug to clean them up?

Ah no, it's just that in chrome/chrome_tests_unit.gypi this was moved from a
sources list variable that was shared with GN to a concrete target's sources.

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_service.h (right):

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_service.h:238:
syncer::NetworkTimeUpdateCallback network_time_update_callback);
On 2015/10/28 20:33:18, Nicolas Zea wrote:
> nit: pass by const ref

Done.

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_test_util.cc (right):

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_test_util.cc:11: #include "base/time/time.h"
On 2015/10/28 20:33:18, Nicolas Zea wrote:
> nit: is this needed? (looks like you can leave time forward declared)

Done.

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_test_util.h (right):

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_test_util.h:24: void
EmptyNetworkTimeUpdate(const base::Time&,
On 2015/10/28 20:33:18, Nicolas Zea wrote:
> forward declare Time and TimeDelta?

Done.

https://codereview.chromium.org/1410013008/diff/60001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_test_util.h:24: void
EmptyNetworkTimeUpdate(const base::Time&,
On 2015/10/28 20:33:18, Nicolas Zea wrote:
> nit: Add a comment about what this is for (and the fact that it's a definition
> of NetworkTimeCallback)

Done.

[maxbogue, 2015-10-28 23:19:55]

The CQ bit was checked by maxbogue@chromium.org

[maxbogue, 2015-10-28 23:19:56]

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

[commit-bot: I haz the power, 2015-10-28 23:20:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410013008/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410013008/80001

[commit-bot: I haz the power, 2015-10-28 23:31:22]

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

[commit-bot: I haz the power, 2015-10-28 23:31:23]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[maxbogue, 2015-10-28 23:43:16]

Description was changed from

==========
[Sync] Remove some http-related chrome deps from SyncBackendHostImpl.

Previously on ::Initialize, SBHI would get passed a NetworkResources
object, and use that combined with profile_->GetRequestContext(),
UpdateNetworkTime, and the Core's CancelationSignal to create an
HttpPostProviderFactory.

Uses of Profile need to go away, and UpdateNetworkTime depends on
chrome/browser/browser_process.h, so this change introduces the
HttpPostProviderFactoryGetter callback typedef to SBH, which just takes
a CancelationSignal and returns the HttpPostProviderFactory.

The HttpPostProviderFactoryGetter replaces NetworkResources in the
Initialize argument list.

BUG=512056
==========

to

==========
[Sync] Remove some http-related chrome deps from SyncBackendHostImpl.

Previously on ::Initialize, SBHI would get passed a NetworkResources
object, and use that combined with profile_->GetRequestContext(),
UpdateNetworkTime, and the Core's CancelationSignal to create an
HttpPostProviderFactory.

Uses of Profile need to go away, and UpdateNetworkTime depends on
chrome/browser/browser_process.h, so this change introduces the
HttpPostProviderFactoryGetter callback typedef to SBH, which just takes
a CancelationSignal and returns the HttpPostProviderFactory.

The HttpPostProviderFactoryGetter replaces NetworkResources in the
Initialize argument list.

BUG=512056
TBR=sky
==========

[maxbogue, 2015-10-28 23:43:16]

maxbogue@chromium.org changed reviewers:
+ sky@chromium.org

[maxbogue, 2015-10-28 23:43:22]

The CQ bit was checked by maxbogue@chromium.org

[commit-bot: I haz the power, 2015-10-28 23:43:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410013008/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410013008/80001

[maxbogue, 2015-10-29 00:23:27]

TBR'ing sky for:

chrome/browser/BUILD.gn
chrome/browser/extensions/api/preferences_private/preferences_private_apitest.cc

[commit-bot: I haz the power, 2015-10-29 01:24:59]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-10-29 01:25:39]

Patchset 5 (id:??) landed as
https://crrev.com/d2ea9249e1b8b76a73a6af3d50db9b523926f671
Cr-Commit-Position: refs/heads/master@{#356725}