NTPSnippets: Set MaxRetriesOn5xx only for interactive requests.

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 <utility>
 
 #include "base/command_line.h"
@@ skipping 643 matching lines @@
 
 NTPSnippetsFetcher::RequestBuilder&
 NTPSnippetsFetcher::RequestBuilder::SetUserClassifier(
     const UserClassifier& user_classifier) {
   if (IsSendingUserClassEnabled()) {
     user_class_ = GetUserClassString(user_classifier.GetUserClass());
   }
   return *this;
 }
 
+std::unique_ptr<net::URLFetcher>
+NTPSnippetsFetcher::RequestBuilder::PreviewURLFetcherForTesting() const {
+  return BuildURLFetcher(/*request=*/nullptr,  // Can never be called anyways.

[tschumann, 2016/12/05 17:26:33]

this is a smell. If you really need something like this, then subclass the
builder, do this modification there and test it. But again, I'd much rather have
the test using the public interface and test the actual class.

[fhorschig, 2016/12/06 14:31:40]

Removed. As noted in your last comment, this was easy to do with the
TestURLFetcher.

+                         /*headers=*/"",       // Can be set later if needed.
+                         /*body=*/"");         // Can be set later if needed.
+}
+
 bool NTPSnippetsFetcher::RequestBuilder::IsSendingTopLanguagesEnabled() const {
   return IsBooleanParameterEnabled(kSendTopLanguagesName,
                                    /*default_value=*/false);
 }
 
 bool NTPSnippetsFetcher::RequestBuilder::IsSendingUserClassEnabled() const {
   return IsBooleanParameterEnabled(kSendUserClassName,
                                    /*default_value=*/false);
 }
 
@@ skipping 131 matching lines @@
   url_fetcher->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
                             net::LOAD_DO_NOT_SAVE_COOKIES);
   data_use_measurement::DataUseUserData::AttachToFetcher(
       url_fetcher.get(), data_use_measurement::DataUseUserData::NTP_SNIPPETS);
 
   url_fetcher->SetExtraRequestHeaders(headers);
   url_fetcher->SetUploadData("application/json", body);
 
   // Fetchers are sometimes cancelled because a network change was detected.
   url_fetcher->SetAutomaticallyRetryOnNetworkChanges(3);
-  // Try to make fetching the files bit more robust even with poor connection.
-  url_fetcher->SetMaxRetriesOn5xx(3);
+  if (params_.interactive_request) {
+    // Try to make fetching the files bit more robust even with poor connection.
+    url_fetcher->SetMaxRetriesOn5xx(2);
+  } else {
+    // Don't retry immediately if a scheduled fetch fails.
+    // TODO(fhorschig): back off for 1h (+1h jitter) and then retry.
+    url_fetcher->SetMaxRetriesOn5xx(0);
+  }
   return url_fetcher;
 }
 
 void NTPSnippetsFetcher::RequestBuilder::PrepareLanguages(
     translate::LanguageModel::LanguageInfo* ui_language,
     translate::LanguageModel::LanguageInfo* other_top_language) const {
   // TODO(jkrcal): Add language model factory for iOS and add fakes to tests so
   // that |language_model| is never nullptr. Remove this check and add a DCHECK
   // into the constructor.
   if (!language_model_ || !IsSendingTopLanguagesEnabled()) {
@@ skipping 270 matching lines @@
   NOTREACHED();
   return false;
 }
 
 bool NTPSnippetsFetcher::NeedsAuthentication() const {
   return (personalization_ == Personalization::kPersonal ||
           personalization_ == Personalization::kBoth);
 }
 
 }  // namespace ntp_snippets