Stopping the history recording for a supervised user

chrome/browser/supervised_user/supervised_user_service_unittest.cc

Index: chrome/browser/supervised_user/supervised_user_service_unittest.cc
diff --git a/chrome/browser/supervised_user/supervised_user_service_unittest.cc b/chrome/browser/supervised_user/supervised_user_service_unittest.cc
index bc68d836c399d1ecf61b0b6d6cd4b9a048587eb8..9aa95d0a1051fca5b264496ccf7b7c6e4022e35d 100644
--- a/chrome/browser/supervised_user/supervised_user_service_unittest.cc
+++ b/chrome/browser/supervised_user/supervised_user_service_unittest.cc
@@ -16,8 +16,11 @@
 #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"
@@ -154,6 +157,25 @@ TEST_F(SupervisedUserServiceTest, GetManualExceptionsForHost) {
             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) {