Skip ARC initial screen when everything is set up by policy

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator.h"
 
 #include <string>
 
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.h"
@@ skipping 63 matching lines @@
 }
 
 void ArcTermsOfServiceDefaultNegotiator::OnMetricsModeChanged(bool enabled,
                                                               bool managed) {
   support_host_->SetMetricsPreferenceCheckbox(enabled, managed);
 }
 
 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