Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 1432563002: Enable 'Browsing data volume counters on Desktop' by default. (Closed)

Created:
4 years, 8 months ago by msramek
Modified:
4 years, 7 months ago
CC:
chromium-reviews, dbeam+watch-options_chromium.org, michaelpg+watch-options_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Enable 'Browsing data volume counters on Desktop' by default. This was approved for the Dev channel in crbug.com/548620. BUG=548620 Committed: https://crrev.com/b3f105f17cb6ff224dc8892f2bde7b4cb4b24388 Cr-Commit-Position: refs/heads/master@{#358617}

Patch Set 1 #

Patch Set 2 : Fixed histograms.xml. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -5 lines) Patch
M chrome/browser/about_flags.cc View 1 chunk +2 lines, -1 line 2 comments Download
M chrome/browser/ui/webui/options/clear_browser_data_handler.cc View 1 chunk +12 lines, -2 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 2 chunks +5 lines, -2 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 21 (7 generated)
msramek
Hi Dan, can you please have a look at this? Normally I send these to ...
4 years, 8 months ago (2015-11-03 10:11:33 UTC) #2
msramek
Ah, tests were failing because I didn't update histograms.xml. Dan, can you PTAL? +Ilya, please ...
4 years, 8 months ago (2015-11-04 18:20:03 UTC) #4
Ilya Sherman
histograms.xml lgtm
4 years, 8 months ago (2015-11-04 23:07:15 UTC) #5
msramek
Dan, friendly ping :-) I'd like this to be enabled ASAP, so that it spends ...
4 years, 8 months ago (2015-11-06 17:00:47 UTC) #6
msramek
...alternatively, Steven, can you please have a look? In case Dan is OOO or something ...
4 years, 8 months ago (2015-11-09 17:25:27 UTC) #8
stevenjb
lgtm
4 years, 8 months ago (2015-11-09 17:33:41 UTC) #11
msramek
Thanks!
4 years, 8 months ago (2015-11-09 17:39:01 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1432563002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1432563002/20001
4 years, 8 months ago (2015-11-09 17:39:26 UTC) #14
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 8 months ago (2015-11-09 18:57:50 UTC) #15
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/b3f105f17cb6ff224dc8892f2bde7b4cb4b24388 Cr-Commit-Position: refs/heads/master@{#358617}
4 years, 8 months ago (2015-11-09 18:59:34 UTC) #16
Peter Mayo
On 2015/11/09 18:59:34, commit-bot: I haz the power wrote: > Patchset 2 (id:??) landed as ...
4 years, 8 months ago (2015-11-09 21:43:54 UTC) #17
Peter Mayo
https://codereview.chromium.org/1432563002/diff/20001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/1432563002/diff/20001/chrome/browser/about_flags.cc#newcode2008 chrome/browser/about_flags.cc:2008: switches::kDisableClearBrowsingDataCounters) This includes OS_CHROMEOS. It doesn't seem like the ...
4 years, 8 months ago (2015-11-09 21:46:29 UTC) #19
Peter Mayo
Reverting at https://codereview.chromium.org/1423373006/
4 years, 8 months ago (2015-11-09 22:11:06 UTC) #20
msramek
4 years, 8 months ago (2015-11-09 22:48:04 UTC) #21
Message was sent while issue was closed.
https://codereview.chromium.org/1432563002/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/1432563002/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:2008: switches::kDisableClearBrowsingDataCounters)
On 2015/11/09 21:46:29, Peter Mayo wrote:
> This includes OS_CHROMEOS.
> 
> It doesn't seem like the tests stay happy when it does.
> Please fix the code or tests appropriately.

Hm, I see the problem in HistoryCounter. It's not caused by this CL, but it is
exposed by it. I'll fix it tomorrow and reland this. Sorry for the trouble!

Powered by Google App Engine
This is Rietveld 408576698