[Chrome OS] Plumb new error codes from SignedSettings to consumers of the API

[Chrome OS] Plumb new error codes from SignedSettings to consumers of the API

Finish taking new SignedSettings::ReturnCode values all the way out to consumers of the SignedSettingsHelper API

BUG=chromium-os:9666
TEST=unit tests, install on device, ensure that Guest mode setting can be updated and is honored.

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

[Chris Masone, 2010-12-09 02:10:16]

On 2010/12/09 02:05:50, Chris Masone wrote:

I didn't put in any useful handling of the new error codes, but consumers of the
API should be able to distinguish between different failure modes now.  I also
changed the SignedSettings API a bit, enabling me to simplify the
SignedSettingsHelper code.

[Denis Lagno, 2010-12-09 11:54:06]

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/login/s...
File chrome/browser/chromeos/login/signed_settings_helper.cc (right):

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/login/s...
chrome/browser/chromeos/login/signed_settings_helper.cc:125: value = false;  //
Should be true already, but just to be sure.
for my eye comment disagrees with the code

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/login/s...
File chrome/browser/chromeos/login/signed_settings_helper.h (right):

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/login/s...
chrome/browser/chromeos/login/signed_settings_helper.h:25: bool found,
Do we really need this "bool found"?  ReturnCode cannot contain this info?

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/proxy_c...
File chrome/browser/chromeos/proxy_config_service_impl.cc (right):

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/proxy_c...
chrome/browser/chromeos/proxy_config_service_impl.cc:504: store_property_op_ =
NULL;
AFAIU you should reset store_property_op_ before calling PersistConfigToDevice,
not after.

[Chris Masone, 2010-12-09 16:33:42]

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/login/s...
File chrome/browser/chromeos/login/signed_settings_helper.cc (right):

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/login/s...
chrome/browser/chromeos/login/signed_settings_helper.cc:125: value = false;  //
Should be true already, but just to be sure.
On 2010/12/09 11:54:12, Denis Lagno wrote:
> for my eye comment disagrees with the code

again, you are right!

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/login/s...
File chrome/browser/chromeos/login/signed_settings_helper.h (right):

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/login/s...
chrome/browser/chromeos/login/signed_settings_helper.h:25: bool found,
On 2010/12/09 11:54:12, Denis Lagno wrote:
> Do we really need this "bool found"?  ReturnCode cannot contain this info?

You're probably right...SUCCESS + 'found == false' could just be NOT_FOUND.

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/proxy_c...
File chrome/browser/chromeos/proxy_config_service_impl.cc (right):

http://codereview.chromium.org/5671003/diff/1/chrome/browser/chromeos/proxy_c...
chrome/browser/chromeos/proxy_config_service_impl.cc:504: store_property_op_ =
NULL;
On 2010/12/09 11:54:12, Denis Lagno wrote:
> AFAIU you should reset store_property_op_ before calling
PersistConfigToDevice,
> not after.

That explains the bug I was seeing as I tested further!

[Denis Lagno, 2010-12-09 17:35:38]

LGTM

[xiyuan, 2010-12-09 17:55:38]

LGTM with one nit.

http://codereview.chromium.org/5671003/diff/8001/chrome/browser/chromeos/prox...
File chrome/browser/chromeos/proxy_config_service_impl.cc (right):

http://codereview.chromium.org/5671003/diff/8001/chrome/browser/chromeos/prox...
chrome/browser/chromeos/proxy_config_service_impl.cc:503:
PersistConfigToDevice();
nit: let's move this out of the "if" since we do this in both branches.