[Sync] Fix ConfigureDataTypes bug that leads to extra nudge

[Sync] Fix ConfigureDataTypes bug that leads to extra nudge

BUG=78165
TEST=

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

[akalin, 2011-04-01 20:13:23]

+tim for review

[akalin, 2011-04-04 18:12:00]

Ping!

On 2011/04/01 20:13:23, akalin wrote:
> +tim for review

[tim (not reviewing), 2011-04-05 04:56:01]

LGTM

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

http://codereview.chromium.org/6764024/diff/3002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.cc:354: /* static */
style typically "// static"

Also, I'd call this 'Make' or 'Build' rather than Get just to convey ownership.

http://codereview.chromium.org/6764024/diff/3002/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host_unittest.cc (right):

http://codereview.chromium.org/6764024/diff/3002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host_unittest.cc:29: class
MockSyncFrontend : public SyncFrontend {
I've been waiting for MockSyncFrontend day for a very long time.
April 4, forevermore.

http://codereview.chromium.org/6764024/diff/3002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host_unittest.cc:63: //
ProfileSyncService.
what you lose is the convenience / peace of mind of having all sync prefs in one
place;  looking at the browser_main initial register prefs calls, that seemed to
be the pattern (at least at the time).

[akalin, 2011-04-05 09:55:45]

Checking in as soon as trybots pass

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

http://codereview.chromium.org/6764024/diff/3002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.cc:354: /* static */
On 2011/04/05 04:56:01, timsteele wrote:
> style typically "// static"
> 
> Also, I'd call this 'Make' or 'Build' rather than Get just to convey
ownership.

Done.

http://codereview.chromium.org/6764024/diff/3002/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host_unittest.cc (right):

http://codereview.chromium.org/6764024/diff/3002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host_unittest.cc:63: //
ProfileSyncService.
On 2011/04/05 04:56:01, timsteele wrote:
> what you lose is the convenience / peace of mind of having all sync prefs in
one
> place;  looking at the browser_main initial register prefs calls, that seemed
to
> be the pattern (at least at the time).

I'm more concerned with there being a magic incantation needed before creating a
SBH.  Maybe some sort of RegisterSyncPrefs() function?  Anyway, modified TODO.

[tim (not reviewing), 2011-04-19 18:07:05]

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

http://codereview.chromium.org/6764024/diff/6002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.cc:370: state->deleted_type =
(routing_info->erase(type) > 0);
this line is wrong.  we need to just set deleted_type to true here, because
DeactivateDataType will be called and is currently what yanks things from the
routing_info.

[tim (not reviewing), 2011-04-19 18:13:12]

On 2011/04/19 18:07:05, timsteele wrote:
>
http://codereview.chromium.org/6764024/diff/6002/chrome/browser/sync/glue/syn...
> File chrome/browser/sync/glue/sync_backend_host.cc (right):
> 
>
http://codereview.chromium.org/6764024/diff/6002/chrome/browser/sync/glue/syn...
> chrome/browser/sync/glue/sync_backend_host.cc:370: state->deleted_type =
> (routing_info->erase(type) > 0);
> this line is wrong.  we need to just set deleted_type to true here, because
> DeactivateDataType will be called and is currently what yanks things from the
> routing_info.

hit send too fast
DDT happens first, meaning we never end up setting deleted_type to true here, so
we never purge. this can manifest as bug 56416