Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(22)

Issue 2891773002: Add a stub implementation of the URLLoaderFactory for AppCache. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 1 week ago by ananta
Modified:
2 weeks ago
Reviewers:
kinuko, jam
CC:
chromium-reviews, michaeln (ooo till 7-05), wjmaclean, jam, Randy Smith (Not in Mondays), darin-cc_chromium.org, loading-reviews_chromium.org, mmenke
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add a stub implementation of the URLLoaderFactory for AppCache. The AppCache factory has one instance per Storage partition and is maintained by the URLLoaderFactoryGetter class. The AppCache factory maintains a reference to the URLLoaderFactoryGetter instance and uses it to retrieve the network service to issue unhandled requests to it. Currently all requests are issued to the network service. Next step is to add functionality to serve AppCache responses from the AppCache. We no longer need the AppCacheNetworkServiceHandler class which was added in the last patchset. Additionally the corresponding methods to initialize the AppCache factory in the AppCacheRequestHandler class are also no longer needed. BUG=715632 Review-Url: https://codereview.chromium.org/2891773002 Cr-Commit-Position: refs/heads/master@{#473081} Committed: https://chromium.googlesource.com/chromium/src/+/2e65213dd03a7a0f277c5d16d460ae22d391f33c

Patch Set 1 #

Patch Set 2 : Remove includes and address presubmit #

Patch Set 3 : Fix compile failures #

Total comments: 28

Patch Set 4 : Address review comments #

Patch Set 5 : Call the network service if the origin is not in the usage map in the AppCacheStorage instance #

Total comments: 2

Patch Set 6 : Address review comments #

Patch Set 7 : Generate a request_id on the same lines as RDHI to prevent DCHECKs in metrics code which validate t… #

Total comments: 2

Patch Set 8 : Address final round of comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+298 lines, -218 lines) Patch
M content/browser/BUILD.gn View 2 chunks +2 lines, -2 lines 0 comments Download
D content/browser/appcache/appcache_network_service_handler.h View 1 chunk +0 lines, -83 lines 0 comments Download
D content/browser/appcache/appcache_network_service_handler.cc View 1 chunk +0 lines, -68 lines 0 comments Download
M content/browser/appcache/appcache_request_handler.h View 1 3 chunks +0 lines, -17 lines 0 comments Download
M content/browser/appcache/appcache_request_handler.cc View 2 chunks +0 lines, -18 lines 0 comments Download
A content/browser/appcache/appcache_url_loader_factory.h View 1 2 3 4 5 1 chunk +63 lines, -0 lines 0 comments Download
A content/browser/appcache/appcache_url_loader_factory.cc View 1 2 3 4 5 1 chunk +185 lines, -0 lines 0 comments Download
M content/browser/loader/navigation_url_loader_network_service.cc View 1 2 3 4 5 6 7 6 chunks +12 lines, -18 lines 0 comments Download
M content/browser/storage_partition_impl.cc View 1 2 3 1 chunk +1 line, -5 lines 0 comments Download
M content/browser/url_loader_factory_getter.h View 1 2 3 4 5 3 chunks +14 lines, -3 lines 0 comments Download
M content/browser/url_loader_factory_getter.cc View 1 2 3 4 5 2 chunks +21 lines, -4 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 35 (24 generated)
ananta
1 month, 1 week ago (2017-05-17 15:21:39 UTC) #2
jam
https://codereview.chromium.org/2891773002/diff/40001/content/browser/appcache/appcache_url_loader_factory.cc File content/browser/appcache/appcache_url_loader_factory.cc (right): https://codereview.chromium.org/2891773002/diff/40001/content/browser/appcache/appcache_url_loader_factory.cc#newcode89 content/browser/appcache/appcache_url_loader_factory.cc:89: mojo::MakeRequest(&network_loader_request_), routing_id_, request_id_, I think you want to call ...
1 month, 1 week ago (2017-05-17 16:51:12 UTC) #11
kinuko
Hi-- I want to sync-up before we proceed further, could we chat a bit today? ...
1 month, 1 week ago (2017-05-17 16:54:30 UTC) #13
ananta
https://codereview.chromium.org/2891773002/diff/40001/content/browser/appcache/appcache_url_loader_factory.cc File content/browser/appcache/appcache_url_loader_factory.cc (right): https://codereview.chromium.org/2891773002/diff/40001/content/browser/appcache/appcache_url_loader_factory.cc#newcode71 content/browser/appcache/appcache_url_loader_factory.cc:71: // The AppCacheNetworkServiceHandler instance is deleted on return from ...
1 month, 1 week ago (2017-05-17 18:14:09 UTC) #14
ananta
https://codereview.chromium.org/2891773002/diff/40001/content/browser/loader/navigation_url_loader_network_service.cc File content/browser/loader/navigation_url_loader_network_service.cc (right): https://codereview.chromium.org/2891773002/diff/40001/content/browser/loader/navigation_url_loader_network_service.cc#newcode104 content/browser/loader/navigation_url_loader_network_service.cc:104: if (appcache_handle_core) { On 2017/05/17 18:14:09, ananta wrote: > ...
1 month, 1 week ago (2017-05-17 20:45:27 UTC) #19
jam
https://codereview.chromium.org/2891773002/diff/40001/content/browser/appcache/appcache_url_loader_factory.cc File content/browser/appcache/appcache_url_loader_factory.cc (right): https://codereview.chromium.org/2891773002/diff/40001/content/browser/appcache/appcache_url_loader_factory.cc#newcode89 content/browser/appcache/appcache_url_loader_factory.cc:89: mojo::MakeRequest(&network_loader_request_), routing_id_, request_id_, On 2017/05/17 18:14:08, ananta wrote: > ...
1 month ago (2017-05-18 14:16:14 UTC) #24
ananta
https://codereview.chromium.org/2891773002/diff/40001/content/browser/appcache/appcache_url_loader_factory.h File content/browser/appcache/appcache_url_loader_factory.h (right): https://codereview.chromium.org/2891773002/diff/40001/content/browser/appcache/appcache_url_loader_factory.h#newcode21 content/browser/appcache/appcache_url_loader_factory.h:21: class CONTENT_EXPORT AppCacheURLLoaderFactory : public mojom::URLLoaderFactory { On 2017/05/18 ...
1 month ago (2017-05-18 19:13:52 UTC) #25
jam
lgtm with the one comment about request ID https://codereview.chromium.org/2891773002/diff/40001/content/browser/appcache/appcache_url_loader_factory.cc File content/browser/appcache/appcache_url_loader_factory.cc (right): https://codereview.chromium.org/2891773002/diff/40001/content/browser/appcache/appcache_url_loader_factory.cc#newcode89 content/browser/appcache/appcache_url_loader_factory.cc:89: mojo::MakeRequest(&network_loader_request_), ...
1 month ago (2017-05-19 00:03:51 UTC) #28
ananta
https://codereview.chromium.org/2891773002/diff/120001/content/browser/loader/navigation_url_loader_network_service.cc File content/browser/loader/navigation_url_loader_network_service.cc (right): https://codereview.chromium.org/2891773002/diff/120001/content/browser/loader/navigation_url_loader_network_service.cc#newcode185 content/browser/loader/navigation_url_loader_network_service.cc:185: request_id_--; On 2017/05/19 00:03:50, jam wrote: > this class ...
1 month ago (2017-05-19 00:23:13 UTC) #29
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/2891773002/140001
1 month ago (2017-05-19 00:24:35 UTC) #32
commit-bot: I haz the power
1 month ago (2017-05-19 04:08:51 UTC) #35
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/2e65213dd03a7a0f277c5d16d460...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318