Skip ARC initial screen when everything is set up by policy

chrome/browser/chromeos/arc/arc_session_manager.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/arc_session_manager.h"
 
 #include <utility>
 
 #include "ash/common/shelf/shelf_delegate.h"
 #include "ash/common/wm_shell.h"
@@ skipping 486 matching lines @@
   }
   ShutdownSession();
   if (support_host_)
     support_host_->Close();
 }
 
 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.
     UpdateOptInActionUMA(arc_enabled ? OptInActionType::OPTED_IN
                                      : OptInActionType::OPTED_OUT);
 
     if (!arc_enabled) {
       // Remove the pinned Play Store icon launcher in Shelf.
       // This is only for non-Managed cases. In managed cases, it is expected
       // to be "disabled" rather than "removed", so keep it here.
       ash::ShelfDelegate* shelf_delegate = GetShelfDelegate();
       if (shelf_delegate)
         shelf_delegate->UnpinAppWithID(ArcSupportHost::kHostAppId);
     }
   }
 
   for (auto& observer : observer_list_)
     observer.OnArcOptInChanged(arc_enabled);
 
   // 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();
   }
 
   if (!arc_enabled) {
     // Reset any pending request to re-enable ARC.
     VLOG(1) << "ARC opt-out. Removing user data.";
     reenable_arc_ = false;
     StopArc();
     RemoveArcData();
     return;
   }
 
   if (state_ == State::ACTIVE)
     return;
 
   if (state_ == State::REMOVING_DATA_DIR) {
     // Data removal request is in progress. Set flag to re-enable Arc once it is
     // finished.
     reenable_arc_ = true;
     return;
   }
 
   if (support_host_)
     support_host_->SetArcManaged(IsArcManaged());
 
   // For ARC Kiosk we skip ToS because it is very likely that near the device
   // 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();
 
     // Skip Android management check for testing.
     // We also skip if Android management check for Kiosk mode,
     // because there are no managed human users for Kiosk exist.
     if (IsArcOptInVerificationDisabled() || IsArcKioskMode() ||
         (g_disable_ui_for_testing &&
          !g_enable_check_android_management_for_testing)) {
       return;
@@ skipping 13 matching lines @@
     return;
   }
 
   // If it is marked that the Terms of service is accepted already,
   // just skip the negotiation with user, and start Android management
   // check directly.
   // This happens, e.g., when;
   // 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;
   }
 
   // Need user's explicit Terms Of Service agreement. Prevent race condition
   // when ARC can be enabled before profile is synced. In last case
   // OnOptInPreferenceChanged is called twice.
   // TODO(crbug.com/687185): Remove the condition.
   if (state_ != State::SHOWING_TERMS_OF_SERVICE)
     StartTermsOfServiceNegotiation();
@@ skipping 359 matching lines @@
 
 #undef MAP_STATE
 
   // Some compilers report an error even if all values of an enum-class are
   // covered exhaustively in a switch statement.
   NOTREACHED() << "Invalid value " << static_cast<int>(state);
   return os;
 }
 
 }  // namespace arc