Server directed error handling backend code.

chrome/browser/sync/engine/sync_scheduler.cc

Index: chrome/browser/sync/engine/sync_scheduler.cc
diff --git a/chrome/browser/sync/engine/sync_scheduler.cc b/chrome/browser/sync/engine/sync_scheduler.cc
index 993b72755abcd019a89a4ff8380c7a91d5c9dbac..700c41abff519af9a54879ed07bc57f216eadaad 100644
--- a/chrome/browser/sync/engine/sync_scheduler.cc
+++ b/chrome/browser/sync/engine/sync_scheduler.cc
@@ -29,6 +29,39 @@ using syncable::ModelTypePayloadMap;
 using syncable::ModelTypeBitSet;
 using sync_pb::GetUpdatesCallerInfo;
 
+namespace {
+bool ShouldRequestEarlyExit(const browser_sync::sessions::SyncError& error) {
+  switch (error.error_type) {
+    case browser_sync::sessions::SUCCESS:
+    case browser_sync::sessions::MIGRATION_DONE:
+    case browser_sync::sessions::THROTTLED:
+    case browser_sync::sessions::TRANSIENT_ERROR:
+      return false;
+    case browser_sync::sessions::NOT_MY_BIRTHDAY:
+    case browser_sync::sessions::CLEAR_PENDING:
+      // If we send terminate sync early then |sync_cycle_ended| notification
+      // would not be sent. If there were no actions then |ACTIONABLE_ERROR|
+      // notification wouldnt be sent either. Then the UI layer would be left
+      // waiting forever. So assert we would send something.
+      DCHECK(error.action != browser_sync::sessions::UNKNOWN_ACTION);
+      return true;
+    case browser_sync::sessions::INVALID_CREDENTIAL:
+      // The notification for this is handled by PostAndProcessHeaders|.
+      // Server does no have to send any action for this.
+      return true;
+    // Make the default a NOTREACHED. So if a new error is introduced we
+    // think about its expected functionality.
+    default:
+      NOTREACHED();
+      return false;
+  }
+}
+
+bool IsActionableError(const browser_sync::sessions::SyncError& error) {
+  return (error.action != browser_sync::sessions::UNKNOWN_ACTION);
+}
+}  // namespace
+
 SyncScheduler::DelayProvider::DelayProvider() {}
 SyncScheduler::DelayProvider::~DelayProvider() {}
 
@@ -1084,6 +1117,31 @@ void SyncScheduler::OnShouldStopSyncingPermanently() {
   Notify(SyncEngineEvent::STOP_SYNCING_PERMANENTLY);
 }
 
+void SyncScheduler::OnRequestEarlyExit() {

[tim (not reviewing), 2011/08/24 14:47:51]

I think we should inline this method for now, and the MessageLoop DCHECK should
go in OnSyncError instead.

[lipalani1, 2011/08/25 06:14:06]

Done.

+  DCHECK_EQ(MessageLoop::current(), sync_loop_);
+  SVLOG(2) << "OnRequestEarlyExit";
+  syncer_->RequestEarlyExit();  // Thread-safe.
+}
+
+void SyncScheduler::OnActionableError(sessions::SyncSession* session) {
+  DCHECK_EQ(MessageLoop::current(), sync_loop_);
+  SVLOG(2) << "OnActionableError";
+  SyncEngineEvent event(SyncEngineEvent::ACTIONABLE_ERROR);
+  sessions::SyncSessionSnapshot snapshot(session->TakeSnapshot());
+  event.snapshot = &snapshot;
+  session->context()->NotifyListeners(event);
+}
+
+void SyncScheduler::OnSyncError(sessions::SyncSession* session) {

[tim (not reviewing), 2011/08/24 14:47:51]

We should just pass the error here, and / or just a session snapshot.  In
general we should prefer passing snapshots around than sessions, since we
usually never need access to the full mutable session object.

[lipalani1, 2011/08/25 06:14:06]

The problem is we need the snapshot/error and the context(to send the
notification). Instead of splitting them out it makes to pass the session which
is meant to hold this stuff.
On 2011/08/24 14:47:51, timsteele wrote:

[tim (not reviewing), 2011/08/25 15:58:54]

The scheduler owns the context, so there's no need to pass it.  You can pass in
the snapshot, and the scheduler can use its context pointer.

+  if (ShouldRequestEarlyExit(session->status_controller()->sync_error())) {
+    OnRequestEarlyExit();
+  }
+  if (IsActionableError(session->status_controller()->sync_error())) {
+    OnActionableError(session);
+  }

[tim (not reviewing), 2011/08/24 14:47:51]

file doesn't have braces on single line ifs

[lipalani1, 2011/08/25 06:14:06]

Done.

+}
+
+
 void SyncScheduler::OnServerConnectionEvent(
     const ServerConnectionEvent& event) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);