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

chrome/browser/sync/sync_ui_util.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/sync_ui_util.h"
 
 #include "base/i18n/number_formatting.h"
 #include "base/i18n/time_formatting.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
@@ skipping 492 matching lines @@
   if (!service) {
     strings->SetString("summary", "SYNC DISABLED");
   } else {
     sync_api::SyncManager::Status full_status(
         service->QueryDetailedSyncStatus());
 
     strings->SetString("service_url", service->sync_service_url().spec());
     strings->SetString("summary",
                        ProfileSyncService::BuildSyncStatusSummaryText(
                        full_status.summary));
+    if (full_status.summary == sync_api::SyncManager::Status::UNININITIALIZED) {

[rlarocque, 2012/02/02 22:44:13]

I'm not sure this is a good idea.  

What is the purpose of about:sync?  It's certainly not built for users.  If it
were, it would be prettier and internationalized.

If the purpose is to provide useful information for developers, then this is
unnecessary.  A developer who wishes to know the cause of this status would read
the code to find out where it came from.

[lipalani1, 2012/02/03 01:55:39]

Answer in the previous comment.
On 2012/02/02 22:44:13, rlarocque wrote:

[rlarocque, 2012/02/03 18:55:03]

I still think this is unusual.  

We have plenty of other variables that are even more misleading that don't have
any explanatory text.  Why annotate this one?  

Does it make sense to hard-code the explanation here, rather than in AllStatus
where the flag is given meaning in the first place?

Wouldn't it be easier to give the value an easier to interpret name?

+       strings->Set("summary_explanation_needed",
+                    new base::FundamentalValue(true));
+      std::string message = "A sync has not happened since restart. ";
+      message += "The following values are speculative.";
+      strings->SetString("summary_explanation_message", message);
+    }
 
     strings->SetString("version", GetVersionString());
     strings->Set("authenticated",
                  new base::FundamentalValue(full_status.authenticated));
     strings->SetString("auth_problem",
                        sync_ui_util::MakeSyncAuthErrorText(
                        service->GetAuthError().state()));
 
     strings->SetString("time_since_sync", service->GetLastSyncedTimeString());
 
@@ skipping 30 matching lines @@
                                    full_status.conflicting_count);
     sync_ui_util::AddIntSyncDetail(details,
                                    "Local Overwrites",
                                    full_status.num_local_overwrites_total);
     sync_ui_util::AddIntSyncDetail(details,
                                    "Server Overwrites",
                                    full_status.num_server_overwrites_total);
     sync_ui_util::AddBoolSyncDetail(details, "Syncing", full_status.syncing);
     sync_ui_util::AddBoolSyncDetail(details,
                                     "Initial Sync Ended",
-                                    full_status.initial_sync_ended);
+                                    full_status.sync_count > 0);

[rlarocque, 2012/02/02 22:44:13]

That's not what initial_sync_ended used to mean.  The old initial_sync_ended was
set only when we had fully downloaded all data for all enabled data types.  This
usually requires several *successful* sync sessions, and is based on information
that is persisted to the database after first-time sync.  

[lipalani1, 2012/02/03 01:55:39]

code reverted. I kind of don't like that piece of information as it is
misleading. people could interpret this as the first sync of the account or
first sync after restart. but may be in another CL.
On 2012/02/02 22:44:13, rlarocque wrote:

     sync_ui_util::AddIntSyncDetail(details,
                                    "Updates Available",
                                    full_status.updates_available);
     sync_ui_util::AddIntSyncDetail(details,
                                    "Updates Downloaded (All)",
                                    full_status.updates_received);
     sync_ui_util::AddIntSyncDetail(details,
                                    "Updates Downloaded (Tombstones)",
                                    full_status.tombstone_updates_received);
     sync_ui_util::AddIntSyncDetail(details,
@@ skipping 123 matching lines @@
     }
   } else {
     version_modifier = " " + version_modifier;
   }
   return chrome_version.Name() + " " + chrome_version.OSType() + " " +
       chrome_version.Version() + " (" + chrome_version.LastChange() + ")" +
       version_modifier;
 }
 
 }  // namespace sync_ui_util