[Sync] Make SyncBackendHost::Core use a WeakHandle to its host

[Sync] Make SyncBackendHost::Core use a WeakHandle to its host

This removes a lot of boilerplate with checking host_ (which was never
[locally] thread-safe, anyway).

Make as many things in SyncBackendHost private as possible.

BUG=103410, 107279
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114597

[akalin, 2011-12-14 02:34:38]

+zea for review

Sorry for the lame diff.  I moved some code around.

[tim (not reviewing), 2011-12-14 03:17:02]

On 2011/12/14 02:34:38, akalin wrote:
> +zea for review
> 
> Sorry for the lame diff.  I moved some code around.

re: "was never thread-safe anyway"

We only modify host_ from the sync_thread in DoShutdown, and we actually join on
the sync_thread_ right after posting that task waiting for it to finish, right? 
I'm curious if you spotted true disaster there or not...

[akalin, 2011-12-14 07:44:48]

On 2011/12/14 03:17:02, timsteele wrote:
> On 2011/12/14 02:34:38, akalin wrote:
> > +zea for review
> > 
> > Sorry for the lame diff.  I moved some code around.
> 
> re: "was never thread-safe anyway"
> 
> We only modify host_ from the sync_thread in DoShutdown, and we actually join
on
> the sync_thread_ right after posting that task waiting for it to finish,
right? 
> I'm curious if you spotted true disaster there or not...

Ah, the fact that we join immediately after posting DoShutdown (the only writer)
makes it thread-safe, I think.  So it's "globally" thread-safe.

Amending CL description.

[Nicolas Zea, 2011-12-14 20:37:25]

http://codereview.chromium.org/8938013/diff/8001/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host.cc (right):

http://codereview.chromium.org/8938013/diff/8001/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.cc:140:
base::Bind(&MakeHttpBridgeFactory,
Since MakeHttpBridgeFactory is now an anon function, do we need the closure? Or
is this primarily for unit tests?

http://codereview.chromium.org/8938013/diff/8001/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host.h (right):

http://codereview.chromium.org/8938013/diff/8001/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.h:317: InitializationState
initialization_state_;
I think style guide prefers private member, protected setter/getters?

[akalin, 2011-12-15 01:21:05]

PTAL

http://codereview.chromium.org/8938013/diff/8001/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host.cc (right):

http://codereview.chromium.org/8938013/diff/8001/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.cc:140:
base::Bind(&MakeHttpBridgeFactory,
On 2011/12/14 20:37:25, nzea wrote:
> Since MakeHttpBridgeFactory is now an anon function, do we need the closure?
Or
> is this primarily for unit tests?

Yeah.  Added comment in header file saying the closure is for testing.

http://codereview.chromium.org/8938013/diff/8001/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host.h (right):

http://codereview.chromium.org/8938013/diff/8001/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.h:317: InitializationState
initialization_state_;
On 2011/12/14 20:37:25, nzea wrote:
> I think style guide prefers private member, protected setter/getters?

Done.  Actually only really need to check whether the state is downloading
nigori, so just added a getter for that.

[Nicolas Zea, 2011-12-15 01:34:03]

LGTM

[commit-bot: I haz the power, 2011-12-15 01:43:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/8938013/13001

[commit-bot: I haz the power, 2011-12-15 01:43:15]

Can't apply patch for file chrome/browser/sync/glue/sync_backend_host.cc.
While running patch -p1 --forward --force;
patching file chrome/browser/sync/glue/sync_backend_host.cc
Hunk #16 FAILED at 855.
Hunk #17 FAILED at 899.
Hunk #18 succeeded at 1085 (offset 4 lines).
Hunk #19 succeeded at 1096 (offset 4 lines).
Hunk #20 succeeded at 1119 (offset 4 lines).
2 out of 20 hunks FAILED -- saving rejects to file
chrome/browser/sync/glue/sync_backend_host.cc.rej

[commit-bot: I haz the power, 2011-12-15 02:30:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/8938013/20001

[commit-bot: I haz the power, 2011-12-15 05:04:32]

Change committed as 114597