Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(402)

Side by Side Diff: sync/internal_api/attachments/attachment_downloader_impl.cc

Issue 554743004: Update AttachmentServiceImpl to retry attachment uploads. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Merge with master. Created 6 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "sync/internal_api/public/attachments/attachment_downloader_impl.h" 5 #include "sync/internal_api/public/attachments/attachment_downloader_impl.h"
6 6
7 #include "base/bind.h" 7 #include "base/bind.h"
8 #include "base/message_loop/message_loop.h" 8 #include "base/message_loop/message_loop.h"
9 #include "net/base/load_flags.h" 9 #include "net/base/load_flags.h"
10 #include "net/http/http_status_code.h" 10 #include "net/http/http_status_code.h"
(...skipping 97 matching lines...) Expand 10 before | Expand all | Expand 10 after
108 DCHECK(request == access_token_request_.get()); 108 DCHECK(request == access_token_request_.get());
109 access_token_request_.reset(); 109 access_token_request_.reset();
110 StateList::const_iterator iter; 110 StateList::const_iterator iter;
111 // Without access token all downloads fail. 111 // Without access token all downloads fail.
112 for (iter = requests_waiting_for_access_token_.begin(); 112 for (iter = requests_waiting_for_access_token_.begin();
113 iter != requests_waiting_for_access_token_.end(); 113 iter != requests_waiting_for_access_token_.end();
114 ++iter) { 114 ++iter) {
115 DownloadState* download_state = *iter; 115 DownloadState* download_state = *iter;
116 scoped_refptr<base::RefCountedString> null_attachment_data; 116 scoped_refptr<base::RefCountedString> null_attachment_data;
117 ReportResult( 117 ReportResult(
118 *download_state, DOWNLOAD_UNSPECIFIED_ERROR, null_attachment_data); 118 *download_state, DOWNLOAD_TRANSIENT_ERROR, null_attachment_data);
119 DCHECK(state_map_.find(download_state->attachment_url) != state_map_.end()); 119 DCHECK(state_map_.find(download_state->attachment_url) != state_map_.end());
120 state_map_.erase(download_state->attachment_url); 120 state_map_.erase(download_state->attachment_url);
121 } 121 }
122 requests_waiting_for_access_token_.clear(); 122 requests_waiting_for_access_token_.clear();
123 } 123 }
124 124
125 void AttachmentDownloaderImpl::OnURLFetchComplete( 125 void AttachmentDownloaderImpl::OnURLFetchComplete(
126 const net::URLFetcher* source) { 126 const net::URLFetcher* source) {
127 DCHECK(CalledOnValidThread()); 127 DCHECK(CalledOnValidThread());
128 128
129 // Find DownloadState by url. 129 // Find DownloadState by url.
130 AttachmentUrl url = source->GetOriginalURL().spec(); 130 AttachmentUrl url = source->GetOriginalURL().spec();
131 StateMap::iterator iter = state_map_.find(url); 131 StateMap::iterator iter = state_map_.find(url);
132 DCHECK(iter != state_map_.end()); 132 DCHECK(iter != state_map_.end());
133 const DownloadState& download_state = *iter->second; 133 const DownloadState& download_state = *iter->second;
134 DCHECK(source == download_state.url_fetcher.get()); 134 DCHECK(source == download_state.url_fetcher.get());
135 135
136 DownloadResult result = DOWNLOAD_UNSPECIFIED_ERROR; 136 DownloadResult result = DOWNLOAD_TRANSIENT_ERROR;
137 scoped_refptr<base::RefCountedString> attachment_data; 137 scoped_refptr<base::RefCountedString> attachment_data;
138 138
139 if (source->GetResponseCode() == net::HTTP_OK) { 139 const int response_code = source->GetResponseCode();
140 if (response_code == net::HTTP_OK) {
140 result = DOWNLOAD_SUCCESS; 141 result = DOWNLOAD_SUCCESS;
141 std::string data_as_string; 142 std::string data_as_string;
142 source->GetResponseAsString(&data_as_string); 143 source->GetResponseAsString(&data_as_string);
143 attachment_data = base::RefCountedString::TakeString(&data_as_string); 144 attachment_data = base::RefCountedString::TakeString(&data_as_string);
144 } else if (source->GetResponseCode() == net::HTTP_UNAUTHORIZED) { 145 } else if (response_code == net::HTTP_UNAUTHORIZED) {
146 // Server tells us we've got a bad token so invalidate it.
145 OAuth2TokenServiceRequest::InvalidateToken(token_service_provider_.get(), 147 OAuth2TokenServiceRequest::InvalidateToken(token_service_provider_.get(),
146 account_id_, 148 account_id_,
147 oauth2_scopes_, 149 oauth2_scopes_,
148 download_state.access_token); 150 download_state.access_token);
149 // TODO(pavely): crbug/380437. This is transient error. Request new access 151 // Fail the request, but indicate that it may be successful if retried.
150 // token for this DownloadState. The only trick is to do it with exponential 152 result = DOWNLOAD_TRANSIENT_ERROR;
151 // backoff. 153 } else if (response_code == net::HTTP_FORBIDDEN) {
154 // User is not allowed to use attachments. Retrying won't help.
155 result = DOWNLOAD_UNSPECIFIED_ERROR;
156 } else if (response_code == net::URLFetcher::RESPONSE_CODE_INVALID) {
157 result = DOWNLOAD_TRANSIENT_ERROR;
152 } 158 }
153 ReportResult(download_state, result, attachment_data); 159 ReportResult(download_state, result, attachment_data);
154 state_map_.erase(iter); 160 state_map_.erase(iter);
155 } 161 }
156 162
157 scoped_ptr<net::URLFetcher> AttachmentDownloaderImpl::CreateFetcher( 163 scoped_ptr<net::URLFetcher> AttachmentDownloaderImpl::CreateFetcher(
158 const AttachmentUrl& url, 164 const AttachmentUrl& url,
159 const std::string& access_token) { 165 const std::string& access_token) {
160 scoped_ptr<net::URLFetcher> url_fetcher( 166 scoped_ptr<net::URLFetcher> url_fetcher(
161 net::URLFetcher::Create(GURL(url), net::URLFetcher::GET, this)); 167 net::URLFetcher::Create(GURL(url), net::URLFetcher::GET, this));
168 url_fetcher->SetAutomaticallyRetryOn5xx(false);
162 const std::string auth_header("Authorization: Bearer " + access_token); 169 const std::string auth_header("Authorization: Bearer " + access_token);
163 url_fetcher->AddExtraRequestHeader(auth_header); 170 url_fetcher->AddExtraRequestHeader(auth_header);
164 url_fetcher->SetRequestContext(url_request_context_getter_.get()); 171 url_fetcher->SetRequestContext(url_request_context_getter_.get());
165 url_fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES | 172 url_fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
166 net::LOAD_DO_NOT_SEND_COOKIES | 173 net::LOAD_DO_NOT_SEND_COOKIES |
167 net::LOAD_DISABLE_CACHE); 174 net::LOAD_DISABLE_CACHE);
168 // TODO(maniscalco): Set an appropriate headers (User-Agent, what else?) on 175 // TODO(maniscalco): Set an appropriate headers (User-Agent, what else?) on
169 // the request (bug 371521). 176 // the request (bug 371521).
170 return url_fetcher.Pass(); 177 return url_fetcher.Pass();
171 } 178 }
(...skipping 21 matching lines...) Expand all
193 attachment.reset(new Attachment(Attachment::CreateWithId( 200 attachment.reset(new Attachment(Attachment::CreateWithId(
194 download_state.attachment_id, attachment_data))); 201 download_state.attachment_id, attachment_data)));
195 } 202 }
196 203
197 base::MessageLoop::current()->PostTask( 204 base::MessageLoop::current()->PostTask(
198 FROM_HERE, base::Bind(*iter, result, base::Passed(&attachment))); 205 FROM_HERE, base::Bind(*iter, result, base::Passed(&attachment)));
199 } 206 }
200 } 207 }
201 208
202 } // namespace syncer 209 } // namespace syncer
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698