Show ARC OptIn dialog if B&R or GLS enabled

Update the logic to decide if ARC OptIn dialog is shown for managed
users to only skip it if both ArcBackupRestoreEnabled and
ArcLocationServiceEnabled are managed and false.

BUG=b:73339871
TEST=unit_tests, Opt in to ARC++ with Managed User

Change-Id: Ibf5a513f0559f511ef386042da7a227af9d4af89
Reviewed-on: https://chromium-review.googlesource.com/963003
Commit-Queue: Josh Horwich <jhorwich@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543268}

chrome/browser/chromeos/arc/arc_session_manager_unittest.cc

@@ -779,13 +779,17 @@ TEST_P(ArcSessionManagerPolicyTest, SkippingTerms) {
   arc_session_manager()->RequestEnable();
 
   // Terms of Service are skipped iff ARC is enabled by policy and both policies
-  // are either managed or unused (for Active Directory users a LaForge account
-  // is created, not a full Dasher account, where the policies have no meaning).
-  const bool prefs_managed = backup_restore_pref_value().is_bool() &&
-                             location_service_pref_value().is_bool();
+  // are either managed/OFF or unused (for Active Directory users a LaForge
+  // account is created, not a full Dasher account, where the policies have no
+  // meaning).
   const bool prefs_unused = is_active_directory_user();
+  const bool backup_managed_off = backup_restore_pref_value().is_bool() &&
+                                  !backup_restore_pref_value().GetBool();
+  const bool location_managed_off = location_service_pref_value().is_bool() &&
+                                    !location_service_pref_value().GetBool();
   const bool expected_terms_skipping =
-      (arc_enabled_pref_managed() && (prefs_managed || prefs_unused));
+      (arc_enabled_pref_managed() &&
+       ((backup_managed_off && location_managed_off) || prefs_unused));
   EXPECT_EQ(expected_terms_skipping
                 ? ArcSessionManager::State::CHECKING_ANDROID_MANAGEMENT
                 : ArcSessionManager::State::NEGOTIATING_TERMS_OF_SERVICE,

chrome/browser/chromeos/arc/arc_util.cc

@@ -388,14 +388,19 @@ bool AreArcAllOptInPreferencesIgnorableForProfile(const Profile* profile) {
   if (IsActiveDirectoryUserForProfile(profile))
     return true;
 
-  if (profile->GetPrefs()->IsManagedPreference(
-          prefs::kArcBackupRestoreEnabled) &&
-      profile->GetPrefs()->IsManagedPreference(
-          prefs::kArcLocationServiceEnabled)) {
-    return true;
+  // Otherwise, the preferences are ignorable iff both backup&restore and
+  // location services are set off by policy.
+  const PrefService* prefs = profile->GetPrefs();
+  if (!prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled) ||
+      prefs->GetBoolean(prefs::kArcBackupRestoreEnabled) == true) {
+    return false;
+  }
+  if (!prefs->IsManagedPreference(prefs::kArcLocationServiceEnabled) ||
+      prefs->GetBoolean(prefs::kArcLocationServiceEnabled) == true) {
+    return false;
   }
 
-  return false;
+  return true;
 }
 
 bool IsActiveDirectoryUserForProfile(const Profile* profile) {

chrome/browser/chromeos/arc/arc_util_unittest.cc

@@ -619,28 +619,52 @@ TEST_F(ChromeArcUtilTest, AreArcAllOptInPreferencesIgnorableForProfile) {
     EXPECT_TRUE(AreArcAllOptInPreferencesIgnorableForProfile(profile()));
   }
 
-  // OptIn prefs are set to unmanaged values, and the function returns false.
+  // OptIn prefs are set to unmanaged/OFF values, and the function returns
+  // false.
   profile()->GetPrefs()->SetBoolean(prefs::kArcBackupRestoreEnabled, false);
   profile()->GetPrefs()->SetBoolean(prefs::kArcLocationServiceEnabled, false);
   EXPECT_FALSE(AreArcAllOptInPreferencesIgnorableForProfile(profile()));
 
-  // Backup-restore pref is managed, while location-service is not, and the
-  // function returns false.
+  // OptIn prefs are set to unmanaged/ON values, and the function returns false.
+  profile()->GetPrefs()->SetBoolean(prefs::kArcBackupRestoreEnabled, true);
+  profile()->GetPrefs()->SetBoolean(prefs::kArcLocationServiceEnabled, true);
+  EXPECT_FALSE(AreArcAllOptInPreferencesIgnorableForProfile(profile()));
+
+  // Backup-restore pref is managed/OFF, while location-service is unmanaged,
+  // and the function returns false.
   profile()->GetTestingPrefService()->SetManagedPref(
       prefs::kArcBackupRestoreEnabled, std::make_unique<base::Value>(false));
+  profile()->GetPrefs()->SetBoolean(prefs::kArcLocationServiceEnabled, false);
   EXPECT_FALSE(AreArcAllOptInPreferencesIgnorableForProfile(profile()));
 
-  // Location-service pref is managed, while backup-restore is not, and the
-  // function returns false.
+  // Location-service pref is managed/OFF, while backup-restore is unmanaged,
+  // and the function returns false.
   profile()->GetTestingPrefService()->RemoveManagedPref(
       prefs::kArcBackupRestoreEnabled);
   profile()->GetTestingPrefService()->SetManagedPref(
       prefs::kArcLocationServiceEnabled, std::make_unique<base::Value>(false));
   EXPECT_FALSE(AreArcAllOptInPreferencesIgnorableForProfile(profile()));
 
-  // Both OptIn prefs are set to managed values, and the function returns true.
+  // Backup-restore pref is set to managed/ON, and the function returns false.
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcBackupRestoreEnabled, std::make_unique<base::Value>(true));
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcLocationServiceEnabled, std::make_unique<base::Value>(false));
+  EXPECT_FALSE(AreArcAllOptInPreferencesIgnorableForProfile(profile()));
+
+  // Location-service pref is set to managed/ON, and the function returns false.
   profile()->GetTestingPrefService()->SetManagedPref(
       prefs::kArcBackupRestoreEnabled, std::make_unique<base::Value>(false));
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcLocationServiceEnabled, std::make_unique<base::Value>(true));
+  EXPECT_FALSE(AreArcAllOptInPreferencesIgnorableForProfile(profile()));
+
+  // Both OptIn prefs are set to managed/OFF values, and the function returns
+  // true.
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcBackupRestoreEnabled, std::make_unique<base::Value>(false));
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcLocationServiceEnabled, std::make_unique<base::Value>(false));
   EXPECT_TRUE(AreArcAllOptInPreferencesIgnorableForProfile(profile()));
 }
 
@@ -694,6 +718,24 @@ TEST_F(ChromeArcUtilTest, TermsOfServiceNegotiationNeededForManagedUser) {
       prefs::kArcLocationServiceEnabled, std::make_unique<base::Value>(false));
   EXPECT_FALSE(IsArcTermsOfServiceNegotiationNeeded(profile()));
   EXPECT_FALSE(IsArcTermsOfServiceOobeNegotiationNeeded());
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcBackupRestoreEnabled, std::make_unique<base::Value>(true));
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcLocationServiceEnabled, std::make_unique<base::Value>(false));
+  EXPECT_TRUE(IsArcTermsOfServiceNegotiationNeeded(profile()));
+  EXPECT_TRUE(IsArcTermsOfServiceOobeNegotiationNeeded());
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcBackupRestoreEnabled, std::make_unique<base::Value>(false));
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcLocationServiceEnabled, std::make_unique<base::Value>(true));
+  EXPECT_TRUE(IsArcTermsOfServiceNegotiationNeeded(profile()));
+  EXPECT_TRUE(IsArcTermsOfServiceOobeNegotiationNeeded());
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcBackupRestoreEnabled, std::make_unique<base::Value>(true));
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcLocationServiceEnabled, std::make_unique<base::Value>(true));
+  EXPECT_TRUE(IsArcTermsOfServiceNegotiationNeeded(profile()));
+  EXPECT_TRUE(IsArcTermsOfServiceOobeNegotiationNeeded());
 }
 
 TEST_F(ChromeArcUtilTest, TermsOfServiceOobeNegotiationNeededNoLogin) {