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

Issue 2275953002: Prerender: Remove deprecated prerender experiment histograms. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 months ago by mattcary (OOO July 14-26)
Modified:
10 months, 3 weeks ago
CC:
chromium-reviews, shishir+watch_chromium.org, gavinp+prer_chromium.org, cbentzel+watch_chromium.org, tburkard+watch_chromium.org, asvitkine+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Prerender: Remove deprecated prerender experiment histograms. This only removes the suffixing done for initial prerender experiments using the prerender mode. As a side effect, this effectively resets all prerender histograms as all current histograms currently reported are "_Enabled", and this change drops the _Enabled suffix and so renames existing histograms. However, all of those _Enabled histograms seem to be empty currently. Possibly some histograms should just be removed? BUG= Committed: https://crrev.com/81efc02ef67475e403bc5f6c4a79446eda627423 Cr-Commit-Position: refs/heads/master@{#414997}

Patch Set 1 #

Total comments: 5

Patch Set 2 : Drop histogram.xml changes #

Patch Set 3 : mor deprecations #

Total comments: 1

Patch Set 4 : don't deprecate all the histograms #

Total comments: 8

Patch Set 5 : trim a few more unsued ones #

Unified diffs Side-by-side diffs Delta from patch set Stats (+276 lines, -261 lines) Patch
M chrome/browser/predictors/autocomplete_action_predictor.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/prerender/prerender_histograms.cc View 2 chunks +2 lines, -4 lines 0 comments Download
M chrome/browser/prerender/prerender_manager.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/prerender/prerender_manager.cc View 1 chunk +0 lines, -26 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 4 chunks +273 lines, -228 lines 0 comments Download
Trybot results:  win_chromium_rel_ng   chromium_presubmit   win_chromium_rel_ng   win_clang   win_chromium_compile_dbg_ng   win_chromium_x64_rel_ng   win_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-device   ios-simulator   mac_chromium_rel_ng   linux_android_rel_ng   android_clang_dbg_recipe   cast_shell_android   android_arm64_dbg_recipe   android_compile_dbg   linux_chromium_chromeos_compile_dbg_ng   linux_chromium_asan_rel_ng   linux_chromium_rel_ng   chromeos_x86-generic_chromium_compile_only_ng   chromeos_amd64-generic_chromium_compile_only_ng   linux_chromium_compile_dbg_ng   chromium_presubmit   cast_shell_linux   chromeos_daisy_chromium_compile_only_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_chromeos_rel_ng   linux_chromium_clobber_rel_ng   chromium_presubmit   win_clang   win_chromium_compile_dbg_ng   win_chromium_rel_ng   win_chromium_x64_rel_ng   mac_chromium_compile_dbg_ng   ios-device   mac_chromium_rel_ng   ios-simulator   linux_android_rel_ng   android_clang_dbg_recipe   cast_shell_android   android_arm64_dbg_recipe   android_compile_dbg   linux_chromium_chromeos_compile_dbg_ng   linux_chromium_asan_rel_ng   linux_chromium_rel_ng   chromeos_x86-generic_chromium_compile_only_ng   chromium_presubmit   linux_chromium_compile_dbg_ng   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_linux   linux_chromium_chromeos_ozone_rel_ng   chromeos_daisy_chromium_compile_only_ng   linux_chromium_chromeos_rel_ng   linux_chromium_clobber_rel_ng 
Commit queue not available (can’t edit this change).

Messages

Total messages: 39 (17 generated)
mattcary (OOO July 14-26)
11 months ago (2016-08-24 12:24:56 UTC) #2
pasko
thanks! https://codereview.chromium.org/2275953002/diff/1/chrome/browser/prerender/prerender_histograms.cc File chrome/browser/prerender/prerender_histograms.cc (right): https://codereview.chromium.org/2275953002/diff/1/chrome/browser/prerender/prerender_histograms.cc#newcode156 chrome/browser/prerender/prerender_histograms.cc:156: "Prerender.OmniboxPrerenderCount", 1, 2); this histogram (Prerender.OmniboxNavigationsUsedPrerenderCount_Enabled) and another ...
11 months ago (2016-08-24 13:08:29 UTC) #5
mmenke
Thanks for doing this, LGTM (Modulo all of pasko's comments, which I agree with)
11 months ago (2016-08-24 14:54:59 UTC) #8
mattcary (OOO July 14-26)
https://codereview.chromium.org/2275953002/diff/1/chrome/browser/prerender/prerender_histograms.cc File chrome/browser/prerender/prerender_histograms.cc (right): https://codereview.chromium.org/2275953002/diff/1/chrome/browser/prerender/prerender_histograms.cc#newcode156 chrome/browser/prerender/prerender_histograms.cc:156: "Prerender.OmniboxPrerenderCount", 1, 2); On 2016/08/24 13:08:29, pasko wrote: > ...
11 months ago (2016-08-24 15:49:11 UTC) #9
pasko
https://codereview.chromium.org/2275953002/diff/1/chrome/browser/prerender/prerender_histograms.cc File chrome/browser/prerender/prerender_histograms.cc (right): https://codereview.chromium.org/2275953002/diff/1/chrome/browser/prerender/prerender_histograms.cc#newcode156 chrome/browser/prerender/prerender_histograms.cc:156: "Prerender.OmniboxPrerenderCount", 1, 2); On 2016/08/24 15:49:11, mattcary wrote: > ...
11 months ago (2016-08-24 17:51:38 UTC) #14
mattcary (OOO July 14-26)
On 2016/08/24 17:51:38, pasko wrote: > https://codereview.chromium.org/2275953002/diff/1/chrome/browser/prerender/prerender_histograms.cc > File chrome/browser/prerender/prerender_histograms.cc (right): > > https://codereview.chromium.org/2275953002/diff/1/chrome/browser/prerender/prerender_histograms.cc#newcode156 > ...
11 months ago (2016-08-25 13:43:03 UTC) #15
mattcary (OOO July 14-26)
On 2016/08/25 13:43:03, mattcary wrote: > On 2016/08/24 17:51:38, pasko wrote: > > > https://codereview.chromium.org/2275953002/diff/1/chrome/browser/prerender/prerender_histograms.cc ...
11 months ago (2016-08-25 13:43:28 UTC) #16
pasko
https://codereview.chromium.org/2275953002/diff/40001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2275953002/diff/40001/tools/metrics/histograms/histograms.xml#newcode102573 tools/metrics/histograms/histograms.xml:102573: + <obsolete> some of the below is still not ...
11 months ago (2016-08-25 14:05:25 UTC) #17
mattcary (OOO July 14-26)
Okay try #2 of X. I split out the deprecated parts of PrerenderSource to DeprecatedPrerenderSource. ...
11 months ago (2016-08-25 16:03:28 UTC) #18
pasko
https://codereview.chromium.org/2275953002/diff/60001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (left): https://codereview.chromium.org/2275953002/diff/60001/tools/metrics/histograms/histograms.xml#oldcode102631 tools/metrics/histograms/histograms.xml:102631: <affected-histogram name="Prerender.FinalStatusMatchComplete"/> this guy is also deprecated, and I'm ...
11 months ago (2016-08-25 16:32:04 UTC) #19
pasko
on the other hand, these remaining deprecated-but-not-marked-as-obsolete are not very relevant to this specific change, ...
11 months ago (2016-08-25 17:21:33 UTC) #20
mattcary (OOO July 14-26)
On 2016/08/25 17:21:33, pasko wrote: > on the other hand, these remaining deprecated-but-not-marked-as-obsolete are not ...
10 months, 4 weeks ago (2016-08-26 15:24:14 UTC) #21
mattcary (OOO July 14-26)
https://codereview.chromium.org/2275953002/diff/60001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (left): https://codereview.chromium.org/2275953002/diff/60001/tools/metrics/histograms/histograms.xml#oldcode102631 tools/metrics/histograms/histograms.xml:102631: <affected-histogram name="Prerender.FinalStatusMatchComplete"/> On 2016/08/25 16:32:04, pasko wrote: > this ...
10 months, 4 weeks ago (2016-08-26 15:32:27 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2275953002/80001
10 months, 4 weeks ago (2016-08-26 15:32:50 UTC) #25
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/247036)
10 months, 4 weeks ago (2016-08-26 15:38:56 UTC) #27
mattcary (OOO July 14-26)
+isherman for histograms.xml review. Thanks!
10 months, 4 weeks ago (2016-08-26 15:57:12 UTC) #29
Ilya Sherman (Away til Aug 14)
Thanks for the cleanup! LGTM.
10 months, 4 weeks ago (2016-08-26 21:49:48 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2275953002/80001
10 months, 3 weeks ago (2016-08-29 08:14:39 UTC) #32
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/283147)
10 months, 3 weeks ago (2016-08-29 10:34:47 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2275953002/80001
10 months, 3 weeks ago (2016-08-29 10:56:47 UTC) #36
commit-bot: I haz the power
Committed patchset #5 (id:80001)
10 months, 3 weeks ago (2016-08-29 11:44:35 UTC) #37
commit-bot: I haz the power
10 months, 3 weeks ago (2016-08-29 11:45:53 UTC) #39
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/81efc02ef67475e403bc5f6c4a79446eda627423
Cr-Commit-Position: refs/heads/master@{#414997}
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 25c286973