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

chrome/browser/android/most_visited_sites.cc

 // Copyright 2013 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 "chrome/browser/android/most_visited_sites.h"
 
 #include <string>
 #include <vector>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/callback.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/profiles/profile.h"
 #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"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/url_data_source.h"
 #include "jni/MostVisitedSites_jni.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/android/java_bitmap.h"
 #include "ui/gfx/codec/jpeg_codec.h"
 
@@ skipping 127 matching lines @@
   base::HistogramBase* counter = base::LinearHistogram::FactoryGet(
       histogram,
       1,
       num_sites,
       num_sites + 1,
       base::Histogram::kUmaTargetedHistogramFlag);
   if (counter)
     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() {
 }
 
 void MostVisitedSites::Destroy(JNIEnv* env, jobject obj) {
   delete this;
 }
 
 void MostVisitedSites::OnLoadingComplete(JNIEnv* env, jobject obj) {
@@ skipping 28 matching lines @@
                                        jstring url,
                                        jobject j_callback_obj) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   ScopedJavaGlobalRef<jobject>* j_callback =
       new ScopedJavaGlobalRef<jobject>();
   j_callback->Reset(env, j_callback_obj);
 
   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,
                  base::Owned(new ScopedJavaGlobalRef<jobject>(*j_callback))) :
       base::Closure();
   LookupSuccessCallback lookup_success_callback =
       base::Bind(&MostVisitedSites::OnObtainedThumbnail,
                  weak_ptr_factory_.GetWeakPtr());
 
   BrowserThread::PostTask(
@@ skipping 62 matching lines @@
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
   DCHECK_EQ(type, chrome::NOTIFICATION_TOP_SITES_CHANGED);
 
   if (mv_source_ == TOP_SITES) {
     // The displayed suggestions are invalidated.
     QueryMostVisitedURLs();
   }
 }
 
+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);
 }
 
 void MostVisitedSites::QueryMostVisitedURLs() {
   SuggestionsService* suggestions_service =
       SuggestionsServiceFactory::GetForProfile(profile_);
   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(),
           base::Owned(new ScopedJavaGlobalRef<jobject>(observer_))));
   } else {
     InitiateTopSitesQuery();
   }
 }
 
 void MostVisitedSites::InitiateTopSitesQuery() {
@@ skipping 137 matching lines @@
   num_empty_thumbs_ = 0;
   UMA_HISTOGRAM_SPARSE_SLOWLY(kNumServerTilesHistogramName, num_server_thumbs_);
   num_server_thumbs_ = 0;
 }
 
 static jlong Init(JNIEnv* env, jobject obj, jobject jprofile) {
   MostVisitedSites* most_visited_sites =
       new MostVisitedSites(ProfileAndroid::FromProfileAndroid(jprofile));
   return reinterpret_cast<intptr_t>(most_visited_sites);
 }