Shade uninitialized about:sync fields

Shade uninitialized about:sync fields

This change includes the UI and C++ changes required to highlight
uninitialized fields in about:sync.

On the UI side, the sync details now include an 'is_valid' field which
drives some logic in jstemplate and CSS that will change the gray out
the text for uninitialized values.

The is_valid field is populated in sync_ui_util.  The Add*SyncDetails()
functions now take an additional 'is_valid' parameter, which indicates
whether or not the value is believed to have originated from within
sync, rather than some default constructor.

The signature for ProfileSyncService::QueryDetailedSyncStatus() has been
modified to return an additional parameter indicating whether or not the
returned status actually came from the backend.  This is used to
populate the 'is_valid' field for the many about:sync fields whose
values originate from this function.

The SyncSesionSnapshot takes a different approach.  The object is
immutable and has two constructors.  If it is constructed with the
zero-argument constructor it is clearly uninitialized, and will remain
so.  This change adds a flag which is set or unset based on which
constructor is used.  This flag is used to populte the 'is_valid' field
for about:sync values populated from the snapshot.

BUG=134927
TEST=

[rlarocque, 2012-06-30 00:27:37]

I don't think this change solves *all* about:sync problems related to
uninitialized data.  However, it does provide an infrastructure and a decent
first guess that we can build on.

Please review.

[akalin, 2012-07-02 21:18:25]

I thought about it and I'm not a big fan of the 'is_valid' flag. :/ It seems to
me it's not needed roughly half the time and so adds some clutter.

What do you think of an approach that involves 'registering' an about:sync entry
with a default value (similar to the PrefService)?  Then, you can keep track of
whether a particular entry is set by whether or not it's explicitly set.  That
is, instead of:

bool is_snapshot_valid = ...;

AddBoolValue(snapshot.some_bool_value, is_snapshot_valid);

you'd have:

if (is_snapshot_valid) {
  AddBoolValue(....);
  ...
} else {
  // Do nothing; default values will be displayed (and greyed out)
}

Of course, that seems like a significant change, so I'm okay with this being a
stepping stone to that (if you agree).

If you think the is_valid flag is the way to go for now, I'd like it not to be a
bool, especially since we're setting bool values also.  How about changing it to
an { UNINITIALIZED, INITIALIZED } or { DEFAULT, NON_DEFAULT } enum or something?

http://codereview.chromium.org/10694013/diff/6016/sync/internal_api/public/se...
File sync/internal_api/public/sessions/sync_session_snapshot.h (right):

http://codereview.chromium.org/10694013/diff/6016/sync/internal_api/public/se...
sync/internal_api/public/sessions/sync_session_snapshot.h:75: // Set iff this
snapshot was built using the default constructor.
don't you mean 'not built'?

[rlarocque, 2012-07-02 21:50:26]

Pre-registering fields?  That's an interesting idea.

I've been moving in precisely the opposite direction lately, but that was mainly
in an attempt to de-couple the HTML from the C++ code.  I never thought of
declaring the structure in C++.  

I'll try implementing this to see how big and intrusive the diff would be.

http://codereview.chromium.org/10694013/diff/6016/sync/internal_api/public/se...
File sync/internal_api/public/sessions/sync_session_snapshot.h (right):

http://codereview.chromium.org/10694013/diff/6016/sync/internal_api/public/se...
sync/internal_api/public/sessions/sync_session_snapshot.h:75: // Set iff this
snapshot was built using the default constructor.
On 2012/07/02 21:18:25, akalin wrote:
> don't you mean 'not built'?

Correct.  I negated the logic of this flag at one point, I must have forgot to
update the comment.

[akalin, 2012-07-20 00:22:25]

This is obsolete, right?

[rlarocque, 2012-07-20 00:35:27]

On 2012/07/20 00:22:25, akalin wrote:
> This is obsolete, right?

Yes.  I'll delete it.