Skip ARC initial screen when everything is set up by policy

chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator.cc

Index: chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator.cc
diff --git a/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator.cc b/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator.cc
index ca0fe339fb59128b7a44c4f9c2dadb84d2084f20..ab3f358a4a40d8de1e6ea25a0ff6d45fea0da005 100644
--- a/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator.cc
+++ b/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator.cc
@@ -81,13 +81,23 @@ void ArcTermsOfServiceDefaultNegotiator::OnMetricsModeChanged(bool enabled,
 void ArcTermsOfServiceDefaultNegotiator::OnBackupAndRestoreModeChanged(
     bool enabled,
     bool managed) {
-  support_host_->SetBackupAndRestorePreferenceCheckbox(enabled, managed);
+  // Tick the checkbox by default in the unmanaged case, regardless of the pref

[hidehiko, 2017/02/15 04:59:16]

Ok, then, could you move them to arc_optin_preference_handler.cc instead?
Here, callbacks are called on initialization *or update*.
So, anytime even the preference is explicitly set to false, the checkbox is
set to true (= checked), if it is for unmanaged user.

You can see if the value is explicitly set or not by HasPref() in
ArcOptinPreferenceHandler.

[emaxx, 2017/02/15 05:12:31]

Hmm, but would it be more appropriate in ArcOptinPreferenceHandler? The
interface of this class seems to talk about actual prefs, and so is
ArcOptInPreferenceHandlerObserver. And it's probably not clean to "patch" the
passed pref value for the sole goal of having the desired UI effect (having the
checkbox checked by default).

Another option is to modify the ArcOptInPreferenceHandlerObserver interface to
include the HasPref bit of information, so that the observers may choose how to
react...

[hidehiko, 2017/02/15 14:38:43]

I think it is ok to modify ArcOptinPreferenceHandler directly.
Quote from the comment: "This helper encapsulates access to preferences"...
I think what you're trying to do is changing default value of the preference
based on managed state, which is not supported by Preference framework by
default.
So, in the above sense, it is not only for changing checkbox default value, but
also
changing default value for the preference (assuming all access to the preference
should pass through the class).

What I'm imaging is something like;
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/arc/optin/arc_op...

bool is_managed = pref_service->IsManagedPreference(...);

// If the preference is not set, switch the default value based on whether
// it is managed preference or not: "true" if unmanaged, otherwise "false".
bool value =
    pref_service_->HasPrefPath(kArcBackupRestoreEnabled) ?
    pref_service_->GetBoolean(...) :
    !is_managed;
observer_->OnBackupAndRestoreModeChanged(value, is_managed);

WDYT?

+  // value (which is false by default). In the managed case, follow the managed
+  // value.
+  const bool checkbox_checked = enabled || !managed;
+  support_host_->SetBackupAndRestorePreferenceCheckbox(checkbox_checked,
+                                                       managed);
 }
 
 void ArcTermsOfServiceDefaultNegotiator::OnLocationServicesModeChanged(
     bool enabled,
     bool managed) {
-  support_host_->SetLocationServicesPreferenceCheckbox(enabled, managed);
+  // Tick the checkbox by default in the unmanaged case, regardless of the pref
+  // value (which is false by default). In the managed case, follow the managed
+  // value.
+  const bool checkbox_checked = enabled || !managed;
+  support_host_->SetLocationServicesPreferenceCheckbox(checkbox_checked,
+                                                       managed);
 }
 
 }  // namespace arc