Skip ARC initial screen when everything is set up by policy

chrome/browser/chromeos/arc/arc_session_manager.cc

Index: chrome/browser/chromeos/arc/arc_session_manager.cc
diff --git a/chrome/browser/chromeos/arc/arc_session_manager.cc b/chrome/browser/chromeos/arc/arc_session_manager.cc
index a1a050fc65aaa9f5e30e744166c2c3e6cf8d87f6..9e6186ab36fedab8a86134aa3683eb3c3dca0511 100644
--- a/chrome/browser/chromeos/arc/arc_session_manager.cc
+++ b/chrome/browser/chromeos/arc/arc_session_manager.cc
@@ -504,6 +504,8 @@ void ArcSessionManager::OnOptInPreferenceChanged() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile_);
 
+  PrefService* const prefs = profile_->GetPrefs();
+
   const bool arc_enabled = IsArcEnabled();
   if (!IsArcManaged()) {
     // Update UMA only for non-Managed cases.
@@ -525,8 +527,7 @@ void ArcSessionManager::OnOptInPreferenceChanged() {
 
   // Hide auth notification if it was opened before and arc.enabled pref was
   // explicitly set to true or false.
-  if (!g_disable_ui_for_testing &&
-      profile_->GetPrefs()->HasPrefPath(prefs::kArcEnabled)) {
+  if (!g_disable_ui_for_testing && prefs->HasPrefPath(prefs::kArcEnabled)) {

[hidehiko, 2017/02/09 15:28:04]

Style: Could you elide the brace for one-line if-body case?

[emaxx, 2017/02/10 15:40:15]

Done.

     ArcAuthNotification::Hide();
   }
 
@@ -556,14 +557,21 @@ void ArcSessionManager::OnOptInPreferenceChanged() {
   // there will be no one who is eligible to accept them.
   // TODO(poromov): Move to more Kiosk dedicated set-up phase.
   if (IsArcKioskMode())
-    profile_->GetPrefs()->SetBoolean(prefs::kArcTermsAccepted, true);
+    prefs->SetBoolean(prefs::kArcTermsAccepted, true);
+
+  // Skip ToS when all displayed preferences are managed by the admin policy.
+  if (IsArcManaged() &&
+      prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled) &&
+      prefs->IsManagedPreference(prefs::kArcLocationServiceEnabled)) {
+    prefs->SetBoolean(prefs::kArcTermsAccepted, true);

[hidehiko, 2017/02/09 15:28:04]

How about:

// Skip to show UI asking users to enable/disable their preference for
backup-restore and location-service, if both are managed by the admin policy.
// Note that, in managed case, ToS agreement is already taken in ${where it is},
so it is ok to mark arc.terms.accepted.

BTW, is there any case that arc.enabled pref is changed from managed to
unmanaged at some point while ARC is running? If yes, what's the expected
behavior in the case?

[emaxx, 2017/02/10 15:40:15]

Done, with a slight modification of the second sentence, because the actual ToS
text is already hidden in the managed ARC case regardless of this CL.
PTAL.

+  }
 
   // If it is marked that sign in has been successfully done, then directly
   // start ARC.
   // For testing, and for Kisok mode, we also skip ToS negotiation procedure.
   // For backward compatibility, this check needs to be prior to the
   // kArcTermsAccepted check below.
-  if (profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn) ||
+  if (prefs->GetBoolean(prefs::kArcSignedIn) ||
       IsArcOptInVerificationDisabled() || IsArcKioskMode()) {
     StartArc();
 
@@ -597,10 +605,12 @@ void ArcSessionManager::OnOptInPreferenceChanged() {
   // 1) User accepted the Terms of service on OOBE flow.
   // 2) User accepted the Terms of service on Opt-in flow, but logged out
   //   before ARC sign in procedure was done. Then, logs in again.
-  if (profile_->GetPrefs()->GetBoolean(prefs::kArcTermsAccepted)) {
+  if (prefs->GetBoolean(prefs::kArcTermsAccepted)) {
     // Don't show UI for this progress if it was not shown.
-    if (support_host_->ui_page() != ArcSupportHost::UIPage::NO_PAGE)
+    if (support_host_ &&

[hidehiko, 2017/02/09 15:28:04]

Good catch! Thank you for fix.

[emaxx, 2017/02/10 15:40:15]

Acknowledged.

+        support_host_->ui_page() != ArcSupportHost::UIPage::NO_PAGE) {
       support_host_->ShowArcLoading();
+    }
     StartArcAndroidManagementCheck();
     return;
   }