Issue 6528042: sync: rewrite DataTypeManagerImpl without pause/resume logic.

Issue 6528042: sync: rewrite DataTypeManagerImpl without pause/resume logic. (Closed)

Created:
9 years, 10 months ago by tim (not reviewing)

Modified:
9 years, 6 months ago

Reviewers:
Nicolas Zea

CC:
chromium-reviews, Raghu Simha, ncarter (slow), Paweł Hajdan Jr., tim (not reviewing), idana

Base URL:
svn://svn.chromium.org/chrome/trunk/src/chrome

Visibility:
Public.

Description

sync: rewrite DataTypeManagerImpl without pause/resume logic. Simpler! -=150 lines :) Need this to be able to use SyncerThread2. BUG=26339 TEST=DataTypeManagerImpl2Test Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=75308

Patch Set 1 : lint #

Patch Set 2 : gypi #

Total comments: 6

Patch Set 3 : comments #

Created: 9 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+884 lines, -0 lines)			Patch
M	chrome/browser/sync/glue/data_type_manager.h	View		1 chunk	+5 lines, -0 lines	0 comments	Download
A	chrome/browser/sync/glue/data_type_manager_impl2.h	View		1 chunk	+70 lines, -0 lines	0 comments	Download
A	chrome/browser/sync/glue/data_type_manager_impl2.cc	View	1 2	1 chunk	+337 lines, -0 lines	0 comments	Download
A	chrome/browser/sync/glue/data_type_manager_impl2_unittest.cc	View		1 chunk	+469 lines, -0 lines	0 comments	Download
M	chrome/chrome_browser.gypi	View	1	1 chunk	+2 lines, -0 lines	0 comments	Download
M	chrome/chrome_tests.gypi	View	1	1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Nicolas Zea

LGTM with some comment nits. http://codereview.chromium.org/6528042/diff/2005/chrome/browser/sync/glue/data_type_manager_impl2.cc File chrome/browser/sync/glue/data_type_manager_impl2.cc (right): http://codereview.chromium.org/6528042/diff/2005/chrome/browser/sync/glue/data_type_manager_impl2.cc#newcode21 chrome/browser/sync/glue/data_type_manager_impl2.cc:21: syncable::NIGORI, Presumably this is ...

9 years, 10 months ago (2011-02-16 18:40:53 UTC) #2

tim (not reviewing)

9 years, 10 months ago (2011-02-17 21:27:25 UTC) #3

Thanks

http://codereview.chromium.org/6528042/diff/2005/chrome/browser/sync/glue/dat...
File chrome/browser/sync/glue/data_type_manager_impl2.cc (right):

http://codereview.chromium.org/6528042/diff/2005/chrome/browser/sync/glue/dat...
chrome/browser/sync/glue/data_type_manager_impl2.cc:21: syncable::NIGORI,
On 2011/02/16 18:40:53, nzea wrote:
> Presumably this is just for the sake of being complete? If so, a comment
noting
> that Nigori doesn't actually have a datatype controller/change processor would
> be helpful.

Done.

http://codereview.chromium.org/6528042/diff/2005/chrome/browser/sync/glue/dat...
chrome/browser/sync/glue/data_type_manager_impl2.cc:197: // If no more data
types need starting, we're done.  Resume the sync
On 2011/02/16 18:40:53, nzea wrote:
> "Resume sync backend" no longer applicable, right?

Done.

http://codereview.chromium.org/6528042/diff/2005/chrome/browser/sync/glue/dat...
chrome/browser/sync/glue/data_type_manager_impl2.cc:213: // need to reset. 
Resume the syncer.
On 2011/02/16 18:40:53, nzea wrote:
> Same here.

Done.

Expand Messages | Collapse Messages