Adds a Fetch() method to the ContentSuggestionService which asks any provider to provide more conte…

components/ntp_snippets/remote/ntp_snippets_fetcher.cc

 // Copyright 2016 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/ntp_snippets/remote/ntp_snippets_fetcher.h"
 
 #include <cstdlib>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ skipping 298 matching lines @@
         << "Unknown value for " << kPersonalizationName << ": "
         << personalization;
   }
 }
 
 NTPSnippetsFetcher::~NTPSnippetsFetcher() {
   if (waiting_for_refresh_token_)
     token_service_->RemoveObserver(this);
 }
 
-void NTPSnippetsFetcher::SetCallback(
-    const SnippetsAvailableCallback& callback) {
-  snippets_available_callback_ = callback;
-}
-
-void NTPSnippetsFetcher::FetchSnippets(const Params& params) {
+void NTPSnippetsFetcher::FetchSnippets(const Params& params,
+                                       SnippetsAvailableCallback callback) {
   if (!DemandQuotaForRequest(params.interactive_request)) {
     FetchFinished(OptionalFetchedCategories(),
                   params.interactive_request
                       ? FetchResult::INTERACTIVE_QUOTA_ERROR
                       : FetchResult::NON_INTERACTIVE_QUOTA_ERROR,
                   /*extra_message=*/std::string());
     return;
   }
 
   params_ = params;
   fetch_start_time_ = tick_clock_->NowTicks();
+  snippets_available_callback_ = std::move(callback);
 
   bool use_authentication = UsesAuthentication();
   if (use_authentication && signin_manager_->IsAuthenticated()) {
     // Signed-in: get OAuth token --> fetch snippets.
     oauth_token_retried_ = false;
     StartTokenRequest();
   } else if (use_authentication && signin_manager_->AuthInProgress()) {
     // Currently signing in: wait for auth to finish (the refresh token) -->
     //     get OAuth token --> fetch snippets.
     if (!waiting_for_refresh_token_) {
@@ skipping 385 matching lines @@
 }
 
 void NTPSnippetsFetcher::OnJsonError(const std::string& error) {
   LOG(WARNING) << "Received invalid JSON (" << error
                << "): " << last_fetch_json_;
   FetchFinished(
       OptionalFetchedCategories(), FetchResult::JSON_PARSE_ERROR,
       /*extra_message=*/base::StringPrintf(" (error %s)", error.c_str()));
 }
 
+// The response from the backend might include suggestions from multiple
+// categories. If only fetches for a single category were requested, this
+// function filters them out.
+void NTPSnippetsFetcher::FilterCategories(FetchedCategoriesVector* categories) {
+  if (!params_.exclusive_category.has_value())
+    return;
+  Category exclusive = params_.exclusive_category.value();
+  auto category_it =
+      std::find_if(categories->begin(), categories->end(),
+                   [&exclusive](const FetchedCategory& c) -> bool {
+                     return c.category == exclusive;
+                   });
+  if (category_it == categories->end()) {
+    categories->clear();
+    return;
+  }
+  categories->erase(categories->begin(), category_it);
+  categories->erase(category_it + 1, categories->end());

[Marc Treib, 2016/11/07 14:28:23]

After looking at this for the 10th time, I'm now convinced that it's not only
ugly, it also doesn't work :D
The first erase() call will invalidate category_it (or at least "reinterpret"
it), so the second erase() call will do either nothing or maybe
unexpected/undefined things.

[tschumann, 2016/11/08 16:57:35]

Good catch! I'll fix it.

+}
+
 void NTPSnippetsFetcher::FetchFinished(
     OptionalFetchedCategories fetched_categories,
     FetchResult result,
     const std::string& extra_message) {
   DCHECK(result == FetchResult::SUCCESS || !fetched_categories);
   last_status_ = FetchResultToString(result) + extra_message;
 
+  // TODO(fhorschig): Filter (un)wanted categories by modifying fetch request.
+  // As soon as backends support the parameter, there is no reason to overfetch
+  // and filter here.

[Marc Treib, 2016/11/07 14:28:23]

I find this comment super confusing. In my version of the CL, I had changed it
to:
// Filter out unwanted categories if necessary.
// TODO(fhorschig): As soon as the server supports filtering by
// that instead of over-fetching and filtering here.

[tschumann, 2016/11/08 16:57:35]

Done.

+  if (fetched_categories.has_value())
+    FilterCategories(&fetched_categories.value());
+
   // Don't record FetchTimes if the result indicates that a precondition
   // failed and we never actually sent a network request
   if (!IsFetchPreconditionFailed(result)) {
     UMA_HISTOGRAM_TIMES("NewTabPage.Snippets.FetchTime",
                         tick_clock_->NowTicks() - fetch_start_time_);
   }
   UMA_HISTOGRAM_ENUMERATION("NewTabPage.Snippets.FetchResult",
                             static_cast<int>(result),
                             static_cast<int>(FetchResult::RESULT_MAX));
 
   DVLOG(1) << "Fetch finished: " << last_status_;
   if (!snippets_available_callback_.is_null())
-    snippets_available_callback_.Run(std::move(fetched_categories));
+    std::move(snippets_available_callback_).Run(std::move(fetched_categories));
 }
 
 bool NTPSnippetsFetcher::DemandQuotaForRequest(bool interactive_request) {
   switch (user_classifier_->GetUserClass()) {
     case UserClassifier::UserClass::RARE_NTP_USER:
       return request_throttler_rare_ntp_user_.DemandQuotaForRequest(
           interactive_request);
     case UserClassifier::UserClass::ACTIVE_NTP_USER:
       return request_throttler_active_ntp_user_.DemandQuotaForRequest(
           interactive_request);
     case UserClassifier::UserClass::ACTIVE_SUGGESTIONS_CONSUMER:
       return request_throttler_active_suggestions_consumer_
           .DemandQuotaForRequest(interactive_request);
   }
   NOTREACHED();
   return false;
 }
 
 }  // namespace ntp_snippets