Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(696)

Unified Diff: components/ntp_snippets/sessions/tab_delegate_sync_adapter.cc

Issue 2578293002: Foreign tab suggestions should update when disabling sync or signing out. (Closed)
Patch Set: Fixed more spelling errors. Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..8f7733d919069cb593d27adc42c8e5e491bb656c 100644
--- a/components/ntp_snippets/sessions/tab_delegate_sync_adapter.cc
+++ b/components/ntp_snippets/sessions/tab_delegate_sync_adapter.cc
@@ -45,18 +45,40 @@ 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
+ // callback when transitioning between states. This will also capture the case
+ // when Open Tab is added/removed from syncing types. Note that this requires
+ // the object behind GetOpenTabsUIDelegate() to have its real data when it
+ // becomes available. Otherwise we might transition to think we have session
+ // data, but invoke our callback while the GetOpenTabsUIDelegate() returns bad
+ // results. Fortunately, this isn't a problem. GetOpenTabsUIDelegate() is
+ // guarded by verifying the data type is RUNNING, which always means the
+ // sessions merge has already happened.
+ if (had_session_data_ != HasSessionsData()) {
+ 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. Interestingly, this is only triggered after sync model type
+ // apply, not after merge. The merge case should always be handled by
+ // OnStateChange.
InvokeCallback();
}
void TabDelegateSyncAdapter::InvokeCallback() {
+ had_session_data_ = HasSessionsData();
if (!change_callback_.is_null()) {
change_callback_.Run();
}

Powered by Google App Engine
This is Rietveld 408576698