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

Issue 149172: Modify ResourceFetcher to use WebURLLoader instead of ResourceHandle.... (Closed)

Created:
11 years, 5 months ago by darin (slow to review)
Modified:
9 years, 7 months ago
Reviewers:
brettw
CC:
chromium-reviews_googlegroups.com, darin (slow to review)
Visibility:
Public.

Description

Modify ResourceFetcher to use WebURLLoader instead of ResourceHandle. This is step 1 of moving ResourceFetcher usage out of WebFrame. This CL adds a new method to WebFrame, named DispatchWillSendRequest, which may be used to associate a WebURLRequest with the WebFrame. This triggers the WebViewDelegate's WillSendRequest method among other things. ResourceFetcher and friends have been modified to use callbacks instead of delegates. I just find this approach a bit cleaner and easier to work with. BUG=15648 TEST=none R=brettw Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=20031

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+259 lines, -231 lines) Patch
M webkit/glue/alt_404_page_resource_fetcher.h View 4 chunks +12 lines, -8 lines 0 comments Download
M webkit/glue/alt_404_page_resource_fetcher.cc View 3 chunks +11 lines, -4 lines 0 comments Download
M webkit/glue/alt_error_page_resource_fetcher.h View 3 chunks +10 lines, -12 lines 0 comments Download
M webkit/glue/alt_error_page_resource_fetcher.cc View 2 chunks +11 lines, -5 lines 0 comments Download
M webkit/glue/image_resource_fetcher.h View 2 chunks +16 lines, -13 lines 0 comments Download
M webkit/glue/image_resource_fetcher.cc View 2 chunks +21 lines, -29 lines 0 comments Download
M webkit/glue/resource_fetcher.h View 3 chunks +48 lines, -49 lines 0 comments Download
M webkit/glue/resource_fetcher.cc View 3 chunks +60 lines, -57 lines 2 comments Download
M webkit/glue/resource_fetcher_unittest.cc View 9 chunks +21 lines, -32 lines 0 comments Download
M webkit/glue/webframe.h View 1 chunk +5 lines, -0 lines 0 comments Download
M webkit/glue/webframe_impl.h View 5 chunks +8 lines, -4 lines 0 comments Download
M webkit/glue/webframe_impl.cc View 1 3 chunks +11 lines, -1 line 0 comments Download
M webkit/glue/webframeloaderclient_impl.h View 2 chunks +5 lines, -1 line 0 comments Download
M webkit/glue/webframeloaderclient_impl.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M webkit/glue/webview_impl.h View 6 chunks +7 lines, -8 lines 0 comments Download
M webkit/glue/webview_impl.cc View 3 chunks +11 lines, -8 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
darin (slow to review)
11 years, 5 months ago (2009-07-06 17:48:18 UTC) #1
darin (slow to review)
11 years, 5 months ago (2009-07-07 05:45:46 UTC) #2
brettw
LGTM, seems nicer. http://codereview.chromium.org/149172/diff/1028/62 File webkit/glue/resource_fetcher.cc (right): http://codereview.chromium.org/149172/diff/1028/62#newcode26 Line 26: : url_(url), callback_(c), completed_(false) { ...
11 years, 5 months ago (2009-07-07 15:59:52 UTC) #3
darin (slow to review)
11 years, 5 months ago (2009-07-07 16:20:52 UTC) #4
http://codereview.chromium.org/149172/diff/1028/62
File webkit/glue/resource_fetcher.cc (right):

http://codereview.chromium.org/149172/diff/1028/62#newcode26
Line 26: : url_(url), callback_(c), completed_(false) {
On 2009/07/07 15:59:53, brettw wrote:
> Can you put these initializers on separate lines?

OK

Powered by Google App Engine
This is Rietveld 408576698