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

Issue 2835533002: Remove URLRequestJob::GetResponseCode implementations (Closed)

Created:
3 years, 8 months ago by mmenke
Modified:
3 years, 7 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, romax+watch_chromium.org, fgorski+watch_chromium.org, dewittj+watch_chromium.org, jam, petewil+watch_chromium.org, net-reviews_chromium.org, chili+watch_chromium.org, darin-cc_chromium.org, dimich+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove URLRequestJob::GetResponseCode implementations from content, chrome, and net/. URLRequestJob itself now has a default implementation, which gets the response code from the headers returned by GetResponseInfo. We eventually want to remove this method, since it's redundant, and has weird behavior for just when it is and is not safe to call. BUG=688481 Review-Url: https://codereview.chromium.org/2835533002 Cr-Commit-Position: refs/heads/master@{#467839} Committed: https://chromium.googlesource.com/chromium/src/+/e47392b3d5fa37b785ea7494bc7bd32163ba9188

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -26 lines) Patch
M chrome/browser/android/offline_pages/offline_page_request_job.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/android/offline_pages/offline_page_request_job.cc View 1 chunk +0 lines, -7 lines 0 comments Download
M content/browser/webui/url_data_manager_backend.cc View 2 chunks +0 lines, -5 lines 0 comments Download
M content/public/test/test_download_request_handler.cc View 2 chunks +0 lines, -6 lines 0 comments Download
M net/url_request/url_request_test_job.h View 1 chunk +0 lines, -1 line 0 comments Download
M net/url_request/url_request_test_job.cc View 1 chunk +0 lines, -6 lines 0 comments Download

Messages

Total messages: 23 (12 generated)
mmenke
dbeam: Please review content/browser/webui/url_data_manager_backend.cc phajdan.jr: Please review content/public/test/test_download_request_handler.cc
3 years, 8 months ago (2017-04-21 16:30:20 UTC) #6
mmenke
Oops, sent that out without finishing the message! This change just removes a number of ...
3 years, 8 months ago (2017-04-21 16:33:49 UTC) #8
Julia Tuttle
On 2017/04/21 16:33:49, mmenke wrote: > Oops, sent that out without finishing the message! This ...
3 years, 8 months ago (2017-04-21 17:33:24 UTC) #9
Dan Beam
lgtm
3 years, 8 months ago (2017-04-21 17:38:52 UTC) #10
mmenke
On 2017/04/21 17:38:52, Dan Beam wrote: > lgtm [dimich / phajdan.jr]: Ping!
3 years, 8 months ago (2017-04-25 16:08:55 UTC) #11
Dmitry Titov
+jianli@, -dimich@ I'd like jianli@ to take a look at this instead of me, since ...
3 years, 8 months ago (2017-04-27 02:36:03 UTC) #13
jianli
offline_pages lgtm
3 years, 7 months ago (2017-04-27 20:34:33 UTC) #14
mmenke
[+sky]: Please review content/public/test/test_download_request_handler.cc. I'm just removing a GetResponseCode implementation (GetResponseInfo provides access to a ...
3 years, 7 months ago (2017-04-27 20:40:21 UTC) #16
sky
LGTM
3 years, 7 months ago (2017-04-27 22:32:44 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2835533002/1
3 years, 7 months ago (2017-04-27 22:34:11 UTC) #20
commit-bot: I haz the power
3 years, 7 months ago (2017-04-28 02:12:32 UTC) #23
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/e47392b3d5fa37b785ea7494bc7b...

Powered by Google App Engine
This is Rietveld 408576698