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

Issue 538263002: Revert of This is a reland of ServiceWorkerCache::Keys. (Closed)

Created:
6 years, 3 months ago by jkarlin
Modified:
6 years, 3 months ago
Reviewers:
michaeln
CC:
chromium-reviews, jsbell+serviceworker_chromium.org, tzik, serviceworker-reviews, jam, kinuko+serviceworker, nhiroki, darin-cc_chromium.org, horo+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Revert of This is a reland of ServiceWorkerCache::Keys. (patchset #3 id:70001 of https://codereview.chromium.org/528233003/) Reason for revert: Now getting a Valgrind leak that is likely due to this CL: Suppression (error hash=#935FB14CBC8B5858#): For more info on using suppressions see http://dev.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/memory-sheriff#TOC-Suppressing-memory-reports { <insert_a_suppression_name_here> Memcheck:Leak fun:_Znw* fun:_ZN9__gnu_cxx13new_allocatorIN10disk_cache20SimpleEntryOperationEE8allocateEmPKv fun:_ZNSt11_Deque_baseIN10disk_cache20SimpleEntryOperationESaIS1_EE16_M_allocate_nodeEv fun:_ZNSt5dequeIN10disk_cache20SimpleEntryOperationESaIS1_EE16_M_push_back_auxIJS1_EEEvDpOT_ fun:_ZNSt5dequeIN10disk_cache20SimpleEntryOperationESaIS1_EE12emplace_backIJS1_EEEvDpOT_ fun:_ZNSt5dequeIN10disk_cache20SimpleEntryOperationESaIS1_EE9push_backEOS1_ fun:_ZNSt5queueIN10disk_cache20SimpleEntryOperationESt5dequeIS1_SaIS1_EEE4pushEOS1_ fun:_ZN10disk_cache15SimpleEntryImpl5CloseEv fun:_ZN10disk_cache12EntryDeleterclEPNS_5EntryE fun:_ZN4base8internal15scoped_ptr_implIN10disk_cache5EntryENS2_12EntryDeleterEED2Ev fun:_ZN10scoped_ptrIN10disk_cache5EntryENS0_12EntryDeleterEED2Ev } Original issue's description: > This is a reland of ServiceWorkerCache::Keys. > > Reland of https://codereview.chromium.org/477973002/ > > There was a memory leak. EndEnumeration wasn't called on the iterator. > > Fixed and tested with ASAN. > > The leak was detected because SimpleCache doesn't clean up its enumerators on deletion, but it should. See https://code.google.com/p/chromium/issues/detail?id=410276 > > BUG=392621 > > Committed: https://chromium.googlesource.com/chromium/src/+/a3a3703b91203db3e2248d656b86cb26bf2e30e3 TBR=michaeln@chromium.org NOTREECHECKS=true NOTRY=true BUG=392621 Committed: https://crrev.com/5b4dd4870897dfa9fe7e12e504f84d1e4dfb7302 Cr-Commit-Position: refs/heads/master@{#293373}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+60 lines, -334 lines) Patch
M content/browser/service_worker/service_worker_cache.h View 4 chunks +0 lines, -23 lines 0 comments Download
M content/browser/service_worker/service_worker_cache.cc View 11 chunks +38 lines, -213 lines 0 comments Download
M content/browser/service_worker/service_worker_cache_unittest.cc View 6 chunks +22 lines, -98 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
jkarlin
Created Revert of This is a reland of ServiceWorkerCache::Keys.
6 years, 3 months ago (2014-09-04 23:22:04 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jkarlin@chromium.org/538263002/1
6 years, 3 months ago (2014-09-04 23:24:02 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as 2375daae28b384210b51c4991941e89537a77317
6 years, 3 months ago (2014-09-04 23:28:47 UTC) #3
earthdok
On 2014/09/04 23:28:47, I haz the power (commit-bot) wrote: > Committed patchset #1 (id:1) as ...
6 years, 3 months ago (2014-09-09 17:41:03 UTC) #4
jkarlin
On 2014/09/09 17:41:03, earthdok wrote: > On 2014/09/04 23:28:47, I haz the power (commit-bot) wrote: ...
6 years, 3 months ago (2014-09-09 17:45:11 UTC) #5
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:33:56 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/5b4dd4870897dfa9fe7e12e504f84d1e4dfb7302
Cr-Commit-Position: refs/heads/master@{#293373}

Powered by Google App Engine
This is Rietveld 408576698