kiosk: Retry failed app data fetch.

chrome/browser/extensions/webstore_data_fetcher.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/extensions/webstore_data_fetcher.h"
 
 #include "base/bind.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/webstore_data_fetcher_delegate.h"
 #include "chrome/browser/safe_json_parser.h"
@@ skipping 11 matching lines @@
 namespace extensions {
 
 WebstoreDataFetcher::WebstoreDataFetcher(
     WebstoreDataFetcherDelegate* delegate,
     net::URLRequestContextGetter* request_context,
     const GURL& referrer_url,
     const std::string webstore_item_id)
     : delegate_(delegate),
       request_context_(request_context),
       referrer_url_(referrer_url),
-      id_(webstore_item_id) {
+      id_(webstore_item_id),
+      max_auto_retries_(0) {
 }
 
 WebstoreDataFetcher::~WebstoreDataFetcher() {}
 
 void WebstoreDataFetcher::Start() {
   GURL webstore_data_url(extension_urls::GetWebstoreItemJsonDataURL(id_));
 
   webstore_data_url_fetcher_.reset(net::URLFetcher::Create(
       webstore_data_url, net::URLFetcher::GET, this));
   webstore_data_url_fetcher_->SetRequestContext(request_context_);
   webstore_data_url_fetcher_->SetReferrer(referrer_url_.spec());
   webstore_data_url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
                                            net::LOAD_DISABLE_CACHE);
+  webstore_data_url_fetcher_->SetMaxRetriesOn5xx(max_auto_retries_);

[asargent_no_longer_on_chrome, 2014/07/10 23:08:34]

Should you only call this if max_auto_retries_ is > 0?

[xiyuan, 2014/07/10 23:23:13]

The code assumes that the default retry numbers in URLFetcher are 0 (which is
currently the case) and this is a no-op when |max_auto_retries_| is 0.

Do you feel this assumes too much implementation details and we should wrap it
with the suggested "if"?

[asargent_no_longer_on_chrome, 2014/07/10 23:49:06]

Yeah, I'm not sure. The URLFetcher header comments don't really make this clear,
so I was thinking that we might want to just take whatever the default behavior
is (which is currently 0, but could conceivably change some day?) unless our own
consumer wanted to change it to something else. I don't think it's a really a
big deal either way though, so use your best judgement. 

[xiyuan, 2014/07/11 04:05:44]

Okay. Think it makes more sense than assuming the default is no retry.

I have added the "if" so that we use the URLFetcher default behavior on 0 and
updated the comment in header file.

+  webstore_data_url_fetcher_->SetAutomaticallyRetryOnNetworkChanges(
+      max_auto_retries_);
   webstore_data_url_fetcher_->Start();
 }
 
 void WebstoreDataFetcher::OnJsonParseSuccess(
     scoped_ptr<base::Value> parsed_json) {
   if (!parsed_json->IsType(base::Value::TYPE_DICTIONARY)) {
     OnJsonParseFailure(kInvalidWebstoreResponseError);
     return;
   }
 
@@ skipping 24 matching lines @@
       new SafeJsonParser(webstore_json_data,
                          base::Bind(&WebstoreDataFetcher::OnJsonParseSuccess,
                                     AsWeakPtr()),
                          base::Bind(&WebstoreDataFetcher::OnJsonParseFailure,
                                     AsWeakPtr()));
   // The parser will call us back via one of the callbacks.
   parser->Start();
 }
 
 }  // namespace extensions