Parse and Store the throttled datatypes and the unthrottling time, which is obtained from the server response, in the sync_session_context.

This is the first patch for per datatype throttling feature. The next patch would consume this data to decide what items to commit.

BUG=104819
TEST=sync_unit_tests.exe, sync_integration_tests.exe, unit_tests.exe


Review URL: http://codereview.chromium.org/8595023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@111408 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 142 insertions, 6 deletions

diff --git a/chrome/browser/sync/engine/syncer_proto_util.cc b/chrome/browser/sync/engine/syncer_proto_util.cc
index 7da3140..dbee66e 100644
--- a/chrome/browser/sync/engine/syncer_proto_util.cc
+++ b/chrome/browser/sync/engine/syncer_proto_util.cc
@@ -188,6 +188,20 @@ base::TimeDelta SyncerProtoUtil::GetThrottleDelay(
   return throttle_delay;
 }
 
+void SyncerProtoUtil::HandleThrottleError(
+    const SyncProtocolError& error,
+    const base::TimeTicks& throttled_until,
+    sessions::SyncSessionContext* context,
+    sessions::SyncSession::Delegate* delegate) {
+  DCHECK_EQ(error.error_type, browser_sync::THROTTLED);
+  if (error.error_data_types.size() > 0) {
+     context->SetUnthrottleTime(error.error_data_types, throttled_until);
+  } else {
+    // No datatypes indicates the client should be completely throttled.
+    delegate->OnSilencedUntil(throttled_until);
+  }
+}
+
 namespace {
 
 // Helper function for an assertion in PostClientToServerMessage.
@@ -259,6 +273,17 @@ browser_sync::SyncProtocolError ConvertErrorPBToLocalType(
   sync_protocol_error.url = error.url();
   sync_protocol_error.action = ConvertClientActionPBToLocalClientAction(
       error.action());
+
+  if (error.error_data_type_ids_size() > 0) {
+    // THROTTLED is currently the only error code that uses |error_data_types|.
+    DCHECK_EQ(error.error_type(), ClientToServerResponse::THROTTLED);
+    for (int i = 0; i < error.error_data_type_ids_size(); ++i) {
+      sync_protocol_error.error_data_types.insert(
+          syncable::GetModelTypeFromExtensionFieldNumber(
+              error.error_data_type_ids(i)));
+    }
+  }
+
   return sync_protocol_error;
 }
 
@@ -332,8 +357,10 @@ bool SyncerProtoUtil::PostClientToServerMessage(
       return true;
     case browser_sync::THROTTLED:
       LOG(WARNING) << "Client silenced by server.";
-      session->delegate()->OnSilencedUntil(base::TimeTicks::Now() +
-          GetThrottleDelay(*response));
+      HandleThrottleError(sync_protocol_error,
+                          base::TimeTicks::Now() + GetThrottleDelay(*response),
+                          session->context(),
+                          session->delegate());
       return false;
     case browser_sync::TRANSIENT_ERROR:
       return false;
diff --git a/chrome/browser/sync/engine/syncer_proto_util.h b/chrome/browser/sync/engine/syncer_proto_util.h
index 325a795..0b209a7 100644
--- a/chrome/browser/sync/engine/syncer_proto_util.h
+++ b/chrome/browser/sync/engine/syncer_proto_util.h
@@ -9,6 +9,8 @@
 #include <string>
 
 #include "base/gtest_prod_util.h"
+#include "base/time.h"
+#include "chrome/browser/sync/sessions/sync_session.h"
 #include "chrome/browser/sync/syncable/blob.h"
 #include "chrome/browser/sync/syncable/model_type.h"
 
@@ -25,7 +27,8 @@ class EntitySpecifics;
 namespace browser_sync {
 
 namespace sessions {
-class SyncSession;
+class SyncProtocolError;
+class SyncSessionContext;
 }
 
 class ClientToServerMessage;
@@ -115,10 +118,17 @@ class SyncerProtoUtil {
   static base::TimeDelta GetThrottleDelay(
       const sync_pb::ClientToServerResponse& response);
 
+  static void HandleThrottleError(const SyncProtocolError& error,
+                                  const base::TimeTicks& throttled_until,
+                                  sessions::SyncSessionContext* context,
+                                  sessions::SyncSession::Delegate* delegate);
+
   friend class SyncerProtoUtilTest;
   FRIEND_TEST_ALL_PREFIXES(SyncerProtoUtilTest, AddRequestBirthday);
   FRIEND_TEST_ALL_PREFIXES(SyncerProtoUtilTest, PostAndProcessHeaders);
   FRIEND_TEST_ALL_PREFIXES(SyncerProtoUtilTest, VerifyResponseBirthday);
+  FRIEND_TEST_ALL_PREFIXES(SyncerProtoUtilTest, HandleThrottlingNoDatatypes);
+  FRIEND_TEST_ALL_PREFIXES(SyncerProtoUtilTest, HandleThrottlingWithDatatypes);
 
   DISALLOW_COPY_AND_ASSIGN(SyncerProtoUtil);
 };
diff --git a/chrome/browser/sync/engine/syncer_proto_util_unittest.cc b/chrome/browser/sync/engine/syncer_proto_util_unittest.cc
index 809ce1e..c05b192 100644
--- a/chrome/browser/sync/engine/syncer_proto_util_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_proto_util_unittest.cc
@@ -9,7 +9,13 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/message_loop.h"
+#include "base/time.h"
 #include "chrome/browser/sync/engine/syncproto.h"
+#include "chrome/browser/sync/sessions/session_state.h"
+#include "chrome/browser/sync/sessions/sync_session_context.h"
+#include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
+#include "chrome/browser/sync/protocol/password_specifics.pb.h"
+#include "chrome/browser/sync/protocol/sync.pb.h"
 #include "chrome/browser/sync/syncable/blob.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
 #include "chrome/browser/sync/syncable/syncable.h"
@@ -20,8 +26,33 @@
 
 using syncable::Blob;
 using syncable::ScopedDirLookup;
+using syncable::ModelTypeSet;
+using ::testing::_;
 
 namespace browser_sync {
+using sessions::SyncSessionContext;
+
+class MockSyncSessionContext : public SyncSessionContext {
+ public:
+  MockSyncSessionContext() {}
+  ~MockSyncSessionContext() {}
+  MOCK_METHOD2(SetUnthrottleTime, void(const ModelTypeSet&,
+                                       const base::TimeTicks&));
+};
+
+class MockDelegate : public sessions::SyncSession::Delegate {
+ public:
+   MockDelegate() {}
+   ~MockDelegate() {}
+
+  MOCK_METHOD0(IsSyncingCurrentlySilenced, bool());
+  MOCK_METHOD1(OnReceivedShortPollIntervalUpdate, void(const base::TimeDelta&));
+  MOCK_METHOD1(OnReceivedLongPollIntervalUpdate ,void(const base::TimeDelta&));
+  MOCK_METHOD1(OnReceivedSessionsCommitDelay, void(const base::TimeDelta&));
+  MOCK_METHOD1(OnSyncProtocolError, void(const sessions::SyncSessionSnapshot&));
+  MOCK_METHOD0(OnShouldStopSyncingPermanently, void());
+  MOCK_METHOD1(OnSilencedUntil, void(const base::TimeTicks&));
+};
 
 TEST(SyncerProtoUtil, TestBlobToProtocolBufferBytesUtilityFunctions) {
   unsigned char test_data1[] = {1, 2, 3, 4, 5, 6, 7, 8, 0, 1, 4, 2, 9};
@@ -240,4 +271,31 @@ TEST_F(SyncerProtoUtilTest, PostAndProcessHeaders) {
       msg, &response));
 }
 
+TEST_F(SyncerProtoUtilTest, HandleThrottlingWithDatatypes) {
+  MockSyncSessionContext context;
+  SyncProtocolError error;
+  error.error_type = browser_sync::THROTTLED;
+  syncable::ModelTypeSet types;
+  types.insert(syncable::BOOKMARKS);
+  types.insert(syncable::PASSWORDS);
+  error.error_data_types = types;
+
+  base::TimeTicks ticks = base::TimeTicks::Now();
+
+  EXPECT_CALL(context, SetUnthrottleTime(types, ticks));
+
+  SyncerProtoUtil::HandleThrottleError(error, ticks, &context, NULL);
+}
+
+TEST_F(SyncerProtoUtilTest, HandleThrottlingNoDatatypes) {
+  MockDelegate delegate;
+  SyncProtocolError error;
+  error.error_type = browser_sync::THROTTLED;
+
+  base::TimeTicks ticks = base::TimeTicks::Now();
+
+  EXPECT_CALL(delegate, OnSilencedUntil(ticks));
+
+  SyncerProtoUtil::HandleThrottleError(error, ticks, NULL, &delegate);
+}
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/protocol/sync.proto b/chrome/browser/sync/protocol/sync.proto
index b4e1826..9f46bcd 100644
--- a/chrome/browser/sync/protocol/sync.proto
+++ b/chrome/browser/sync/protocol/sync.proto
@@ -420,7 +420,7 @@ message ClearUserDataResponse {
 
 message ClientToServerMessage {
   required string share = 1;
-  optional int32 protocol_version = 2 [default = 29];
+  optional int32 protocol_version = 2 [default = 30];
   enum Contents {
     COMMIT = 1;
     GET_UPDATES = 2;
@@ -627,7 +627,8 @@ message ClientToServerResponse {
     MIGRATION_DONE     = 9;   // Migration has finished for one or more data
                               // types.  Client should clear the cache for
                               // these data types only and then re-sync with
-                              // a server.
+                              // a server. The migrated datatypes are in the
+                              // |migrated_datatype_ids| field.
     UNKNOWN            = 100; // Unknown value. This should never be explicitly
                               // used; it is the default value when an
                               // out-of-date client parses a value it doesn't
@@ -650,6 +651,10 @@ message ClientToServerResponse {
       UNKNOWN_ACTION              = 5; // This is the default.
     }
     optional Action action              = 4 [default = UNKNOWN_ACTION];
+
+    // Currently only meaningful if |error_type| is throttled. If this field
+    // is absent then the whole client (all datatypes) is throttled.
+    repeated int32 error_data_type_ids = 5;
   }
 
   optional Error error = 13;
diff --git a/chrome/browser/sync/protocol/sync_protocol_error.h b/chrome/browser/sync/protocol/sync_protocol_error.h
index 0544aa0..0729989 100644
--- a/chrome/browser/sync/protocol/sync_protocol_error.h
+++ b/chrome/browser/sync/protocol/sync_protocol_error.h
@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/values.h"
+#include "chrome/browser/sync/syncable/model_type.h"
 
 namespace browser_sync{
 
@@ -67,6 +68,7 @@ struct SyncProtocolError {
   std::string error_description;
   std::string url;
   ClientAction action;
+  syncable::ModelTypeSet error_data_types;
   SyncProtocolError();
   ~SyncProtocolError();
   DictionaryValue* ToValue() const;
diff --git a/chrome/browser/sync/sessions/sync_session_context.cc b/chrome/browser/sync/sessions/sync_session_context.cc
index b92015a..c0aeb34 100644
--- a/chrome/browser/sync/sessions/sync_session_context.cc
+++ b/chrome/browser/sync/sessions/sync_session_context.cc
@@ -33,6 +33,14 @@ SyncSessionContext::SyncSessionContext(
     listeners_.AddObserver(*it);
 }
 
+SyncSessionContext::SyncSessionContext()
+    : connection_manager_(NULL),
+      directory_manager_(NULL),
+      registrar_(NULL),
+      extensions_activity_monitor_(NULL),
+      debug_info_getter_(NULL) {
+}
+
 SyncSessionContext::~SyncSessionContext() {
   // In unittests, there may be no UI thread, so the above will fail.
   if (!BrowserThread::DeleteSoon(BrowserThread::UI, FROM_HERE,
@@ -41,5 +49,14 @@ SyncSessionContext::~SyncSessionContext() {
   }
 }
 
+void SyncSessionContext::SetUnthrottleTime(const syncable::ModelTypeSet& types,
+                                           const base::TimeTicks& time) {
+  for (syncable::ModelTypeSet::const_iterator it = types.begin();
+       it != types.end();
+       ++it) {
+    unthrottle_times_[*it] = time;
+  }
+}
+
 }  // namespace sessions
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/sessions/sync_session_context.h b/chrome/browser/sync/sessions/sync_session_context.h
index 89a75c4..b08b64c 100644
--- a/chrome/browser/sync/sessions/sync_session_context.h
+++ b/chrome/browser/sync/sessions/sync_session_context.h
@@ -19,9 +19,11 @@
 #define CHROME_BROWSER_SYNC_SESSIONS_SYNC_SESSION_CONTEXT_H_
 #pragma once
 
+#include <map>
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
+#include "base/time.h"
 #include "chrome/browser/sync/engine/model_safe_worker.h"
 #include "chrome/browser/sync/engine/syncer_types.h"
 #include "chrome/browser/sync/sessions/debug_info_getter.h"
@@ -52,7 +54,10 @@ class SyncSessionContext {
                      ModelSafeWorkerRegistrar* model_safe_worker_registrar,
                      const std::vector<SyncEngineEventListener*>& listeners,
                      DebugInfoGetter* debug_info_getter);
-  ~SyncSessionContext();
+
+  // Empty constructor for unit tests.
+  SyncSessionContext();
+  virtual ~SyncSessionContext();
 
   ConflictResolver* resolver() { return resolver_; }
   ServerConnectionManager* connection_manager() {
@@ -103,7 +108,15 @@ class SyncSessionContext {
                       OnSyncEngineEvent(event));
   }
 
+  // This is virtual for unit tests.
+  // TODO(lipalani)  Consult the unthrottle times before committing data to
+  // the server.
+  virtual void SetUnthrottleTime(const syncable::ModelTypeSet& types,
+                                 const base::TimeTicks& time);
+
  private:
+  typedef std::map<syncable::ModelType, base::TimeTicks> UnthrottleTimes;
+
   // Rather than force clients to set and null-out various context members, we
   // extend our encapsulation boundary to scoped helpers that take care of this
   // once they are allocated. See definitions of these below.
@@ -147,6 +160,10 @@ class SyncSessionContext {
   // client behavior on server side.
   DebugInfoGetter* const debug_info_getter_;
 
+  // This is a map from throttled data types to the time at which they can be
+  // unthrottled.
+  UnthrottleTimes unthrottle_times_;
+
   DISALLOW_COPY_AND_ASSIGN(SyncSessionContext);
 };