Skip ARC initial screen when everything is set up by policy

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

Index: chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc
diff --git a/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc b/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc
index ce080c8dfbab8fe7e4c24bf783f3d47c00da0be9..018937de4e3411a43e21b0ec1c5ac7b5c5fdb83f 100644
--- a/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc
+++ b/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
+#include "base/values.h"
 #include "chrome/browser/chromeos/arc/arc_support_host.h"
 #include "chrome/browser/chromeos/arc/extensions/fake_arc_support.h"
 #include "chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator.h"
@@ -18,13 +19,14 @@
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/prefs/pref_service.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace arc {
 
 class ArcTermsOfServiceDefaultNegotiatorTest : public testing::Test {
- public:
+ protected:

[hidehiko, 2017/02/14 11:51:36]

nit: could you keep public?

[emaxx, 2017/02/15 03:40:59]

Done. (Although I thought it's more common in the Chrome code base to use
"protected" in such cases.)

   ArcTermsOfServiceDefaultNegotiatorTest() = default;
   ~ArcTermsOfServiceDefaultNegotiatorTest() override = default;
 
@@ -34,8 +36,6 @@ class ArcTermsOfServiceDefaultNegotiatorTest : public testing::Test {
             new chromeos::FakeChromeUserManager());
 
     profile_ = base::MakeUnique<TestingProfile>();
-    profile_->GetPrefs()->SetBoolean(prefs::kArcBackupRestoreEnabled, false);
-    profile_->GetPrefs()->SetBoolean(prefs::kArcLocationServiceEnabled, false);
 
     support_host_ = base::MakeUnique<ArcSupportHost>(profile_.get());
     fake_arc_support_ = base::MakeUnique<FakeArcSupport>(support_host_.get());
@@ -51,7 +51,7 @@ class ArcTermsOfServiceDefaultNegotiatorTest : public testing::Test {
     user_manager_enabler_.reset();
   }
 
-  Profile* profile() { return profile_.get(); }
+  TestingProfile* profile() { return profile_.get(); }
   ArcSupportHost* support_host() { return support_host_.get(); }
   FakeArcSupport* fake_arc_support() { return fake_arc_support_.get(); }
   ArcTermsOfServiceNegotiator* negotiator() { return negotiator_.get(); }
@@ -112,10 +112,34 @@ TEST_F(ArcTermsOfServiceDefaultNegotiatorTest, Accept) {
   EXPECT_EQ(status, Status::PENDING);
   EXPECT_EQ(fake_arc_support()->ui_page(), ArcSupportHost::UIPage::TERMS);
 
-  // Check the preference related checkbox.
-  fake_arc_support()->set_metrics_mode(true);
-  fake_arc_support()->set_backup_and_restore_mode(true);
-  fake_arc_support()->set_location_service_mode(true);
+  // By default, the preference related checkboxes are checked, despite that
+  // the preferences default to false.
+  EXPECT_FALSE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcBackupRestoreEnabled));
+  EXPECT_FALSE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcLocationServiceEnabled));
+  EXPECT_TRUE(fake_arc_support()->backup_and_restore_mode());
+  EXPECT_TRUE(fake_arc_support()->location_service_mode());
+
+  // The preferences are assigned to the managed false value, and the
+  // corresponding checkboxes are unchecked.
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcBackupRestoreEnabled,
+      base::MakeUnique<base::Value>(false).release());

[hidehiko, 2017/02/14 11:51:36]

nit: new base::Value(false)

BTW, according to the comments of SetManagedPref, it takes the ownership of the
Value. Can it be changed to std::unique_ptr<base::Value> (maybe in a separate
CL?).

[emaxx, 2017/02/15 03:40:59]

Done.
Created crbug.com/692121 to track that refactoring.

+  EXPECT_FALSE(fake_arc_support()->backup_and_restore_mode());
+  profile()->GetTestingPrefService()->SetManagedPref(
+      prefs::kArcLocationServiceEnabled,
+      base::MakeUnique<base::Value>(false).release());
+  EXPECT_FALSE(fake_arc_support()->location_service_mode());
+
+  // The managed preference values are removed, and the corresponding checkboxes
+  // are checked again.
+  profile()->GetTestingPrefService()->RemoveManagedPref(
+      prefs::kArcBackupRestoreEnabled);
+  EXPECT_TRUE(fake_arc_support()->backup_and_restore_mode());
+  profile()->GetTestingPrefService()->RemoveManagedPref(
+      prefs::kArcLocationServiceEnabled);
+  EXPECT_TRUE(fake_arc_support()->location_service_mode());
 
   // Make sure preference values are not yet updated.
   EXPECT_FALSE(
@@ -135,6 +159,41 @@ TEST_F(ArcTermsOfServiceDefaultNegotiatorTest, Accept) {
       profile()->GetPrefs()->GetBoolean(prefs::kArcLocationServiceEnabled));
 }
 
+TEST_F(ArcTermsOfServiceDefaultNegotiatorTest, AcceptWithUnchecked) {
+  // Show Terms of service page.
+  Status status = Status::PENDING;
+  negotiator()->StartNegotiation(UpdateStatusCallback(&status));
+
+  // TERMS page should be shown.
+  EXPECT_EQ(status, Status::PENDING);
+  EXPECT_EQ(fake_arc_support()->ui_page(), ArcSupportHost::UIPage::TERMS);
+
+  // Override the preferences from the default values to true.
+  profile()->GetPrefs()->SetBoolean(prefs::kArcBackupRestoreEnabled, true);
+  profile()->GetPrefs()->SetBoolean(prefs::kArcLocationServiceEnabled, true);
+
+  // Uncheck the preference related checkboxes.
+  fake_arc_support()->set_backup_and_restore_mode(false);
+  fake_arc_support()->set_location_service_mode(false);
+
+  // Make sure preference values are not yet updated.
+  EXPECT_TRUE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcBackupRestoreEnabled));
+  EXPECT_TRUE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcLocationServiceEnabled));
+
+  // Click the "AGREE" button so that the callback should be invoked
+  // with |agreed| = true.
+  fake_arc_support()->ClickAgreeButton();
+  EXPECT_EQ(status, Status::ACCEPTED);
+
+  // Make sure preference values are now updated.
+  EXPECT_FALSE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcBackupRestoreEnabled));
+  EXPECT_FALSE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcLocationServiceEnabled));
+}
+
 TEST_F(ArcTermsOfServiceDefaultNegotiatorTest, Cancel) {
   // Show Terms of service page.
   Status status = Status::PENDING;