Foreign tab suggestions should update when disabling sync or signing out.

components/ntp_snippets/sessions/tab_delegate_sync_adapter.cc

 // Copyright 2016 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 "components/ntp_snippets/sessions/tab_delegate_sync_adapter.h"
 
 #include "components/sync/driver/sync_service.h"
 #include "components/sync_sessions/open_tabs_ui_delegate.h"
 
 using syncer::SyncService;
@@ skipping 27 matching lines @@
   return sessions;
 }
 
 void TabDelegateSyncAdapter::SubscribeForForeignTabChange(
     const base::Closure& change_callback) {
   DCHECK(change_callback_.is_null());
   change_callback_ = change_callback;
 }
 
 void TabDelegateSyncAdapter::OnStateChanged() {
-  // Ignored.
+  // OnStateChanged gets called very frequently, and usually is not important.
+  // But there are some events, like disabling sync and signing out, that are
+  // only captured through OnStateChange. In an attempt to send as few messages
+  // as possible, track if there was session data, and always/only invoke the
+  // calback when transitioning between states. This will also capture the case
+  // when Open Tab is added/removed from syncing types. It is important to
+  // realize that this approach requires the data returned by

[maxbogue, 2016/12/16 22:21:34]

Parse error, plz fix this sentence.

[skym, 2016/12/19 20:31:43]

Done.

+  // GetOpenTabsUIDelegate() on a transition to having session data must be
+  // returning the real and most current information as soon as it becomes not
+  // nullptr. This is currently the case, as GetOpenTabsUIDelegate() is gaurded

[maxbogue, 2016/12/16 22:21:34]

guarded*

[skym, 2016/12/19 20:31:43]

Done.

+  // by verifying the data type is RUNNING, which always means the model type
+  // merge has already happened.
+  if (had_session_data_ != HasSessionsData()) {
+    had_session_data_ = !had_session_data_;
+    InvokeCallback();
+  }
 }
 
 void TabDelegateSyncAdapter::OnSyncConfigurationCompleted() {
+  // Ignored. This event can let us know when the set of enabled data types
+  // change. However, we want to avoid useless notifications as much as
+  // possible, and all of the information captured in this event will also be
+  // covered by OnStateChange.
+}
+
+void TabDelegateSyncAdapter::OnForeignSessionUpdated() {
+  // Foreign tab data changed, always invoke the callback to generate new
+  // suggestions. The updating of |had_session_data_| here is a best effort to
+  // reduce spam, but may not be completely effective. Interestingly, this is
+  // only triggered after sync model type apply, not after merge. The merge case
+  // should always be handled by OnStateChange.
+  had_session_data_ = HasSessionsData();

[Marc Treib, 2016/12/19 11:47:53]

I think updating had_sessions_data_ should happen inside InvokeCallback - WDYT?

[skym, 2016/12/19 20:31:43]

Done.

   InvokeCallback();
 }
 
-void TabDelegateSyncAdapter::OnForeignSessionUpdated() {
-  InvokeCallback();
-}
-
 void TabDelegateSyncAdapter::InvokeCallback() {
   if (!change_callback_.is_null()) {
     change_callback_.Run();
   }
 }
 
 }  // namespace ntp_snippets