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

components/ntp_snippets/sessions/tab_delegate_sync_adapter.cc

Index: components/ntp_snippets/sessions/tab_delegate_sync_adapter.cc
diff --git a/components/ntp_snippets/sessions/tab_delegate_sync_adapter.cc b/components/ntp_snippets/sessions/tab_delegate_sync_adapter.cc
index 4ddd3920f269cfa6ca37f057b01cdfa29f2aee11..1e0624cbc930d04f4a959ad4eb55c07db8f9e4f7 100644
--- a/components/ntp_snippets/sessions/tab_delegate_sync_adapter.cc
+++ b/components/ntp_snippets/sessions/tab_delegate_sync_adapter.cc
@@ -45,14 +45,38 @@ void TabDelegateSyncAdapter::SubscribeForForeignTabChange(
 }
 
 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() {
-  InvokeCallback();
+  // 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();
 }