Disable/enable echo for enterprise device.

chrome/browser/chromeos/extensions/echo_private_api.cc

Index: chrome/browser/chromeos/extensions/echo_private_api.cc
diff --git a/chrome/browser/chromeos/extensions/echo_private_api.cc b/chrome/browser/chromeos/extensions/echo_private_api.cc
index 51f195dbf4e0e44638fb24e52fb98891c716a1bf..79564fa1678512b2a50dd6a0332a7c73b2f8873f 100644
--- a/chrome/browser/chromeos/extensions/echo_private_api.cc
+++ b/chrome/browser/chromeos/extensions/echo_private_api.cc
@@ -14,6 +14,7 @@
 #include "base/time.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/system/statistics_provider.h"
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/browser_thread.h"
@@ -98,3 +99,21 @@ bool GetOobeTimestampFunction::GetOobeTimestampOnFileThread() {
   SetResult(new base::StringValue(timestamp));
   return true;
 }
+
+IsEchoDisabledFunction::IsEchoDisabledFunction() {
+}
+
+IsEchoDisabledFunction::~IsEchoDisabledFunction() {
+}
+
+// Check the enterprise policy kChromeOsRegistrationEnabled flag value.
+// This policy is used to enable/disable Echo for enterprise device.
+bool IsEchoDisabledFunction::RunImpl() {
+  bool isEnabled = true;
+  // If the policy is not provided, isEnabled will not be modified by
+  // GetBoolean.

[oscarpan, 2013/02/02 01:50:56]

I assume this comment is correct and seems so from my manual test, but reviewers
please convince me. Thanks a lot!

[xiyuan, 2013/02/02 07:49:27]

I think the comment is correct. If not confortable, you can use return value of
GetBoolean to set the fallback value.

[Mattias Nissler (ping if slow), 2013/02/04 09:06:19]

It should be you convincing the reviewers ;) In these cases, please make the
effort to check the source whether the assumption is justified.

+  chromeos::CrosSettings::Get()->GetBoolean(
+    chromeos::kChromeOsRegistrationEnabled, &isEnabled);

[xiyuan, 2013/02/02 07:49:27]

nit: 4-space indent

[xiyuan, 2013/02/02 07:49:27]

Do we need to worry that this value might not be trusted?

[oscarpan, 2013/02/02 21:52:35]

I'm not sure when this value is unreliable. Is there any way a user can modify
the policy value? This reminds me another thing, right now I create this
separate method to check whether Echo is disabled. But if a user can modify the
extension code, he can still bypass this check and claim offers using Echo. Is
that even possible to modify private extension? If so, it seems safer to remove
this new api and add the check inside GetRegistrationCodeFunction, and never
return the registration code if Echo is disabled.

[xiyuan, 2013/02/02 22:44:16]

I am not worrying about extension code being changed. It's more about the value
returned from GetBoolean call. I have an impression that the settings values are
cached locally somewhere to speed things up. The cached values are not safe
because user might temper with it. If this is a problem, then you need to call
CrsoSettings::PrepareTrustedValues to ensure GetBoolean will return a trusted
value (value that passed signature check etc). 

It's quite simple to use. You simple add a PrepareTrustedValues call before
you call GetBoolean and put your function as the callback when trusted values
are available. Of course, if you do this, you might need to change it to be an
AsyncExtensionFunction.

One example could be found here:
https://code.google.com/p/chromium/codesearch#chrome/src/chrome/browser/polic...

[oscarpan, 2013/02/05 00:17:47]

Done.

[oscarpan, 2013/02/05 00:17:47]

How about we postpone this feature to later cls.

+  SetResult(new base::FundamentalValue(!isEnabled));
+  return true;
+}