Stopping the history recording for a supervised user

chrome/browser/supervised_user/supervised_user_service.h

Index: chrome/browser/supervised_user/supervised_user_service.h
diff --git a/chrome/browser/supervised_user/supervised_user_service.h b/chrome/browser/supervised_user/supervised_user_service.h
index 7bbdfedefab7adc3b972c779ed10fd6ed1ed7fbe..900f910c6e47d33d466f14a327a317ddba6e5deb 100644
--- a/chrome/browser/supervised_user/supervised_user_service.h
+++ b/chrome/browser/supervised_user/supervised_user_service.h
@@ -187,7 +187,10 @@ class SupervisedUserService : public KeyedService,
   friend class SupervisedUserServiceExtensionTestBase;
   friend class SupervisedUserServiceFactory;
   FRIEND_TEST_ALL_PREFIXES(SupervisedUserServiceTest, ClearOmitOnRegistration);
-
+  FRIEND_TEST_ALL_PREFIXES(SupervisedUserServiceTest,
+                           ChangesIncludedSessionOnChangedSettings);
+  FRIEND_TEST_ALL_PREFIXES(SupervisedUserServiceTest,
+                           ChangesSyncSessionStateOnChangedSettings);
   // A bridge from the UI thread to the SupervisedUserURLFilters, one of which
   // lives on the IO thread. This class mediates access to them and makes sure
   // they are kept in sync.
@@ -272,6 +275,28 @@ class SupervisedUserService : public KeyedService,
   // Returns the human readable name of the supervised user.
   std::string GetSupervisedUserName() const;
 
+  // Gets attached to the SupervisedUserSettingsService and writes into
+  // includes_session_sync_ if the history may be recorded.

[Marc Treib, 2014/08/19 15:28:02]

nit: "may be" -> "should be"

[Bernhard Bauer, 2014/08/19 17:11:13]

Nit: pipes around |includes_session_sync_|.

[Pam (message me for reviews), 2014/08/19 20:17:16]

"Attached to" isn't the customary terminology, so it's hard to understand.

[fhorschig, 2014/08/20 11:59:30]

Done.

[fhorschig, 2014/08/20 11:59:30]

Done.

[fhorschig, 2014/08/20 11:59:30]

Done.

+  void OnNewSettingsAvailable(const base::DictionaryValue* settings);
+
+  // Reads the state of |includes_session_sync_| from given SU settings. The
+  // returned

[Marc Treib, 2014/08/19 15:28:01]

nit: Combine with the line below.

[fhorschig, 2014/08/20 11:59:30]

Done.

+  // value indicates if |includes_session_sync_| has changed.
+  bool FetchNewSessionSyncState(const base::DictionaryValue* settings);
+
+  // Reinitializing this Service means reloading the Sync settings. This method

[Pam (message me for reviews), 2014/08/19 20:17:16]

Reconfiguring

But then why not just name it "ReloadSyncSettings"?

[Marc Treib, 2014/08/20 10:04:55]

Because it's not clear what "sync settings" means. "Reconfigure" is the
terminology used in the PSS.
Anyway, this becomes moot when there's a methdod for this in the PSS.

+  // thereby ensures that changes in Sync settings take effect immediately.
+  void ReconfigureProfileSyncService();
+
+  // Reinitializing this Service means reloading the Sync settings. This method
+  // thereby ensures that changes in Sync settings take effect immediately.

[Pam (message me for reviews), 2014/08/19 20:17:16]

Duplicate comment

[fhorschig, 2014/08/20 11:59:30]

Done.

+  bool IncludesSyncSessions() const;
+
+  // The option a custodian sets to either recored or prevent recording the

[Pam (message me for reviews), 2014/08/19 20:17:16]

TYpo: "record"

[fhorschig, 2014/08/20 11:59:30]

Done.

+  // supervised user's history. Set by |FetchNewSessionSyncState()| and
+  // defaults to true.
+  bool includes_session_sync_;
+
   // Owns us via the KeyedService mechanism.
   Profile* profile_;