[Sync] Sync client should to exponential backoff when receive partial failure

components/sync/engine_impl/cycle/data_type_tracker.h

Index: components/sync/engine_impl/cycle/data_type_tracker.h
diff --git a/components/sync/engine_impl/cycle/data_type_tracker.h b/components/sync/engine_impl/cycle/data_type_tracker.h
index 9afd434a482e654302d9ff6f9423231d0ea9a89f..0a7fa71aab4b044105542ad1466a1e0171e43eaa 100644
--- a/components/sync/engine_impl/cycle/data_type_tracker.h
+++ b/components/sync/engine_impl/cycle/data_type_tracker.h
@@ -21,6 +21,27 @@ namespace syncer {
 
 class InvalidationInterface;
 
+struct WaitInterval {
+  enum BlockingMode {
+    // Uninitialized state, should not be set in practice.
+    UNKNOWN = -1,
+    // We enter a series of increasingly longer WaitIntervals if we experience
+    // repeated transient failures.  We retry at the end of each interval.
+    EXPONENTIAL_BACKOFF,
+    // A server-initiated throttled interval.  We do not allow any syncing
+    // during such an interval.
+    THROTTLED,
+  };
+  WaitInterval();
+  ~WaitInterval();
+  WaitInterval(BlockingMode mode, base::TimeDelta length);

[Nicolas Zea, 2016/11/10 23:28:21]

nit: move this constructor above the destructor

[Gang Wu, 2016/11/11 19:15:30]

Done.

+
+  static const char* GetModeString(BlockingMode mode);
+
+  BlockingMode mode;
+  base::TimeDelta length;
+};
+
 // A class to track the per-type scheduling data.
 class DataTypeTracker {
  public:
@@ -94,16 +115,27 @@ class DataTypeTracker {
   // Returns true if the type is currently throttled.
   bool IsThrottled() const;
 
-  // Returns the time until this type's throttling interval expires.  Should not
-  // be called unless IsThrottled() returns true.  The returned value will be
-  // increased to zero if it would otherwise have been negative.
-  base::TimeDelta GetTimeUntilUnthrottle(base::TimeTicks now) const;
+  // Returns true if the type is currently backed off.
+  bool IsBackedOff() const;

[Nicolas Zea, 2016/11/10 23:28:21]

Does it make sense to combine IsThrottled and IsBackedOff into a single
IsBlocked method? As far as I can tell, any callsite that cares about
IsThrottled should also be checking IsBackedOff, so may as well combine the two.

[Gang Wu, 2016/11/11 19:15:30]

Done.

+
+  // Returns the time until this type's throttling or backoff interval expires.
+  // Should not be called unless IsThrottled() or IsBackedOff() returns true.
+  // The returned value will be increased to zero if it would otherwise have
+  // been negative.
+  base::TimeDelta GetTimeUntilUnblock() const;
+
+  // Returns the last backoff interval.
+  base::TimeDelta GetLastBackoffInterval() const;
 
   // Throttles the type from |now| until |now| + |duration|.
   void ThrottleType(base::TimeDelta duration, base::TimeTicks now);
 
-  // Unthrottles the type if |now| >= the throttle expiry time.
-  void UpdateThrottleState(base::TimeTicks now);
+  // Backoffs the type from |now| until |now| + |duration|.

[Nicolas Zea, 2016/11/10 23:28:21]

nit: "backoffs the type" -> "Backs off the type"

[Gang Wu, 2016/11/11 19:15:30]

Done.

+  void BackoffType(base::TimeDelta duration, base::TimeTicks now);

[Nicolas Zea, 2016/11/10 23:28:21]

nit: rename to BackOffType

[Gang Wu, 2016/11/11 19:15:30]

Done.

+
+  // Unthrottles or Unbackoffs the type if base::TimeTicks::Now() >=

[Nicolas Zea, 2016/11/10 23:28:21]

Unbackoff isn't really a word. Perhaps rephrase "Unthrottles or unbackoffs the
tyep" to "Unblocks the type"

That said, I'm a bit concerned about this. It makes sense to unthrottle after
the time is up, but backoff should only end only a successful response.

[Gang Wu, 2016/11/11 19:15:30]

Yes, fixed by add another blocking mode which is EXPONENTIAL_BACKOFF_RETRYING.

+  // |unblock_time_| expiry time.
+  void UpdateThrottleOrBackoffState();
 
   // Update the local change nudge delay for this type.
   void UpdateLocalNudgeDelay(base::TimeDelta delay);
@@ -133,9 +165,13 @@ class DataTypeTracker {
   // Set to true if this type need to get update to resolve conflict issue.
   bool sync_required_to_resolve_conflict_;
 
-  // If !unthrottle_time_.is_null(), this type is throttled and may not download
-  // or commit data until the specified time.
-  base::TimeTicks unthrottle_time_;
+  // If !unblock_time_.is_null(), this type is throttled or backed off, check
+  // |wait_interval_->mode| for specific reason. Now the datatype may not
+  // download or commit data until the specified time.
+  base::TimeTicks unblock_time_;
+
+  // Current wait state.  Null if we're not in backoff or throttling.
+  std::unique_ptr<WaitInterval> wait_interval_;
 
   // A helper to keep track invalidations we dropped due to overflow.
   std::unique_ptr<InvalidationInterface> last_dropped_invalidation_;