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

BUG=386454
R=manzagop@chromium.org, newt@chromium.org
TBR=erikwright
TEST=SuggestionsServiceTest

Review URL: https://codereview.chromium.org/473123002

Cr-Commit-Position: refs/heads/master@{#291116}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291116 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 185 insertions, 37 deletions

diff --git a/components/suggestions/BUILD.gn b/components/suggestions/BUILD.gn
index 1fe2ff9..d505c40 100644
--- a/components/suggestions/BUILD.gn
+++ b/components/suggestions/BUILD.gn
@@ -13,6 +13,8 @@ static_library("suggestions") {
     "suggestions_service.h",
     "suggestions_store.cc",
     "suggestions_store.h",
+    "suggestions_utils.cc",
+    "suggestions_utils.h",
   ]
 
   deps = [
diff --git a/components/suggestions/suggestions_service.cc b/components/suggestions/suggestions_service.cc
index db1e3fc..b12aae1 100644
--- a/components/suggestions/suggestions_service.cc
+++ b/components/suggestions/suggestions_service.cc
@@ -156,8 +156,25 @@ bool SuggestionsService::IsControlGroup() {
 }
 
 void SuggestionsService::FetchSuggestionsData(
+    SyncState sync_state,
     SuggestionsService::ResponseCallback callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  if (sync_state == NOT_INITIALIZED_ENABLED) {
+    // Sync is not initialized yet, but enabled. Serve previously cached
+    // suggestions if available.
+    waiting_requestors_.push_back(callback);
+    ServeFromCache();
+    return;
+  } else if (sync_state == SYNC_OR_HISTORY_SYNC_DISABLED) {
+    // Cancel any ongoing request (and the timeout closure). We must no longer
+    // interact with the server.
+    pending_request_.reset(NULL);
+    pending_timeout_closure_.reset(NULL);
+    suggestions_store_->ClearSuggestions();
+    callback.Run(SuggestionsProfile());
+    DispatchRequestsAndClear(SuggestionsProfile(), &waiting_requestors_);
+    return;
+  }
 
   FetchSuggestionsDataNoTimeout(callback);
 
@@ -170,21 +187,6 @@ void SuggestionsService::FetchSuggestionsData(
       base::TimeDelta::FromMilliseconds(request_timeout_ms_));
 }
 
-void SuggestionsService::FetchSuggestionsDataNoTimeout(
-    SuggestionsService::ResponseCallback callback) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  if (pending_request_.get()) {
-    // Request already exists, so just add requestor to queue.
-    waiting_requestors_.push_back(callback);
-    return;
-  }
-
-  // Form new request.
-  DCHECK(waiting_requestors_.empty());
-  waiting_requestors_.push_back(callback);
-  IssueRequest(suggestions_url_);
-}
-
 void SuggestionsService::GetPageThumbnail(
     const GURL& url,
     base::Callback<void(const GURL&, const SkBitmap*)> callback) {
@@ -235,6 +237,33 @@ void SuggestionsService::RegisterProfilePrefs(
   BlacklistStore::RegisterProfilePrefs(registry);
 }
 
+void SuggestionsService::SetDefaultExpiryTimestamp(
+    SuggestionsProfile* suggestions, int64 default_timestamp_usec) {
+  for (int i = 0; i < suggestions->suggestions_size(); ++i) {
+    ChromeSuggestion* suggestion = suggestions->mutable_suggestions(i);
+    // Do not set expiry if the server has already provided a more specific
+    // expiry time for this suggestion.
+    if (!suggestion->has_expiry_ts()) {
+      suggestion->set_expiry_ts(default_timestamp_usec);
+    }
+  }
+}
+
+void SuggestionsService::FetchSuggestionsDataNoTimeout(
+    SuggestionsService::ResponseCallback callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (pending_request_.get()) {
+    // Request already exists, so just add requestor to queue.
+    waiting_requestors_.push_back(callback);
+    return;
+  }
+
+  // Form new request.
+  DCHECK(waiting_requestors_.empty());
+  waiting_requestors_.push_back(callback);
+  IssueRequest(suggestions_url_);
+}
+
 void SuggestionsService::IssueRequest(const GURL& url) {
   pending_request_.reset(CreateSuggestionsRequest(url));
   pending_request_->Start();
@@ -330,18 +359,6 @@ void SuggestionsService::OnURLFetchComplete(const net::URLFetcher* source) {
   ScheduleBlacklistUpload(true);
 }
 
-void SuggestionsService::SetDefaultExpiryTimestamp(
-    SuggestionsProfile* suggestions, int64 default_timestamp_usec) {
-  for (int i = 0; i < suggestions->suggestions_size(); ++i) {
-    ChromeSuggestion* suggestion = suggestions->mutable_suggestions(i);
-    // Do not set expiry if the server has already provided a more specific
-    // expiry time for this suggestion.
-    if (!suggestion->has_expiry_ts()) {
-      suggestion->set_expiry_ts(default_timestamp_usec);
-    }
-  }
-}
-
 void SuggestionsService::Shutdown() {
   // Cancel pending request and timeout closure, then serve existing requestors
   // from cache.
diff --git a/components/suggestions/suggestions_service.h b/components/suggestions/suggestions_service.h
index 3aa03a0..e495f7f 100644
--- a/components/suggestions/suggestions_service.h
+++ b/components/suggestions/suggestions_service.h
@@ -19,6 +19,7 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/suggestions/image_manager.h"
 #include "components/suggestions/proto/suggestions.pb.h"
+#include "components/suggestions/suggestions_utils.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"
@@ -64,16 +65,20 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
   // Whether the user is part of a control group.
   static bool IsControlGroup();
 
-  // Request suggestions data, which will be passed to |callback|. Initiates a
-  // fetch request unless a pending one exists. To prevent multiple requests,
-  // we place all |callback|s in a queue and update them simultaneously when
-  // fetch request completes. Also posts a task to execute OnRequestTimeout
-  // if the request hasn't completed in a given amount of time.
-  void FetchSuggestionsData(ResponseCallback callback);
-
-  // Similar to FetchSuggestionsData but doesn't post a task to execute
-  // OnDelaySinceFetch.
-  void FetchSuggestionsDataNoTimeout(ResponseCallback callback);
+  // Request suggestions data, which will be passed to |callback|. |sync_state|
+  // will influence the behavior of this function (see SyncState definition).
+  //
+  // |sync_state| must be specified based on the current state of the system
+  // (see suggestions::GetSyncState). Callers should call this function again if
+  // sync state changes.
+  //
+  // If state allows for a network request, it is initiated unless a pending one
+  // exists. To prevent multiple requests, all |callback|s are placed in a queue
+  // and are updated simultaneously when the fetch completes. Also posts a task
+  // to execute OnRequestTimeout if the request hasn't completed in a given
+  // amount of time.
+  void FetchSuggestionsData(SyncState sync_state,
+                            ResponseCallback callback);
 
   // Retrieves stored thumbnail for website |url| asynchronously. Calls
   // |callback| with Bitmap pointer if found, and NULL otherwise.
@@ -97,10 +102,15 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
   void SetDefaultExpiryTimestamp(SuggestionsProfile* suggestions,
                                  int64 timestamp_usec);
  private:
+  friend class SuggestionsServiceTest;
   FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, BlacklistURLFails);
   FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, FetchSuggestionsData);
   FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, UpdateBlacklistDelay);
 
+  // Similar to FetchSuggestionsData but doesn't post a task to execute
+  // OnDelaySinceFetch.
+  void FetchSuggestionsDataNoTimeout(ResponseCallback callback);
+
   // Issue a request.
   void IssueRequest(const GURL& url);
 
diff --git a/components/suggestions/suggestions_service_unittest.cc b/components/suggestions/suggestions_service_unittest.cc
index 819e92b..a4d6058 100644
--- a/components/suggestions/suggestions_service_unittest.cc
+++ b/components/suggestions/suggestions_service_unittest.cc
@@ -18,6 +18,7 @@
 #include "components/suggestions/image_manager.h"
 #include "components/suggestions/proto/suggestions.pb.h"
 #include "components/suggestions/suggestions_store.h"
+#include "components/suggestions/suggestions_utils.h"
 #include "components/variations/entropy_provider.h"
 #include "components/variations/variations_associated_data.h"
 #include "net/http/http_response_headers.h"
@@ -326,6 +327,7 @@ TEST_F(SuggestionsServiceTest, FetchSuggestionsDataRequestError) {
 
   // Send the request. Empty data will be returned to the callback.
   suggestions_service->FetchSuggestionsData(
+      INITIALIZED_ENABLED_HISTORY,  // Normal mode.
       base::Bind(&SuggestionsServiceTest::ExpectEmptySuggestionsProfile,
                  base::Unretained(this)));
 
@@ -361,6 +363,7 @@ TEST_F(SuggestionsServiceTest, FetchSuggestionsDataResponseNotOK) {
 
   // Send the request. Empty data will be returned to the callback.
   suggestions_service->FetchSuggestionsData(
+      INITIALIZED_ENABLED_HISTORY,  // Normal mode.
       base::Bind(&SuggestionsServiceTest::ExpectEmptySuggestionsProfile,
                  base::Unretained(this)));
 
@@ -371,6 +374,62 @@ TEST_F(SuggestionsServiceTest, FetchSuggestionsDataResponseNotOK) {
   EXPECT_EQ(1, suggestions_empty_data_count_);
 }
 
+TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncDisabled) {
+  // Field trial enabled with a specific suggestions URL.
+  EnableFieldTrial(kFakeSuggestionsURL, kFakeSuggestionsCommonParams,
+                   kFakeBlacklistPath, kFakeBlacklistUrlParam, false);
+  scoped_ptr<SuggestionsService> suggestions_service(
+      CreateSuggestionsServiceWithMocks());
+  EXPECT_TRUE(suggestions_service != NULL);
+
+  // Set up expectations on the SuggestionsStore.
+  EXPECT_CALL(*mock_suggestions_store_, ClearSuggestions());
+
+  // Send the request. Cache is cleared and empty data will be returned to the
+  // callback.
+  suggestions_service->FetchSuggestionsData(
+      SYNC_OR_HISTORY_SYNC_DISABLED,
+      base::Bind(&SuggestionsServiceTest::ExpectEmptySuggestionsProfile,
+                 base::Unretained(this)));
+
+  // Wait for posted task to complete.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Ensure that ExpectEmptySuggestionsProfile ran once.
+  EXPECT_EQ(1, suggestions_empty_data_count_);
+}
+
+TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncNotInitializedEnabled) {
+  // Field trial enabled with a specific suggestions URL.
+  EnableFieldTrial(kFakeSuggestionsURL, kFakeSuggestionsCommonParams,
+                   kFakeBlacklistPath, kFakeBlacklistUrlParam, false);
+  scoped_ptr<SuggestionsService> suggestions_service(
+      CreateSuggestionsServiceWithMocks());
+  EXPECT_TRUE(suggestions_service != NULL);
+  scoped_ptr<SuggestionsProfile> suggestions_profile(
+      CreateSuggestionsProfile());
+
+  // Expectations.
+  EXPECT_CALL(*mock_suggestions_store_, LoadSuggestions(_))
+      .WillOnce(DoAll(SetArgPointee<0>(*suggestions_profile), Return(true)));
+  EXPECT_CALL(*mock_thumbnail_manager_,
+              Initialize(EqualsProto(*suggestions_profile)));
+  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_));
+
+  // Send the request. In this state, cached data will be returned to the
+  // caller.
+  suggestions_service->FetchSuggestionsData(
+      NOT_INITIALIZED_ENABLED,
+      base::Bind(&SuggestionsServiceTest::CheckSuggestionsData,
+                 base::Unretained(this)));
+
+  // Wait for posted task to complete.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Ensure that CheckSuggestionsData ran once.
+  EXPECT_EQ(1, suggestions_data_check_count_);
+}
+
 TEST_F(SuggestionsServiceTest, BlacklistURL) {
   EnableFieldTrial(kFakeSuggestionsURL, kFakeSuggestionsCommonParams,
                    kFakeBlacklistPath, kFakeBlacklistUrlParam, false);
diff --git a/components/suggestions/suggestions_utils.cc b/components/suggestions/suggestions_utils.cc
new file mode 100644
index 0000000..9c32791
--- /dev/null
+++ b/components/suggestions/suggestions_utils.cc
@@ -0,0 +1,22 @@
+// Copyright 2014 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 "components/suggestions/suggestions_utils.h"
+
+namespace suggestions {
+
+SyncState GetSyncState(bool sync_enabled,
+                       bool sync_initialized,
+                       bool history_sync_enabled) {
+  if (!sync_enabled)
+    return SYNC_OR_HISTORY_SYNC_DISABLED;
+
+  if (!sync_initialized)
+    return NOT_INITIALIZED_ENABLED;
+
+  return history_sync_enabled ?
+      INITIALIZED_ENABLED_HISTORY : SYNC_OR_HISTORY_SYNC_DISABLED;
+}
+
+}  // namespace suggestions
diff --git a/components/suggestions/suggestions_utils.h b/components/suggestions/suggestions_utils.h
new file mode 100644
index 0000000..eedef03
--- /dev/null
+++ b/components/suggestions/suggestions_utils.h
@@ -0,0 +1,38 @@
+// Copyright 2014 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.
+
+#ifndef COMPONENTS_SUGGESTIONS_SUGGESTIONS_UTILS_H_
+#define COMPONENTS_SUGGESTIONS_SUGGESTIONS_UTILS_H_
+
+namespace suggestions {
+
+// Establishes the different sync states that users of SuggestionsService can
+// specify. There are three different concepts in the sync service: initialized,
+// sync enabled and history sync enabled.
+enum SyncState {
+  // State: Sync service is not initialized, yet not disabled. History sync
+  //     state is unknown (since not initialized).
+  // Behavior: Does not issue a server request, but serves from cache if
+  //     available.
+  NOT_INITIALIZED_ENABLED,
+
+  // State: Sync service is initialized, sync is enabled and history sync is
+  //     enabled.
+  // Behavior: Update suggestions from the server. Serve from cache on timeout.
+  INITIALIZED_ENABLED_HISTORY,
+
+  // State: Sync service is disabled or history sync is disabled.
+  // Behavior: Do not issue a server request. Clear the cache. Serve empty
+  //     suggestions.
+  SYNC_OR_HISTORY_SYNC_DISABLED,
+};
+
+// Users of SuggestionsService should always use this function to get SyncState.
+SyncState GetSyncState(bool sync_enabled,
+                       bool sync_initialized,
+                       bool history_sync_enabled);
+
+}  // namespace suggestions
+
+#endif  // COMPONENTS_SUGGESTIONS_SUGGESTIONS_UTILS_H_