Export syncer return values to about:sync

chrome/browser/sync/sync_ui_util.cc

Index: chrome/browser/sync/sync_ui_util.cc
diff --git a/chrome/browser/sync/sync_ui_util.cc b/chrome/browser/sync/sync_ui_util.cc
index d0cca9d1f78dbd44098454b9241a5d22e82b60a5..928a4f301748b32e3ab875122d2b4ee25b8e3fc3 100644
--- a/chrome/browser/sync/sync_ui_util.cc
+++ b/chrome/browser/sync/sync_ui_util.cc
@@ -649,6 +649,16 @@ void ConstructAboutInformation(ProfileSyncService* service,
       sync_ui_util::AddStringSyncDetails(details, "Last Sync Source",
           browser_sync::GetUpdatesSourceString(
           snapshot->source.updates_source));
+      sync_ui_util::AddStringSyncDetails(

[rlarocque, 2012/02/22 22:01:52]

Note: There are two snapshot-like structures available in this function.

SyncManager::Status full_status: This is a Status struct managed by AllStatus. 
It can be fetched directly from the SyncManager from the UI thread because it is
protected by a lock.  It is always up to date with the most recently emitted
snapshot from the sycner, so it can contain information from an in-progress sync
cycle.

SyncSessionSnapshot snapshot: This snapshot is cached in the SyncBackendHost
when the syncer sends a SYNC_CYCLE_END event with !has_more_to_sync.  Such an
event is generated only when we exit the loop that invokes SyncShare(). 
Therefore, this snapshot may be a bit stale, though it should be updated just
before about:sync is told to refresh itself.  

I decided to use the snapshot to export the return values because:
- I wanted the values to be consistent with the "Last Sync Source".
- It did not require adding code to AllStatus or members to SyncManager::Status.
- I would like to eventually deprecate full_status.

[lipalani1, 2012/02/22 22:09:24]

That part is fine. But is there any value in caching this in all_status as well?
Would we be able to infer anything more based on these values?
On 2012/02/22 22:01:52, rlarocque wrote:

[rlarocque, 2012/02/22 22:22:37]

There would be some value, but I don't think it's worth it.

The only advantage to AllStatus/full_status is that it is more up to date than
the cached snapshot.  So, if for some reason the UI isn't getting pinged (ie. an
infinite loop or freeze in the syncer) we would be able to get slightly fresher
values from full_status.  

+          details, "Last Sync's GetUpdates Result",
+          GetSyncerErrorString(snapshot->errors.last_download_updates_result));
+      sync_ui_util::AddStringSyncDetails(
+          details, "Last Sync's Post Commit Result",
+          GetSyncerErrorString(snapshot->errors.last_post_commit_result));
+      sync_ui_util::AddStringSyncDetails(
+          details, "Last Sync's Process Commit Response Result",
+          GetSyncerErrorString(
+              snapshot->errors.last_process_commit_response_result));
 
       // Print the count of entries from snapshot. Warning: This might be
       // slightly out of date if there are client side changes that are yet