[Sync] Prevent uploading throttled datatypes.

This is the second and final patch of changes for implementing the per datatype throttling feature. Includes the tests.

This consumes the throttled datatypes set in the context and uses that to filter the unsynced handles that we use for committing.

BUG=104819
TEST=unit_tests.exe, sync_unit_tests.exe, sync_integration_tests.exe, manual test cases


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

[lipalani1, 2011-11-22 23:01:24]

Please review.

[akalin, 2011-11-22 23:34:16]

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/engine/g...
File chrome/browser/sync/engine/get_commit_ids_command.cc (right):

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.cc:76: // 2. It is not
currently throttled.
Its type is

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/engine/g...
File chrome/browser/sync/engine/get_commit_ids_command.h (right):

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.h:117: // 2. It is currently
throttled.
"It is" "Its type is"

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.h:121: const
syncable::ModelTypeSet& throttled_types);
typically input arguments (those that don't change) go before output arguments
(those that may change).  This applies for all functions in this file.

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/sessions...
File chrome/browser/sync/sessions/sync_session_context.h (right):

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/sessions...
chrome/browser/sync/sessions/sync_session_context.h:118: virtual
syncable::ModelTypeSet UpdateAndGetCurrentlyThrottledTypes();
I feel like it's kind of awkward to expose the fact that we're pruning the map. 
Why not have:

syncable::ModelTypeSet GetThrottledTypes(base::Time t);

and do the pruning somewhere else, say, when we would update the UnthrottleTimes
map?

[lipalani1, 2011-11-23 01:33:51]

Fixed all feedback.

I have done the pruning in sync_begin.

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/engine/g...
File chrome/browser/sync/engine/get_commit_ids_command.cc (right):

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.cc:76: // 2. It is not
currently throttled.
On 2011/11/22 23:34:17, akalin wrote:
> Its type is

Done.

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/engine/g...
File chrome/browser/sync/engine/get_commit_ids_command.h (right):

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.h:117: // 2. It is currently
throttled.
On 2011/11/22 23:34:17, akalin wrote:
> "It is" "Its type is"

Done.

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.h:121: const
syncable::ModelTypeSet& throttled_types);
On 2011/11/22 23:34:17, akalin wrote:
> typically input arguments (those that don't change) go before output arguments
> (those that may change).  This applies for all functions in this file.

Done.

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/sessions...
File chrome/browser/sync/sessions/sync_session_context.h (right):

http://codereview.chromium.org/8631021/diff/4001/chrome/browser/sync/sessions...
chrome/browser/sync/sessions/sync_session_context.h:118: virtual
syncable::ModelTypeSet UpdateAndGetCurrentlyThrottledTypes();
I have done this little differently. Let me know what you think.
On 2011/11/22 23:34:17, akalin wrote:
> I feel like it's kind of awkward to expose the fact that we're pruning the
map. 
> Why not have:
> 
> syncable::ModelTypeSet GetThrottledTypes(base::Time t);
> 
> and do the pruning somewhere else, say, when we would update the
UnthrottleTimes
> map?

[akalin, 2011-11-23 03:44:31]

LGTM after nits

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/engine/g...
File chrome/browser/sync/engine/get_commit_ids_command.cc (right):

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.cc:47: syncable::ModelTypeSet
throttled_types =
may as well make this a const ref

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/engine/g...
File chrome/browser/sync/engine/get_commit_ids_command.h (right):

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.h:117: // 2. It's type is
currently throttled.
It's -> Its

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/engine/s...
File chrome/browser/sync/engine/syncer_unittest.cc (right):

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/engine/s...
chrome/browser/sync/engine/syncer_unittest.cc:579:
context_->SetUnthrottleTime(throttled_types,
newline before throttled_types

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/engine/s...
chrome/browser/sync/engine/syncer_unittest.cc:592:
context_->SetUnthrottleTime(throttled_types,
here too

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/sessions...
File chrome/browser/sync/sessions/sync_session_context.h (right):

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/sessions...
chrome/browser/sync/sessions/sync_session_context.h:48: class
SyncSessionContextTest;
there's actually no need to forward-declare for FRIEND_TEST

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/sessions...
chrome/browser/sync/sessions/sync_session_context.h:117: void
UpdateThrottledTypes(const base::TimeTicks& time);
maybe a better name would be: "PruneUnthrottledTypes"

Add a comment explaining what it does, and that it's called only at the start of
a sync cycle.

http://codereview.chromium.org/8631021/diff/8002/chrome/browser/sync/sessions...
chrome/browser/sync/sessions/sync_session_context.h:118: syncable::ModelTypeSet
GetThrottledTypes() const;
Add comment explaining what this does, and that it stays constant throughout the
whole sync cycle.

[commit-bot: I haz the power, 2011-11-23 23:47:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lipalani@chromium.org/8631021/16017

[commit-bot: I haz the power, 2011-11-24 01:06:13]

Change committed as 111464