Server directed error handling backend code.

chrome/browser/sync/engine/sync_scheduler.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/engine/sync_scheduler.h"
 
 #include <algorithm>
 #include <cstring>
 
 #include "base/compiler_specific.h"
@@ skipping 11 matching lines @@
 
 namespace browser_sync {
 
 using sessions::SyncSession;
 using sessions::SyncSessionSnapshot;
 using sessions::SyncSourceInfo;
 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() {}
 
 SyncScheduler::WaitInterval::WaitInterval()
     : mode(UNKNOWN),
       had_nudge(false) {
 }
 
 SyncScheduler::WaitInterval::~WaitInterval() {}
 
@@ skipping 1035 matching lines @@
   sessions_commit_delay_ = new_delay;
 }
 
 void SyncScheduler::OnShouldStopSyncingPermanently() {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   SVLOG(2) << "OnShouldStopSyncingPermanently";
   syncer_->RequestEarlyExit();  // Thread-safe.
   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_);
   PostTask(FROM_HERE, "CheckServerConnectionManagerStatus",
            method_factory_.NewRunnableMethod(
                &SyncScheduler::CheckServerConnectionManagerStatus,
                event.connection_code));
 }
 
 void SyncScheduler::set_notifications_enabled(bool notifications_enabled) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   session_context_->set_notifications_enabled(notifications_enabled);
 }
 
 base::TimeDelta SyncScheduler::sessions_commit_delay() const {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   return sessions_commit_delay_;
 }
 
 #undef SVLOG_LOC
 
 #undef SVLOG
 
 #undef SLOG
 
 #undef ENUM_CASE
 
 }  // browser_sync