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
skym
2016/12/19 20:31:43
Done.
|
InvokeCallback(); |
} |