Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2509533002:
Do not add a Resource with DoNotBufferData policy to the memory cache (Closed)
|
Created:
4 years, 1 month ago by yhirano Modified:
4 years, 1 month ago Reviewers:
hiroshige 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. |
DescriptionDo not add a Resource with DoNotBufferData policy to the memory cache
BUG=659789
Committed: https://crrev.com/0b83d55ccf3742ac2f849c33c3db796230d9cf9c
Cr-Commit-Position: refs/heads/master@{#432448}
Patch Set 1 #
Total comments: 4
Patch Set 2 : fix #Messages
Total messages: 22 (16 generated)
The CQ bit was checked by yhirano@chromium.org to run a CQ dry run
Description was changed from ========== Do not add a Resource to MemoryCache if the buffering policy is not cache-friendly BUG=570205 ========== to ========== Do not add a Resource to MemoryCache if the buffering policy is not cache-friendly BUG=659789 ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Do not add a Resource to MemoryCache if the buffering policy is not cache-friendly BUG=659789 ========== to ========== Do not add a Resource to the memory cache if the buffering policy is not cache-friendly BUG=659789 ==========
Description was changed from ========== Do not add a Resource to the memory cache if the buffering policy is not cache-friendly BUG=659789 ========== to ========== Do not add a Resource with DoNotBufferData policy to the memory cache BUG=659789 ==========
yhirano@chromium.org changed reviewers: + hiroshige@chromium.org
hiroshige@, what do you think about this change? See https://codereview.chromium.org/2460573002/ for the background.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm with nits. https://codereview.chromium.org/2509533002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right): https://codereview.chromium.org/2509533002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:747: // Don't add main resource to cache to prevent reuse. Please update the comment. https://codereview.chromium.org/2509533002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:749: request.options().dataBufferingPolicy == BufferData) optional: |request.options().dataBufferingPolicy != DoNotBufferData| might be easier to read ("Add to memory cache, unless MainResource or DoNotBufferData").
The CQ bit was checked by yhirano@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...
https://codereview.chromium.org/2509533002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right): https://codereview.chromium.org/2509533002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:747: // Don't add main resource to cache to prevent reuse. On 2016/11/16 06:34:58, hiroshige wrote: > Please update the comment. Done. https://codereview.chromium.org/2509533002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:749: request.options().dataBufferingPolicy == BufferData) On 2016/11/16 06:34:58, hiroshige wrote: > optional: |request.options().dataBufferingPolicy != DoNotBufferData| might be > easier to read ("Add to memory cache, unless MainResource or DoNotBufferData"). Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by yhirano@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hiroshige@chromium.org Link to the patchset: https://codereview.chromium.org/2509533002/#ps20001 (title: "fix")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Do not add a Resource with DoNotBufferData policy to the memory cache BUG=659789 ========== to ========== Do not add a Resource with DoNotBufferData policy to the memory cache BUG=659789 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Do not add a Resource with DoNotBufferData policy to the memory cache BUG=659789 ========== to ========== Do not add a Resource with DoNotBufferData policy to the memory cache BUG=659789 Committed: https://crrev.com/0b83d55ccf3742ac2f849c33c3db796230d9cf9c Cr-Commit-Position: refs/heads/master@{#432448} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/0b83d55ccf3742ac2f849c33c3db796230d9cf9c Cr-Commit-Position: refs/heads/master@{#432448} |
