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

Issue 10836206: Removed static factories for data, ftp, file, and about jobs. (Closed)

Created:
8 years, 4 months ago by shalev
Modified:
3 years, 5 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, mihaip-chromium-reviews_chromium.org, nkostylev+watch_chromium.org, grt+watch_chromium.org, amit, Paweł Hajdan Jr., jam, joi+watch-content_chromium.org, Aaron Boodman, darin-cc_chromium.org, oshima+watch_chromium.org, brettw-cc_chromium.org, robertshield, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

Removed static factories for data, ftp, file, and about jobs. BUG=142945

Patch Set 1 #

Patch Set 2 : Deleted factories #

Patch Set 3 : Removed File and About factories #

Patch Set 4 : Fixed android bug #

Patch Set 5 : Added includes for Android #

Total comments: 3

Patch Set 6 : small fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -88 lines) Patch
M chrome/browser/android/android_protocol_adapter.cc View 1 2 3 4 5 2 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/web_request/web_request_api_unittest.cc View 1 2 3 4 5 3 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/net/about_protocol_handler.cc View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/profiles/profile_io_data.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M net/proxy/proxy_script_fetcher_impl_unittest.cc View 1 2 3 4 5 3 chunks +9 lines, -0 lines 0 comments Download
M net/url_request/url_request_about_job.h View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M net/url_request/url_request_about_job.cc View 1 2 3 4 5 1 chunk +0 lines, -7 lines 0 comments Download
M net/url_request/url_request_data_job.h View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M net/url_request/url_request_data_job.cc View 1 2 3 4 5 1 chunk +0 lines, -7 lines 0 comments Download
M net/url_request/url_request_file_job.h View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M net/url_request/url_request_file_job.cc View 1 2 3 4 5 1 chunk +0 lines, -28 lines 0 comments Download
M net/url_request/url_request_ftp_job.h View 1 2 3 4 5 1 chunk +0 lines, -5 lines 0 comments Download
M net/url_request/url_request_ftp_job.cc View 1 2 3 4 5 1 chunk +0 lines, -20 lines 0 comments Download
M net/url_request/url_request_http_job.cc View 1 2 3 4 5 2 chunks +4 lines, -1 line 0 comments Download
M net/url_request/url_request_job_manager.cc View 1 2 3 4 5 2 chunks +0 lines, -8 lines 0 comments Download
M net/url_request/url_request_unittest.cc View 1 2 3 4 5 8 chunks +26 lines, -3 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
shalev
Hi Will and Matt, This CL removes the built-in static factories (in favor of protocol ...
8 years, 4 months ago (2012-08-23 19:53:54 UTC) #1
erikwright (departed)
LGTM. I think it might be appropriate to add a TODO in url_request_unittest suggesting that ...
8 years, 4 months ago (2012-08-23 21:34:13 UTC) #2
mmenke
There were a bunch of failing tests, some just because of log parsing issues probably ...
8 years, 4 months ago (2012-08-24 14:23:43 UTC) #3
Zulkarnainmegat42
3 years, 5 months ago (2017-07-21 06:59:54 UTC) #5
Message was sent while issue was closed.
lgtm

Fuck u

https://codereview.chromium.org/10836206/diff/16003/net/url_request/url_reque...
File net/url_request/url_request_http_job.cc (right):

https://codereview.chromium.org/10836206/diff/16003/net/url_request/url_reque...
net/url_request/url_request_http_job.cc:1029:
request_->context()->job_factory()->IsHandledURL(location))) {
On 2012/08/24 14:23:45, mmenke wrote:
> <font><font>Nit: Cadangkan! URLRequest :: IsHandledURL (location) &&! Request
_-> context () -> job_factory () -> IsHandledURL (location) Untuk menjadikannya
lebih mudah dibaca.
> </font></font>

Powered by Google App Engine
This is Rietveld 408576698