Disable tab syncing when history saving is disabled.

chrome/browser/sync/sessions/session_data_type_controller.cc

Index: chrome/browser/sync/sessions/session_data_type_controller.cc
diff --git a/chrome/browser/sync/sessions/session_data_type_controller.cc b/chrome/browser/sync/sessions/session_data_type_controller.cc
index 24930ad44f27ad0ac597b05c419ff48861781972..172c334c400e275b47ff09f93408e7d84bc6e514 100644
--- a/chrome/browser/sync/sessions/session_data_type_controller.cc
+++ b/chrome/browser/sync/sessions/session_data_type_controller.cc
@@ -4,11 +4,13 @@
 
 #include "chrome/browser/sync/sessions/session_data_type_controller.h"
 
+#include "base/prefs/pref_service.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/glue/chrome_report_unrecoverable_error.h"
 #include "chrome/browser/sync/glue/synced_window_delegate.h"
 #include "chrome/browser/sync/sessions/synced_window_delegates_getter.h"
+#include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
@@ -34,12 +36,33 @@ SessionDataTypeController::SessionDataTypeController(
       waiting_on_session_restore_(false),
       waiting_on_local_device_info_(false) {
   DCHECK(local_device_);
+  pref_registrar_.Init(profile->GetPrefs());
+  pref_registrar_.Add(
+      prefs::kSavingBrowserHistoryDisabled,
+      base::Bind(&SessionDataTypeController::OnSavingBrowserHistoryPrefChanged,
+                 base::Unretained(this)));
 }
 
 SessionDataTypeController::~SessionDataTypeController() {}
 
+void SessionDataTypeController::LoadModels(
+    const ModelLoadCallback& model_load_callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[Andrew T Wilson (Slow), 2014/08/21 14:10:37]

OK to land this bit as-is, but ping Nicolas Zea whether we can just remove this
since we should never get LoadModels() if the policy is set during startup, and
if the policy changes at runtime, OnSavingBrowserHistoryPrefChanged() should
handle generating errors/stopping sync for this type.

[Joao da Silva, 2014/08/26 16:06:51]

I don't fully understand how the sync data type controllers work with the sync
backends; this snippet was taken from TypedUrlDataTypeController::LoadModels, so
maybe that should be fixed too.

Nicolas, ping :-)

[Nicolas Zea, 2014/08/26 17:59:30]

There's actually a new method you can override, ReadyForStart, which is better
for this kind of check. TypedURLDataTypeController should probably be using that
too. It's called on every configuration, and stops the datatype if it's not
true.

ExtensionBackedDataTypeController is an example that uses both ReadyForStar and
OnSingleDataTypeUnrecoverableError for a similar purpose (disabling a datatype
if an extension is not installed).

[Joao da Silva, 2014/08/26 18:43:03]

Done.

+  if (profile_->GetPrefs()->GetBoolean(prefs::kSavingBrowserHistoryDisabled)) {
+    model_load_callback.Run(
+        type(),
+        syncer::SyncError(FROM_HERE,
+                          syncer::SyncError::DATATYPE_ERROR,

[Nicolas Zea, 2014/08/26 17:59:30]

nit: this should be DATATYPE_POLICY_ERROR (same in Typed url data type
controller)

[Joao da Silva, 2014/08/26 18:43:02]

Done.

+                          "History and tab sync disabled by policy.",
+                          type()));
+    return;
+  }
+
+  UIDataTypeController::LoadModels(model_load_callback);
+}
+
 bool SessionDataTypeController::StartModels() {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   std::set<browser_sync::SyncedWindowDelegate*> window =
       synced_window_getter_->GetSyncedWindowDelegates();
   for (std::set<browser_sync::SyncedWindowDelegate*>::const_iterator i =
@@ -82,7 +105,7 @@ void SessionDataTypeController::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK_EQ(chrome::NOTIFICATION_SESSION_RESTORE_COMPLETE, type);
   DCHECK_EQ(profile_, content::Source<Profile>(source).ptr());
   notification_registrar_.RemoveAll();
@@ -99,4 +122,21 @@ void SessionDataTypeController::OnLocalDeviceInfoInitialized() {
   MaybeCompleteLoading();
 }
 
+void SessionDataTypeController::OnSavingBrowserHistoryPrefChanged() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (profile_->GetPrefs()->GetBoolean(prefs::kSavingBrowserHistoryDisabled)) {
+    // If history and tabs persistence is turned off then generate an
+    // unrecoverable error. SESSIONS won't be a registered type on the next
+    // Chrome restart.
+    if (state() != NOT_RUNNING && state() != STOPPING) {
+      syncer::SyncError error(
+          FROM_HERE,
+          syncer::SyncError::DATATYPE_POLICY_ERROR,
+          "History and tab saving is now disabled by policy.",
+          syncer::SESSIONS);
+      OnSingleDataTypeUnrecoverableError(error);
+    }
+  }
+}
+
 }  // namespace browser_sync