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 794 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.

[Marc Treib, 2016/12/06 16:06:56]

I'm not convinced we actually want to do this :)
Maybe better put this discussion in the bug?

[fhorschig, 2016/12/08 10:20:58]

Done already. I removed this comment for now as I won't submit this CL anyway
until I know if it is actually wanted behavior.

+    url_fetcher->SetMaxRetriesOn5xx(0);

[Marc Treib, 2016/12/06 16:06:56]

As discussed offline: Should we introduce a variation parameter to control this?

[fhorschig, 2016/12/08 10:20:58]

Done.

+  }
   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