Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2454983002:
Cache-aware Resource loading (Closed)
|
Created:
4 years, 1 month ago by Shao-Chuan Lee Modified:
4 years, 1 month ago CC:
chromium-reviews, tyoshino+watch_chromium.org, Yoav Weiss, gavinp+loader_chromium.org, blink-reviews, loading-reviews+fetch_chromium.org, Nate Chapin Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionCache-aware Resource loading
If cache-aware loading is enabled, the Resource is notified of disk cache miss
from network stack during loading. This is implemented by sending a request to
load only from disk cache, and retry with original request if the first one
fails. Resource is notified of the cache miss through callback
willReloadAfterDiskCacheMiss() before the retry.
Now we provide an option in ResourceLoaderOptions to enable cache-aware loading
through FetchRequest. However there are cases that cache-aware loading is not
actually activated:
- The request specifies loading synchronously
- The request is for Resource revalidation
- Cache policy is already explicitly set
The user may determine whether cache-aware loading is actually activated with
ResourceLoader::isCacheAwareLoadingActivated().
Redirect responses from disk cache are not supported, we will fail the request
as cache miss and retry.
This feature will be used in webfonts cache-aware timeout adaptation, which
changes webfont display behavior according to whether the webfont is already on
disk cache or will be loaded from network. See
https://codereview.chromium.org/2390583002/ for details.
BUG=570205
Committed: https://crrev.com/27405ff42bf94769bb5310a217957551364ee061
Cr-Commit-Position: refs/heads/master@{#430891}
Patch Set 1 #
Total comments: 2
Patch Set 2 : fix #3, add DCHECK, add missing header #
Total comments: 5
Patch Set 3 : move deactivation from fetcher to loader #Patch Set 4 : rebase, use new flag #Patch Set 5 : rebase #Patch Set 6 : don't call willReloadAfterDiskCacheMiss() if not activated #Patch Set 7 : move options check into loader, rename & comments #
Total comments: 1
Patch Set 8 : rebase #Patch Set 9 : check synchronous policy #
Total comments: 4
Patch Set 10 : comment on Resource callback #
Total comments: 2
Patch Set 11 : copy ResourceRequest only on CAL activation #
Total comments: 3
Patch Set 12 : nit #
Dependent Patchsets: Messages
Total messages: 57 (32 generated)
shaochuan@chromium.org changed reviewers: + hiroshige@chromium.org, yhirano@chromium.org
PTAL Still waiting for https://codereview.chromium.org/2398613002/, will have a separate CL for adding new WebCachePolicy.
https://codereview.chromium.org/2454983002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/fetch/ResourceLoader.h (right): https://codereview.chromium.org/2454983002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/fetch/ResourceLoader.h:70: bool willActivateCacheAwareLoading(const ResourceRequest&); activateCacheAwareLoading might be a better name. You can write a comment here such as // Activates the cache aware loading and returns true. Returns false // and does nothing when it should not be activated.
https://codereview.chromium.org/2454983002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/fetch/ResourceLoader.h (right): https://codereview.chromium.org/2454983002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/fetch/ResourceLoader.h:70: bool willActivateCacheAwareLoading(const ResourceRequest&); On 2016/10/27 07:33:00, yhirano wrote: > activateCacheAwareLoading might be a better name. You can write a comment here > such as > // Activates the cache aware loading and returns true. Returns false > // and does nothing when it should not be activated. Done.
lgtm
https://codereview.chromium.org/2454983002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right): https://codereview.chromium.org/2454983002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:1184: } Can we move this block to ResourceLoader::didFail() and make ResourceLoader::deactivateCacheAwareLoading() private? (Probably we can also make ResourceLoader::isCacheAwareLoadingActivated() private) https://codereview.chromium.org/2454983002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:1324: } Can we move this block to ResourceLoader::start() and make activateCacheAwareLoading() private?
https://codereview.chromium.org/2454983002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right): https://codereview.chromium.org/2454983002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:1184: } On 2016/10/27 08:21:48, hiroshige wrote: > Can we move this block to ResourceLoader::didFail() and make > ResourceLoader::deactivateCacheAwareLoading() private? > (Probably we can also make ResourceLoader::isCacheAwareLoadingActivated() > private) Done. As for isCacheAwareLoadingActivated() I'm planning to have it exposed to Resource and ResourceClient. https://codereview.chromium.org/2454983002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:1324: } On 2016/10/27 08:21:48, hiroshige wrote: > Can we move this block to ResourceLoader::start() and make > activateCacheAwareLoading() private? The activation should only occur in startLoad(), if put in ResourceLoader::start() it cannot determine whether it's called from startLoad() or restart().
lgtm. (Also adding unit tests is preferable, but it will require e.g. modest modification to WebURLLoaderMock etc. so I think it shouldn't block this issue) https://codereview.chromium.org/2454983002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right): https://codereview.chromium.org/2454983002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:1324: } On 2016/10/28 04:14:14, Shao-Chuan Lee wrote: > On 2016/10/27 08:21:48, hiroshige wrote: > > Can we move this block to ResourceLoader::start() and make > > activateCacheAwareLoading() private? > > The activation should only occur in startLoad(), if put in > ResourceLoader::start() it cannot determine whether it's called from startLoad() > or restart(). Makes sense. There might be a room for refactoring but it should not block this issue.
On 2016/10/31 08:25:19, hiroshige wrote: > lgtm. > > (Also adding unit tests is preferable, but it will require e.g. modest > modification to WebURLLoaderMock etc. so I think it shouldn't block this issue) Actually I'm having unit tests for FontResource because it's easier to have a mock FontResourceClient, I guess that would be sufficient.
The CQ bit was checked by shaochuan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Cache-aware Resource loading If cache-aware loading is enabled, the Resource is notified of disk cache miss from network stack during loading. This is implemented by sending a request to load only from disk cache, and retry with original request if the first one fails. Resource is notified of the cache miss through callback willReloadAfterDiskCacheMiss() before the retry. Now we provide an option in ResourceLoaderOptions to enable cache-aware loading through FetchRequest. However there are cases that cache-aware loading is not actually activated: - The request is for Resource revalidation - Cache policy is already explicitly set In such cases willReloadAfterDiskCacheMiss() is called immediately before sending the request. Redirect responses from disk cache are not supported, we will fail the request as cache miss and retry. This feature will be used in webfonts cache-aware timeout adaptation, which changes webfont display behavior according to whether the webfont is already on disk cache or will be loaded from network. See https://codereview.chromium.org/2390583002/ for details. BUG=570205 ========== to ========== Cache-aware Resource loading If cache-aware loading is enabled, the Resource is notified of disk cache miss from network stack during loading. This is implemented by sending a request to load only from disk cache, and retry with original request if the first one fails. Resource is notified of the cache miss through callback willReloadAfterDiskCacheMiss() before the retry. Now we provide an option in ResourceLoaderOptions to enable cache-aware loading through FetchRequest. However there are cases that cache-aware loading is not actually activated: - The request is for Resource revalidation - Cache policy is already explicitly set The user may determine whether cache-aware loading is actually activated with ResourceLoader::isCacheAwareLoadingActivated(). Redirect responses from disk cache are not supported, we will fail the request as cache miss and retry. This feature will be used in webfonts cache-aware timeout adaptation, which changes webfont display behavior according to whether the webfont is already on disk cache or will be loaded from network. See https://codereview.chromium.org/2390583002/ for details. BUG=570205 ==========
The CQ bit was checked by shaochuan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Minor design change: now the cache miss callback is not called if CAL cannot be activated, the user may check if activated by themselves through ResourceLoader.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
- Move ResourceLoaderOptions check into ResourceLoader
- Rename activateCacheAwareLoading{,IfNeeded}
https://codereview.chromium.org/2454983002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceLoader.h (right):
https://codereview.chromium.org/2454983002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceLoader.h:73: void
activateCacheAwareLoadingIfNeeded(const ResourceRequest&);
yhirano@: I'm adding "IfNeeded" suffix otherwise it would be confusing when
called in ResourceFetcher, WDYT?
The CQ bit was checked by shaochuan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by shaochuan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by shaochuan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Ping if you have further comments since PS5. This CL is ready for submission.
The CQ bit was checked by shaochuan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
drive-by nit comment https://codereview.chromium.org/2454983002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/fetch/Resource.h (right): https://codereview.chromium.org/2454983002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/fetch/Resource.h:315: virtual void willReloadAfterDiskCacheMiss() {} nit/optional: This method naming feels a little weird to me, because 1) the willDoFoo() naming pattern is often used to notify observers right before doFoo() is called while we're not in that pattern in this change, and 2) the method name seems to imply very specific action (i.e. reload) to be taken while the default impl does nothing. I slightly prefer having a more generic name in the base class like: didDetectCacheMiss() and have a comment to note, say, 'subclasses (e.g. FontResource) may trigger reloading to actually fetch the resource from network'. Wdyt..?
https://codereview.chromium.org/2454983002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/fetch/Resource.h (right): https://codereview.chromium.org/2454983002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/fetch/Resource.h:315: virtual void willReloadAfterDiskCacheMiss() {} On 2016/11/08 06:10:18, kinuko wrote: > nit/optional: > > This method naming feels a little weird to me, because 1) the willDoFoo() naming > pattern is often used to notify observers right before doFoo() is called while > we're not in that pattern in this change, and 2) the method name seems to imply > very specific action (i.e. reload) to be taken while the default impl does > nothing. > > I slightly prefer having a more generic name in the base class like: > didDetectCacheMiss() > > and have a comment to note, say, 'subclasses (e.g. FontResource) may trigger > reloading to actually fetch the resource from network'. Wdyt..? SGTM, but I wonder if it's better to mention "disk" cache miss to distinguish from memory cache miss? Naming is hard... The reload is actually triggered by ResourceLoader, but it's indeed nice to have comments here.
kinuko@chromium.org changed reviewers: + kinuko@chromium.org
https://codereview.chromium.org/2454983002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/fetch/Resource.h (right): https://codereview.chromium.org/2454983002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/fetch/Resource.h:315: virtual void willReloadAfterDiskCacheMiss() {} On 2016/11/08 06:28:01, Shao-Chuan Lee wrote: > On 2016/11/08 06:10:18, kinuko wrote: > > nit/optional: > > > > This method naming feels a little weird to me, because 1) the willDoFoo() > naming > > pattern is often used to notify observers right before doFoo() is called while > > we're not in that pattern in this change, and 2) the method name seems to > imply > > very specific action (i.e. reload) to be taken while the default impl does > > nothing. > > > > I slightly prefer having a more generic name in the base class like: > > didDetectCacheMiss() > > > > and have a comment to note, say, 'subclasses (e.g. FontResource) may trigger > > reloading to actually fetch the resource from network'. Wdyt..? > > SGTM, but I wonder if it's better to mention "disk" cache miss to distinguish > from memory cache miss? Naming is hard... > > The reload is actually triggered by ResourceLoader, but it's indeed nice to have > comments here. Oh that's true, so maybe the current name is just ok. Could we still slightly improve the comment to note that reload is triggered right after this method by ResourceLoader then?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2454983002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/fetch/Resource.h (right): https://codereview.chromium.org/2454983002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/fetch/Resource.h:315: virtual void willReloadAfterDiskCacheMiss() {} On 2016/11/08 06:42:15, kinuko wrote: > On 2016/11/08 06:28:01, Shao-Chuan Lee wrote: > > On 2016/11/08 06:10:18, kinuko wrote: > > > nit/optional: > > > > > > This method naming feels a little weird to me, because 1) the willDoFoo() > > naming > > > pattern is often used to notify observers right before doFoo() is called > while > > > we're not in that pattern in this change, and 2) the method name seems to > > imply > > > very specific action (i.e. reload) to be taken while the default impl does > > > nothing. > > > > > > I slightly prefer having a more generic name in the base class like: > > > didDetectCacheMiss() > > > > > > and have a comment to note, say, 'subclasses (e.g. FontResource) may trigger > > > reloading to actually fetch the resource from network'. Wdyt..? > > > > SGTM, but I wonder if it's better to mention "disk" cache miss to distinguish > > from memory cache miss? Naming is hard... > > > > The reload is actually triggered by ResourceLoader, but it's indeed nice to > have > > comments here. > > Oh that's true, so maybe the current name is just ok. Could we still slightly > improve the comment to note that reload is triggered right after this method by > ResourceLoader then? Done, PTAL
still lgtm
looking good, I have one more question (though no need to be blocked by my comment) https://codereview.chromium.org/2454983002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/fetch/ResourceLoader.cpp (right): https://codereview.chromium.org/2454983002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:87: ResourceRequest request(originalRequest); Always making an extra copy here seems a bit unfortunate. Wondering if this method should just take the request by value? Given that it's activated only for async if WrappedResourceRequest supported overriding fields it might have been useful?
Comments are welcomed! Thanks for looking into this. https://codereview.chromium.org/2454983002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/fetch/ResourceLoader.cpp (right): https://codereview.chromium.org/2454983002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:87: ResourceRequest request(originalRequest); On 2016/11/08 12:21:55, kinuko wrote: > Always making an extra copy here seems a bit unfortunate. Wondering if this > method should just take the request by value? Given that it's activated only > for async if WrappedResourceRequest supported overriding fields it might have > been useful? I thought of using pointers but ResourceRequest has to be allocated on stack. Now I moved the copy into a separate if block below to avoid copies in most cases.
On 2016/11/09 03:48:01, Shao-Chuan Lee wrote: > Comments are welcomed! Thanks for looking into this. > > https://codereview.chromium.org/2454983002/diff/180001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/fetch/ResourceLoader.cpp (right): > > https://codereview.chromium.org/2454983002/diff/180001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:87: ResourceRequest > request(originalRequest); > On 2016/11/08 12:21:55, kinuko wrote: > > Always making an extra copy here seems a bit unfortunate. Wondering if this > > method should just take the request by value? Given that it's activated only > > for async if WrappedResourceRequest supported overriding fields it might have > > been useful? > > I thought of using pointers but ResourceRequest has to be allocated on stack. > Now I moved the copy into a separate if block below to avoid copies in most > cases. Thanks, I like the new code better!
two more nits (and lgtm) https://codereview.chromium.org/2454983002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/fetch/ResourceLoader.cpp (right): https://codereview.chromium.org/2454983002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:90: if (m_resource->options().synchronousPolicy == RequestSynchronously) { nit: maybe should we assert m_isCacheAwareLoadingActivated is false here? https://codereview.chromium.org/2454983002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:98: this); nit: we can remove else's from this if-elseif-else by returning early in each block
Patch Set 11 still lgtm.
https://codereview.chromium.org/2454983002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/fetch/ResourceLoader.cpp (right): https://codereview.chromium.org/2454983002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:90: if (m_resource->options().synchronousPolicy == RequestSynchronously) { On 2016/11/09 04:34:24, kinuko wrote: > nit: maybe should we assert m_isCacheAwareLoadingActivated is false here? Now the CAL check is moved before this and returns early, then we don't have to assert here. CAL is only activated with RequestAsynchronously. The diff is also cleaner now.
The CQ bit was checked by shaochuan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yhirano@chromium.org, hiroshige@chromium.org, kinuko@chromium.org Link to the patchset: https://codereview.chromium.org/2454983002/#ps220001 (title: "nit")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Cache-aware Resource loading If cache-aware loading is enabled, the Resource is notified of disk cache miss from network stack during loading. This is implemented by sending a request to load only from disk cache, and retry with original request if the first one fails. Resource is notified of the cache miss through callback willReloadAfterDiskCacheMiss() before the retry. Now we provide an option in ResourceLoaderOptions to enable cache-aware loading through FetchRequest. However there are cases that cache-aware loading is not actually activated: - The request is for Resource revalidation - Cache policy is already explicitly set The user may determine whether cache-aware loading is actually activated with ResourceLoader::isCacheAwareLoadingActivated(). Redirect responses from disk cache are not supported, we will fail the request as cache miss and retry. This feature will be used in webfonts cache-aware timeout adaptation, which changes webfont display behavior according to whether the webfont is already on disk cache or will be loaded from network. See https://codereview.chromium.org/2390583002/ for details. BUG=570205 ========== to ========== Cache-aware Resource loading If cache-aware loading is enabled, the Resource is notified of disk cache miss from network stack during loading. This is implemented by sending a request to load only from disk cache, and retry with original request if the first one fails. Resource is notified of the cache miss through callback willReloadAfterDiskCacheMiss() before the retry. Now we provide an option in ResourceLoaderOptions to enable cache-aware loading through FetchRequest. However there are cases that cache-aware loading is not actually activated: - The request specifies loading synchronously - The request is for Resource revalidation - Cache policy is already explicitly set The user may determine whether cache-aware loading is actually activated with ResourceLoader::isCacheAwareLoadingActivated(). Redirect responses from disk cache are not supported, we will fail the request as cache miss and retry. This feature will be used in webfonts cache-aware timeout adaptation, which changes webfont display behavior according to whether the webfont is already on disk cache or will be loaded from network. See https://codereview.chromium.org/2390583002/ for details. BUG=570205 ==========
Message was sent while issue was closed.
Description was changed from ========== Cache-aware Resource loading If cache-aware loading is enabled, the Resource is notified of disk cache miss from network stack during loading. This is implemented by sending a request to load only from disk cache, and retry with original request if the first one fails. Resource is notified of the cache miss through callback willReloadAfterDiskCacheMiss() before the retry. Now we provide an option in ResourceLoaderOptions to enable cache-aware loading through FetchRequest. However there are cases that cache-aware loading is not actually activated: - The request specifies loading synchronously - The request is for Resource revalidation - Cache policy is already explicitly set The user may determine whether cache-aware loading is actually activated with ResourceLoader::isCacheAwareLoadingActivated(). Redirect responses from disk cache are not supported, we will fail the request as cache miss and retry. This feature will be used in webfonts cache-aware timeout adaptation, which changes webfont display behavior according to whether the webfont is already on disk cache or will be loaded from network. See https://codereview.chromium.org/2390583002/ for details. BUG=570205 ========== to ========== Cache-aware Resource loading If cache-aware loading is enabled, the Resource is notified of disk cache miss from network stack during loading. This is implemented by sending a request to load only from disk cache, and retry with original request if the first one fails. Resource is notified of the cache miss through callback willReloadAfterDiskCacheMiss() before the retry. Now we provide an option in ResourceLoaderOptions to enable cache-aware loading through FetchRequest. However there are cases that cache-aware loading is not actually activated: - The request specifies loading synchronously - The request is for Resource revalidation - Cache policy is already explicitly set The user may determine whether cache-aware loading is actually activated with ResourceLoader::isCacheAwareLoadingActivated(). Redirect responses from disk cache are not supported, we will fail the request as cache miss and retry. This feature will be used in webfonts cache-aware timeout adaptation, which changes webfont display behavior according to whether the webfont is already on disk cache or will be loaded from network. See https://codereview.chromium.org/2390583002/ for details. BUG=570205 ==========
Message was sent while issue was closed.
Committed patchset #12 (id:220001)
Message was sent while issue was closed.
Description was changed from ========== Cache-aware Resource loading If cache-aware loading is enabled, the Resource is notified of disk cache miss from network stack during loading. This is implemented by sending a request to load only from disk cache, and retry with original request if the first one fails. Resource is notified of the cache miss through callback willReloadAfterDiskCacheMiss() before the retry. Now we provide an option in ResourceLoaderOptions to enable cache-aware loading through FetchRequest. However there are cases that cache-aware loading is not actually activated: - The request specifies loading synchronously - The request is for Resource revalidation - Cache policy is already explicitly set The user may determine whether cache-aware loading is actually activated with ResourceLoader::isCacheAwareLoadingActivated(). Redirect responses from disk cache are not supported, we will fail the request as cache miss and retry. This feature will be used in webfonts cache-aware timeout adaptation, which changes webfont display behavior according to whether the webfont is already on disk cache or will be loaded from network. See https://codereview.chromium.org/2390583002/ for details. BUG=570205 ========== to ========== Cache-aware Resource loading If cache-aware loading is enabled, the Resource is notified of disk cache miss from network stack during loading. This is implemented by sending a request to load only from disk cache, and retry with original request if the first one fails. Resource is notified of the cache miss through callback willReloadAfterDiskCacheMiss() before the retry. Now we provide an option in ResourceLoaderOptions to enable cache-aware loading through FetchRequest. However there are cases that cache-aware loading is not actually activated: - The request specifies loading synchronously - The request is for Resource revalidation - Cache policy is already explicitly set The user may determine whether cache-aware loading is actually activated with ResourceLoader::isCacheAwareLoadingActivated(). Redirect responses from disk cache are not supported, we will fail the request as cache miss and retry. This feature will be used in webfonts cache-aware timeout adaptation, which changes webfont display behavior according to whether the webfont is already on disk cache or will be loaded from network. See https://codereview.chromium.org/2390583002/ for details. BUG=570205 Committed: https://crrev.com/27405ff42bf94769bb5310a217957551364ee061 Cr-Commit-Position: refs/heads/master@{#430891} ==========
Message was sent while issue was closed.
Patchset 12 (id:??) landed as https://crrev.com/27405ff42bf94769bb5310a217957551364ee061 Cr-Commit-Position: refs/heads/master@{#430891} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
