Stopping the history recording for a supervised user

chrome/browser/supervised_user/supervised_user_service_unittest.cc

 // Copyright 2014 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 "base/path_service.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_service_test_base.h"
 #include "chrome/browser/extensions/unpacked_installer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/fake_profile_oauth2_token_service.h"
 #include "chrome/browser/signin/fake_profile_oauth2_token_service_builder.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/supervised_user/custodian_profile_downloader_service.h"
 #include "chrome/browser/supervised_user/custodian_profile_downloader_service_factory.h"
+#include "chrome/browser/supervised_user/supervised_user_constants.h"
 #include "chrome/browser/supervised_user/supervised_user_service.h"
 #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/features/feature_channel.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
 #include "extensions/common/manifest_constants.h"
@@ skipping 116 matching lines @@
             supervised_user_service_->GetManualBehaviorForURL(kExampleBarURL));
   EXPECT_EQ(SupervisedUserService::MANUAL_ALLOW,
             supervised_user_service_->GetManualBehaviorForURL(
                 kExampleFooNoWWWURL));
   EXPECT_EQ(SupervisedUserService::MANUAL_ALLOW,
             supervised_user_service_->GetManualBehaviorForURL(kBlurpURL));
   EXPECT_EQ(SupervisedUserService::MANUAL_NONE,
             supervised_user_service_->GetManualBehaviorForURL(kMooseURL));
 }
 
+TEST_F(SupervisedUserServiceTest, ChangesSessionSyncOnChangedSettings) {
+  DictionaryPrefUpdate update(profile_->GetPrefs(),

[Marc Treib, 2014/08/19 14:00:28]

What does this do? Do you actually need the DictionaryPrefUpdate?

+                              prefs::kSupervisedUserManualURLs);
+  syncer::ModelTypeSet active_data_types =
+      ProfileSyncServiceFactory::GetForProfile(
+          supervised_user_service_->profile_)->GetActiveDataTypes();

[Marc Treib, 2014/08/19 14:06:29]

Is the PSS actually properly set up in a unit test? If so, also check that
GetActiveDataTypes does not contain SESSIONS anymore after you set RecordHistory
to false.

+  base::DictionaryValue* dict = update.Get();

[Marc Treib, 2014/08/19 14:00:28]

Please move this below the EXPECTs, right before it is actually applied.

+  dict->SetBooleanWithoutPathExpansion(supervised_users::kRecordHistory, false);
+  EXPECT_TRUE(supervised_user_service_->MaySyncSessions());
+  EXPECT_TRUE(active_data_types.Has(syncer::SESSIONS));
+
+  supervised_user_service_->OnNewSettingsAvailable(dict);
+  EXPECT_FALSE(supervised_user_service_->MaySyncSessions());
+  dict->SetBooleanWithoutPathExpansion(supervised_users::kRecordHistory, true);
+  bool changed_state = supervised_user_service_->FetchNewSessionSyncState(dict);
+
+  ASSERT_TRUE(changed_state);

[Marc Treib, 2014/08/19 14:00:28]

Why is this not an EXPECT?
Also, maybe just inline the check into the EXPECT and remove the local var?

+}
+
 // Ensure that the CustodianProfileDownloaderService shuts down cleanly. If no
 // DCHECK is hit when the service is destroyed, this test passed.
 TEST_F(SupervisedUserServiceTest, ShutDownCustodianProfileDownloader) {
   CustodianProfileDownloaderService* downloader_service =
       CustodianProfileDownloaderServiceFactory::GetForProfile(profile_.get());
 
   // Emulate being logged in, then start to download a profile so a
   // ProfileDownloader gets created.
   profile_->GetPrefs()->SetString(prefs::kGoogleServicesUsername, "Logged In");
   downloader_service->DownloadProfile(base::Bind(&OnProfileDownloadedFail));
@@ skipping 247 matching lines @@
   site_lists[0]->GetSites(&sites);
   ASSERT_EQ(1u, sites.size());
   EXPECT_EQ(base::ASCIIToUTF16("Moose"), sites[0].name);
 
   EXPECT_EQ(SupervisedUserURLFilter::WARN,
             url_filter->GetFilteringBehaviorForURL(example_url));
   EXPECT_EQ(SupervisedUserURLFilter::ALLOW,
             url_filter->GetFilteringBehaviorForURL(moose_url));
 }
 #endif  // !defined(OS_ANDROID)