[Most Visited] Check for Sync state when using SuggestionsService.

chrome/browser/android/most_visited_sites.cc

Index: chrome/browser/android/most_visited_sites.cc
diff --git a/chrome/browser/android/most_visited_sites.cc b/chrome/browser/android/most_visited_sites.cc
index e13597ca0f14de5acfd9574152ff3e1ef2cf2604..ba70dc8fc16f31fab72b6c12c8255920944e48a0 100644
--- a/chrome/browser/android/most_visited_sites.cc
+++ b/chrome/browser/android/most_visited_sites.cc
@@ -24,6 +24,8 @@
 #include "chrome/browser/profiles/profile_android.h"
 #include "chrome/browser/search/suggestions/suggestions_service_factory.h"
 #include "chrome/browser/search/suggestions/suggestions_source.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/thumbnails/thumbnail_list_source.h"
 #include "components/suggestions/suggestions_service.h"
 #include "content/public/browser/browser_thread.h"
@@ -171,17 +173,44 @@ void LogHistogramEvent(const std::string& histogram, int position,
     counter->Add(position);
 }
 
+// Returns whether full history sync is enabled for this user.
+bool IsHistorySyncEnabled(Profile* profile) {
+  ProfileSyncService* sync =
+      ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile);
+  return sync &&
+         sync->sync_initialized() &&
+         sync->GetActiveDataTypes().Has(syncer::HISTORY_DELETE_DIRECTIVES);
+}
+
+// Returns whether the sync service is initialized.
+bool IsSyncInitialized(Profile* profile) {
+  ProfileSyncService* sync =
+      ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile);
+  return sync && sync->sync_initialized();
+}
+
 }  // namespace
 
 MostVisitedSites::MostVisitedSites(Profile* profile)
-    : profile_(profile), num_sites_(0), is_control_group_(false),
-      num_local_thumbs_(0), num_server_thumbs_(0), num_empty_thumbs_(0),
-      weak_ptr_factory_(this) {
+    : profile_(profile), num_sites_(0), sync_initialized_(false),
+      is_control_group_(false), num_local_thumbs_(0), num_server_thumbs_(0),
+      num_empty_thumbs_(0), weak_ptr_factory_(this) {
   // Register the debugging page for the Suggestions Service and the thumbnails
   // debugging page.
   content::URLDataSource::Add(profile_,
                               new suggestions::SuggestionsSource(profile_));
   content::URLDataSource::Add(profile_, new ThumbnailListSource(profile_));
+
+  // Save sync initialization state and if not initialized, register this class
+  // as an observer.

[manzagop (departed), 2014/08/15 13:01:52]

Perhaps explain why this is needed: ensuring the state can be changed even if
suggestions are only ever requested when initialization is incomplete (the user
only visits the NTP on startup).

[Mathieu, 2014/08/20 14:21:32]

Done.

+  ProfileSyncService* profile_sync_service =

[manzagop (departed), 2014/08/15 13:01:52]

I said yesterday I thought this would be null is the user is not signed in. This
seems wrong: it seems it only checks for a commandline flag.

There is however a IsSyncEnabledAndLoggedIn()

[Mathieu, 2014/08/20 14:21:31]

Done.

+      ProfileSyncServiceFactory::GetForProfile(profile_);
+  if (profile_sync_service) {
+    sync_initialized_ = profile_sync_service->sync_initialized();

[manzagop (departed), 2014/08/15 13:01:52]

sync_initialized_ doesn't need to be a class member. It's always set to a new
value before being used.

[Mathieu, 2014/08/20 14:21:32]

Done.

+    if (!sync_initialized_) {
+      profile_sync_service->AddObserver(this);
+    }
+  }
 }
 
 MostVisitedSites::~MostVisitedSites() {
@@ -230,11 +259,13 @@ void MostVisitedSites::GetURLThumbnail(JNIEnv* env,
   std::string url_string = ConvertJavaStringToUTF8(env, url);
   scoped_refptr<TopSites> top_sites(profile_->GetTopSites());
 
-  // If the Suggestions service is enabled, create a callback to fetch a
-  // server thumbnail from it, in case the local thumbnail is not found.
+  // If the Suggestions service is enabled and in use, create a callback to
+  // fetch a server thumbnail from it, in case the local thumbnail is not found.
   SuggestionsService* suggestions_service =
       SuggestionsServiceFactory::GetForProfile(profile_);
-  base::Closure lookup_failed_callback = suggestions_service ?
+  bool use_suggestions_service = suggestions_service &&
+      mv_source_ == SUGGESTIONS_SERVICE;
+  base::Closure lookup_failed_callback = use_suggestions_service ?
       base::Bind(&MostVisitedSites::GetSuggestionsThumbnailOnUIThread,
                  weak_ptr_factory_.GetWeakPtr(),
                  suggestions_service, url_string,
@@ -317,6 +348,26 @@ void MostVisitedSites::Observe(int type,
   }
 }
 
+void MostVisitedSites::OnStateChanged() {
+  ProfileSyncService* sync =
+      ProfileSyncServiceFactory::GetForProfile(profile_);
+  // We only care about the sync initialization event.
+  if (sync && sync->sync_initialized()) {

[manzagop (departed), 2014/08/15 13:01:52]

We should double check the sync service gets initialized even when sync is
disabled.

[Mathieu, 2014/08/20 14:21:32]

Done.

+    sync_initialized_ = true;
+    SuggestionsService* suggestions_service =
+        SuggestionsServiceFactory::GetForProfile(profile_);
+    if (suggestions_service) {
+      // This serves as a cache warm-up or a cache refresh, depending if the
+      // SuggestionsService query has happened at this point. Both are
+      // important.
+      suggestions_service->FetchSuggestionsData(
+          sync_initialized_,
+          IsHistorySyncEnabled(profile_),
+          base::Callback<void(const SuggestionsProfile&)>());
+    }
+  }
+}
+
 // static
 bool MostVisitedSites::Register(JNIEnv* env) {
   return RegisterNativesImpl(env);
@@ -328,6 +379,8 @@ void MostVisitedSites::QueryMostVisitedURLs() {
   if (suggestions_service) {
     // Suggestions service is enabled, initiate a query.
     suggestions_service->FetchSuggestionsData(
+        IsSyncInitialized(profile_),
+        IsHistorySyncEnabled(profile_),
         base::Bind(
           &MostVisitedSites::OnSuggestionsProfileAvailable,
           weak_ptr_factory_.GetWeakPtr(),