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

Issue 8890054: Adding metrics to track browser launches per primary/secondary profile. Adding metrics to track n... (Closed)

Created:
9 years ago by rpetterson
Modified:
9 years ago
CC:
chromium-reviews, jar (doing other things)
Visibility:
Public.

Description

Adding metrics to track browser launches per primary/secondary profile. Adding metrics to track number of profiles per machine. BUG=160771, 106774 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114088

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 8

Patch Set 3 : '' #

Patch Set 4 : '' #

Total comments: 12

Patch Set 5 : '' #

Patch Set 6 : '' #

Total comments: 1

Patch Set 7 : '' #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -27 lines) Patch
M chrome/browser/profiles/profile_manager.cc View 1 2 3 4 6 4 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/profiles/profile_metrics.h View 1 2 3 4 2 chunks +18 lines, -5 lines 2 comments Download
M chrome/browser/profiles/profile_metrics.cc View 1 2 3 4 4 chunks +43 lines, -22 lines 2 comments Download
M chrome/browser/ui/browser.cc View 1 2 3 4 5 6 2 chunks +4 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
rpetterson
isherman - metrics miranda - profiles sky - browser_init Thanks!
9 years ago (2011-12-09 00:46:00 UTC) #1
Ilya Sherman
http://codereview.chromium.org/8890054/diff/2002/chrome/browser/profiles/profile_manager.cc File chrome/browser/profiles/profile_manager.cc (right): http://codereview.chromium.org/8890054/diff/2002/chrome/browser/profiles/profile_manager.cc#newcode390 chrome/browser/profiles/profile_manager.cc:390: ProfileMetrics::LogNumberOfProfiles(false); nit: Rather than using a bool, I think ...
9 years ago (2011-12-09 00:53:41 UTC) #2
rpetterson
http://codereview.chromium.org/8890054/diff/2002/chrome/browser/profiles/profile_manager.cc File chrome/browser/profiles/profile_manager.cc (right): http://codereview.chromium.org/8890054/diff/2002/chrome/browser/profiles/profile_manager.cc#newcode390 chrome/browser/profiles/profile_manager.cc:390: ProfileMetrics::LogNumberOfProfiles(false); On 2011/12/09 00:53:41, Ilya Sherman wrote: > nit: ...
9 years ago (2011-12-09 01:44:35 UTC) #3
Ilya Sherman
Metrics LGTM (with one last comment addressed) http://codereview.chromium.org/8890054/diff/6/chrome/browser/profiles/profile_metrics.cc File chrome/browser/profiles/profile_metrics.cc (right): http://codereview.chromium.org/8890054/diff/6/chrome/browser/profiles/profile_metrics.cc#newcode218 chrome/browser/profiles/profile_metrics.cc:218: ProfileManager* manager ...
9 years ago (2011-12-09 01:50:47 UTC) #4
jar (doing other things)
http://codereview.chromium.org/8890054/diff/6/chrome/browser/profiles/profile_metrics.cc File chrome/browser/profiles/profile_metrics.cc (right): http://codereview.chromium.org/8890054/diff/6/chrome/browser/profiles/profile_metrics.cc#newcode225 chrome/browser/profiles/profile_metrics.cc:225: UMA_HISTOGRAM_COUNTS_100("Profile.NumberOfProfilesOnAddDelete", Perhaps a better name here would be: NumberOfProfilesBeforeAddOrDelete ...
9 years ago (2011-12-09 04:57:13 UTC) #5
Miranda Callahan
profiles LGTM (w/naming stuff addressed), thanks! http://codereview.chromium.org/8890054/diff/6/chrome/browser/profiles/profile_metrics.cc File chrome/browser/profiles/profile_metrics.cc (right): http://codereview.chromium.org/8890054/diff/6/chrome/browser/profiles/profile_metrics.cc#newcode218 chrome/browser/profiles/profile_metrics.cc:218: ProfileManager* manager = ...
9 years ago (2011-12-09 14:01:01 UTC) #6
sky
http://codereview.chromium.org/8890054/diff/6/chrome/browser/ui/browser_init.cc File chrome/browser/ui/browser_init.cc (right): http://codereview.chromium.org/8890054/diff/6/chrome/browser/ui/browser_init.cc#newcode585 chrome/browser/ui/browser_init.cc:585: ProfileMetrics::LogProfileLaunch(profile_path); Would this make more sense some where in ...
9 years ago (2011-12-09 17:46:38 UTC) #7
rpetterson
http://codereview.chromium.org/8890054/diff/6/chrome/browser/profiles/profile_metrics.cc File chrome/browser/profiles/profile_metrics.cc (right): http://codereview.chromium.org/8890054/diff/6/chrome/browser/profiles/profile_metrics.cc#newcode218 chrome/browser/profiles/profile_metrics.cc:218: ProfileManager* manager = g_browser_process->profile_manager(); On 2011/12/09 14:01:02, Miranda Callahan ...
9 years ago (2011-12-09 21:22:09 UTC) #8
sky
On Fri, Dec 9, 2011 at 1:22 PM, <rlp@chromium.org> wrote: > > http://codereview.chromium.org/8890054/diff/6/chrome/browser/profiles/profile_metrics.cc > File ...
9 years ago (2011-12-09 22:11:40 UTC) #9
rpetterson
On 2011/12/09 22:11:40, sky wrote: > On Fri, Dec 9, 2011 at 1:22 PM, <mailto:rlp@chromium.org> ...
9 years ago (2011-12-09 23:17:41 UTC) #10
sky
http://codereview.chromium.org/8890054/diff/9004/chrome/browser/profiles/profile_manager.cc File chrome/browser/profiles/profile_manager.cc (right): http://codereview.chromium.org/8890054/diff/9004/chrome/browser/profiles/profile_manager.cc#newcode424 chrome/browser/profiles/profile_manager.cc:424: ProfileMetrics::LogProfileLaunch(profile_path); I believe NewWindowWithProfile is not invoked for every ...
9 years ago (2011-12-09 23:29:42 UTC) #11
rpetterson
Fair enough. That will certainly catch everything.
9 years ago (2011-12-09 23:49:04 UTC) #12
sky
LGTM, but I'm not sure if you only care about type_ == TYPE_TABBED
9 years ago (2011-12-09 23:53:41 UTC) #13
rpetterson
On 2011/12/09 23:53:41, sky wrote: > LGTM, but I'm not sure if you only care ...
9 years ago (2011-12-09 23:58:30 UTC) #14
rpetterson
Ping to jar@.
9 years ago (2011-12-10 00:19:31 UTC) #15
jar (doing other things)
http://codereview.chromium.org/8890054/diff/10007/chrome/browser/profiles/profile_metrics.cc File chrome/browser/profiles/profile_metrics.cc (right): http://codereview.chromium.org/8890054/diff/10007/chrome/browser/profiles/profile_metrics.cc#newcode76 chrome/browser/profiles/profile_metrics.cc:76: UMA_HISTOGRAM_COUNTS_100("Profile.NumberOfProfilesAfterAddOrDelete", I'm curious, do you really expect the distribution ...
9 years ago (2011-12-10 22:07:44 UTC) #16
rpetterson
http://codereview.chromium.org/8890054/diff/10007/chrome/browser/profiles/profile_metrics.cc File chrome/browser/profiles/profile_metrics.cc (right): http://codereview.chromium.org/8890054/diff/10007/chrome/browser/profiles/profile_metrics.cc#newcode76 chrome/browser/profiles/profile_metrics.cc:76: UMA_HISTOGRAM_COUNTS_100("Profile.NumberOfProfilesAfterAddOrDelete", On 2011/12/10 22:07:44, jar wrote: > I'm curious, ...
9 years ago (2011-12-11 08:46:11 UTC) #17
jar (doing other things)
The almost redundant histogram is a little much, but at least borderline understandable. I suspect ...
9 years ago (2011-12-12 03:35:25 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rlp@chromium.org/8890054/10007
9 years ago (2011-12-12 20:47:39 UTC) #19
commit-bot: I haz the power
9 years ago (2011-12-12 21:53:59 UTC) #20
Change committed as 114088

Powered by Google App Engine
This is Rietveld 408576698