[Sync] - In about:sync page display if the first sync after restart has taken place or not.

chrome/browser/sync/engine/all_status.cc

 // Copyright (c) 2012 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/all_status.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "base/port.h"
 #include "chrome/browser/sync/engine/net/server_connection_manager.h"
 #include "chrome/browser/sync/protocol/service_constants.h"
 #include "chrome/browser/sync/sessions/session_state.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
 #include "chrome/browser/sync/syncable/model_type.h"
 
 namespace browser_sync {
 
 AllStatus::AllStatus() {
-  status_.summary = sync_api::SyncManager::Status::OFFLINE;
+  status_.summary = sync_api::SyncManager::Status::UNINITIALIZED;
   status_.initial_sync_ended = true;
   status_.notifications_enabled = false;
   status_.cryptographer_ready = false;
   status_.crypto_has_pending_keys = false;
 }
 
 AllStatus::~AllStatus() {
 }
 
 sync_api::SyncManager::Status AllStatus::CreateBlankStatus() const {
@@ skipping 31 matching lines @@
   if (errors.consecutive_errors > status.max_consecutive_errors)
     status.max_consecutive_errors = errors.consecutive_errors;
 
   status.updates_available += snapshot->num_server_changes_remaining;
   status.sync_protocol_error = snapshot->errors.sync_protocol_error;
 
   // Accumulate update count only once per session to avoid double-counting.
   // TODO(ncarter): Make this realtime by having the syncer_status
   // counter preserve its value across sessions.  http://crbug.com/26339
   if (event.what_happened == SyncEngineEvent::SYNC_CYCLE_ENDED) {
+    ++status.sync_count;
     status.updates_received +=
         snapshot->syncer_status.num_updates_downloaded_total;
     status.tombstone_updates_received +=
         snapshot->syncer_status.num_tombstone_updates_downloaded_total;
     status.num_local_overwrites_total +=
         snapshot->syncer_status.num_local_overwrites;
     status.num_server_overwrites_total +=
         snapshot->syncer_status.num_server_overwrites;
     if (snapshot->syncer_status.num_updates_downloaded_total == 0) {
       ++status.empty_get_updates;
     } else {
       ++status.nonempty_get_updates;
     }
     if (snapshot->syncer_status.num_successful_commits == 0 &&
         snapshot->syncer_status.num_updates_downloaded_total == 0) {
       ++status.useless_sync_cycles;
     } else {
       ++status.useful_sync_cycles;
     }
   }
   return status;
 }
 
 void AllStatus::CalcStatusChanges() {
   const bool unsynced_changes = status_.unsynced_count > 0;
-  const bool online = status_.authenticated &&
+  const bool online = status_.sync_count > 0 && status_.authenticated &&
     status_.server_reachable && status_.server_up;
+
+  if (status_.syncing) {

[rlarocque, 2012/02/08 00:07:37]

Why not make this a big series of if/else if branches?

[lipalani1, 2012/02/08 19:41:50]

Nothing functionally wrong with that. The reason is maintainability.

If the 'if else' branching was based on the value of 1 or even 2 variables it
would not be this bad. This is on few different random variables. What I am
hoping to have is some structure in the code so if you are messing with syncing
state you modify the first block, for idle state modify the second block and for
error cases(the most likely case that will get changed soon) modify the third
block. 
On 2012/02/08 00:07:37, rlarocque wrote:

+    status_.summary = sync_api::SyncManager::Status::SYNCING;
+    return;
+  }
+
+  // If it is not syncing check if it is online.
   if (online) {
-    if (status_.syncing)
-      status_.summary = sync_api::SyncManager::Status::SYNCING;
-    else
-      status_.summary = sync_api::SyncManager::Status::READY;
-  } else if (!status_.initial_sync_ended) {
+    status_.summary = sync_api::SyncManager::Status::READY;
+    return;
+  }
+
+  // It is not online. Find out more details for the summary.
+  if (!status_.initial_sync_ended) {
     status_.summary = sync_api::SyncManager::Status::OFFLINE_UNUSABLE;
   } else if (unsynced_changes) {
     status_.summary = sync_api::SyncManager::Status::OFFLINE_UNSYNCED;
+  } else if (status_.sync_count > 0) {
+    status_.summary = sync_api::SyncManager::Status::OFFLINE;
   } else {
-    status_.summary = sync_api::SyncManager::Status::OFFLINE;
+    status_.summary = sync_api::SyncManager::Status::UNINITIALIZED;
   }
 }
 
 void AllStatus::OnSyncEngineEvent(const SyncEngineEvent& event) {
   ScopedStatusLock lock(this);
   switch (event.what_happened) {
     case SyncEngineEvent::SYNC_CYCLE_BEGIN:
     case SyncEngineEvent::STATUS_CHANGED:
     case SyncEngineEvent::SYNC_CYCLE_ENDED:
       status_ = CalcSyncing(event);
@@ skipping 70 matching lines @@
     : allstatus_(allstatus) {
   allstatus->mutex_.Acquire();
 }
 
 ScopedStatusLock::~ScopedStatusLock() {
   allstatus_->CalcStatusChanges();
   allstatus_->mutex_.Release();
 }
 
 }  // namespace browser_sync