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

Issue 277133002: Revert of Resubmit: Add real file for AdNetworks DB (Closed)

Created:
6 years, 7 months ago by Alpha Left Google
Modified:
6 years, 7 months ago
Reviewers:
James Hawkins, Devlin, felt
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, felt, arv+watch_chromium.org, extensions-reviews_chromium.org
Visibility:
Public.

Description

Revert of Resubmit: Add real file for AdNetworks DB (https://codereview.chromium.org/274563003/) Reason for revert: Linux Asan is failing with this patch: http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Tests%20%282%29/builds/2483/steps/unit_tests/logs/stdio Original issue's description: > Resubmit: Add real file for AdNetworks DB > > This is a resubmit of https://codereview.chromium.org/268063004/. > > The previous version was reverted because it caused a memory leak. This was a > problem with RefCounting from ui::ResourceBundle::LoadDataResourceBytes(), which > has been fixed. > > Details: > Even though the method returns a non-refcounted ptr to a RefCountedStaticMemory > (which according to the class comment, "the ref counting does not matter" [1]), > the ref-counting _does_ matter. The fix for this went in as part of a refactor > in https://codereview.chromium.org/263953003/. > > [1] http://src.chromium.org/viewvc/chrome/trunk/src/base/memory/ref_counted_memory.h?revision=267321 line 44 > > BUG=357204 > > TBR=jhawkins@chromium.org (previously approved) > TBR=felt@chromium.org (very minor changes) > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269494 TBR=jhawkins@chromium.org,felt@chromium.org,rdevlin.cronin@chromium.org NOTREECHECKS=true NOTRY=true BUG=357204 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269512 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269535

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -39 lines) Patch
M chrome/browser/extensions/activity_log/hashed_ad_network_database.h View 1 chunk +0 lines, -7 lines 0 comments Download
M chrome/browser/extensions/activity_log/hashed_ad_network_database.cc View 2 chunks +1 line, -4 lines 0 comments Download
M chrome/browser/extensions/activity_log/hashed_ad_network_database_unittest.cc View 4 chunks +0 lines, -28 lines 0 comments Download
M chrome/browser/resources/ad_networks.dat View Binary file 0 comments Download

Messages

Total messages: 4 (0 generated)
Alpha Left Google
Created Revert of Resubmit: Add real file for AdNetworks DB
6 years, 7 months ago (2014-05-10 03:18:49 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hclam@chromium.org/277133002/1
6 years, 7 months ago (2014-05-10 03:20:51 UTC) #2
commit-bot: I haz the power
Change committed as 269512
6 years, 7 months ago (2014-05-10 03:24:31 UTC) #3
Alpha Left Google
6 years, 7 months ago (2014-05-10 05:08:23 UTC) #4
Message was sent while issue was closed.
Committed patchset #1 manually as r269535 (tree was closed).

Powered by Google App Engine
This is Rietveld 408576698