Export syncer return values to about:sync

Export syncer return values to about:sync

This shows the results of the most recent attempts to download updates
and commit items on the main about:sync page.

It improves visibility into the SyncScheduler by exposing some of the
most important inputs to scheduling decisions.  

It should help us remove some of the stranger
AllStatus/ServerConnectionManager state variables by making them
redundant.

BUG=112040, 110954
TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=123350

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

Please review.

http://codereview.chromium.org/9429064/diff/2002/chrome/browser/sync/sync_ui_...
File chrome/browser/sync/sync_ui_util.cc (right):

http://codereview.chromium.org/9429064/diff/2002/chrome/browser/sync/sync_ui_...
chrome/browser/sync/sync_ui_util.cc:652: sync_ui_util::AddStringSyncDetails(
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]

http://codereview.chromium.org/9429064/diff/2002/chrome/browser/sync/sync_ui_...
File chrome/browser/sync/sync_ui_util.cc (right):

http://codereview.chromium.org/9429064/diff/2002/chrome/browser/sync/sync_ui_...
chrome/browser/sync/sync_ui_util.cc:652: sync_ui_util::AddStringSyncDetails(
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:
> 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.

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

http://codereview.chromium.org/9429064/diff/2002/chrome/browser/sync/sync_ui_...
File chrome/browser/sync/sync_ui_util.cc (right):

http://codereview.chromium.org/9429064/diff/2002/chrome/browser/sync/sync_ui_...
chrome/browser/sync/sync_ui_util.cc:652: sync_ui_util::AddStringSyncDetails(
On 2012/02/22 22:09:24, lipalani1 wrote:
> That part is fine. But is there any value in caching this in all_status as
well?

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.

[lipalani1, 2012-02-22 22:25:38]

LGTM.

[commit-bot: I haz the power, 2012-02-22 22:33:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/9429064/2002

[commit-bot: I haz the power, 2012-02-23 00:17:19]

Try job failure for 9429064-2002 (retry) (retry) on linux_rel for step
"ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-02-23 00:23:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/9429064/2002

[commit-bot: I haz the power, 2012-02-23 05:53:35]

Try job failure for 9429064-2002 (retry) (retry) on win_rel for step "ui_tests".
It's a second try, previously, steps "browser_tests, ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-02-23 19:40:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/9429064/2002

[commit-bot: I haz the power, 2012-02-23 22:32:32]

Change committed as 123350