Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1918093003:
Encapsulate |MemoryCacheEntry::m_resource| and refactor pruneDead/LiveResources (Closed)
|
Created:
4 years, 8 months ago by hiroshige Modified:
4 years, 7 months ago Reviewers:
Nate Chapin CC:
chromium-reviews, blink-reviews, loading-reviews+fetch_chromium.org, tyoshino+watch_chromium.org, Nate Chapin, gavinp+loader_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@MemoryCache_1a Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionEncapsulate |MemoryCacheEntry::m_resource| and refactor pruneDead/LiveResources
This CL
- Make |MemoryCacheEntry::m_resource| private and always use MemoryCacheEntry::resource(),
- Removes somes ASSERT() because Issue 594644 is now fixed, and
- Continue pruning when we encounter entries evicted during destroyDecodedData().
BUG=603462, 594644
Committed: https://crrev.com/5ea6d3b8ee5b06fdfa4bc56767637680ca68af95
Cr-Commit-Position: refs/heads/master@{#393632}
Patch Set 1 #Patch Set 2 : #
Total comments: 3
Patch Set 3 : Reflect comment #Patch Set 4 : Rebase #Patch Set 5 : auto-Rebase #Patch Set 6 : Rebase #
Dependent Patchsets: Messages
Total messages: 41 (22 generated)
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1918093003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1918093003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Encapsulate |MemoryCacheEntry::m_resource| and refactor pruneDead/LiveResources BUG= ========== to ========== Encapsulate |MemoryCacheEntry::m_resource| and refactor pruneDead/LiveResources This CL - Make |MemoryCacheEntry::m_resource| private and always use MemoryCacheEntry::resource(), - Removes somes ASSERT() because Issue 594644 is now fixed, and - Continue pruning when we encounter entries evicted during destroyDecodedData(). This is preparation for https://codereview.chromium.org/1915113005/. BUG= ==========
https://codereview.chromium.org/1918093003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/fetch/MemoryCache.cpp (right): https://codereview.chromium.org/1918093003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/fetch/MemoryCache.cpp:309: if (!resource || !contains(resource)) { Previously we just |break| here, but this CL |continue|s because I think it is safe to continue and we'll have more chance of pruning. https://codereview.chromium.org/1918093003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/fetch/MemoryCache.cpp:332: current = previous; ditto.
Description was changed from ========== Encapsulate |MemoryCacheEntry::m_resource| and refactor pruneDead/LiveResources This CL - Make |MemoryCacheEntry::m_resource| private and always use MemoryCacheEntry::resource(), - Removes somes ASSERT() because Issue 594644 is now fixed, and - Continue pruning when we encounter entries evicted during destroyDecodedData(). This is preparation for https://codereview.chromium.org/1915113005/. BUG= ========== to ========== Encapsulate |MemoryCacheEntry::m_resource| and refactor pruneDead/LiveResources This CL - Make |MemoryCacheEntry::m_resource| private and always use MemoryCacheEntry::resource(), - Removes somes ASSERT() because Issue 594644 is now fixed, and - Continue pruning when we encounter entries evicted during destroyDecodedData(). BUG=603462, 594644 ==========
hiroshige@chromium.org changed reviewers: + japhet@chromium.org
PTAL. This is refactoring (with behavior change) and also preparation for https://codereview.chromium.org/1915113005/.
lgtm https://codereview.chromium.org/1918093003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/fetch/MemoryCache.h (right): https://codereview.chromium.org/1918093003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/fetch/MemoryCache.h:89: Member<Resource> m_resource; Nit: I think it's an unwritten convention that private member variables are placed below the private functions?
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1918093003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1918093003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1918093003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1918093003/40001
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 hiroshige@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org Link to the patchset: https://codereview.chromium.org/1918093003/#ps40001 (title: "Reflect comment")
The CQ bit was unchecked by commit-bot@chromium.org
This CL has an open dependency (Issue 1807323002 Patch 380001). Please resolve the dependency and try again. If you are sure that there is no real dependency, please use one of the options listed in https://goo.gl/9Es4OR to land the CL.
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1918093003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1918093003/60001
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 hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1918093003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1918093003/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1918093003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1918093003/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by hiroshige@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org Link to the patchset: https://codereview.chromium.org/1918093003/#ps100001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1918093003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1918093003/100001
Message was sent while issue was closed.
Description was changed from ========== Encapsulate |MemoryCacheEntry::m_resource| and refactor pruneDead/LiveResources This CL - Make |MemoryCacheEntry::m_resource| private and always use MemoryCacheEntry::resource(), - Removes somes ASSERT() because Issue 594644 is now fixed, and - Continue pruning when we encounter entries evicted during destroyDecodedData(). BUG=603462, 594644 ========== to ========== Encapsulate |MemoryCacheEntry::m_resource| and refactor pruneDead/LiveResources This CL - Make |MemoryCacheEntry::m_resource| private and always use MemoryCacheEntry::resource(), - Removes somes ASSERT() because Issue 594644 is now fixed, and - Continue pruning when we encounter entries evicted during destroyDecodedData(). BUG=603462, 594644 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Encapsulate |MemoryCacheEntry::m_resource| and refactor pruneDead/LiveResources This CL - Make |MemoryCacheEntry::m_resource| private and always use MemoryCacheEntry::resource(), - Removes somes ASSERT() because Issue 594644 is now fixed, and - Continue pruning when we encounter entries evicted during destroyDecodedData(). BUG=603462, 594644 ========== to ========== Encapsulate |MemoryCacheEntry::m_resource| and refactor pruneDead/LiveResources This CL - Make |MemoryCacheEntry::m_resource| private and always use MemoryCacheEntry::resource(), - Removes somes ASSERT() because Issue 594644 is now fixed, and - Continue pruning when we encounter entries evicted during destroyDecodedData(). BUG=603462, 594644 Committed: https://crrev.com/5ea6d3b8ee5b06fdfa4bc56767637680ca68af95 Cr-Commit-Position: refs/heads/master@{#393632} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/5ea6d3b8ee5b06fdfa4bc56767637680ca68af95 Cr-Commit-Position: refs/heads/master@{#393632} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
