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

Issue 366233002: Refine UMA quality for warm-start app list cases (Closed)

Created:
6 years, 5 months ago by tapted
Modified:
6 years, 5 months ago
Reviewers:
Matt Giuca
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, tfarina
Visibility:
Public.

Description

Refine UMA quality for warm-start app list cases Currently "warm start" app list cases (i.e. shown via the process singleton) have noisy UMA data. Whether the profile to show is loaded or not has a big impact on the metric. We are only interested in the case that the app list view is "warm", and can be immediately shown. Recording cases that first must load a profile from disk, or switch the app list view to a different profile, count as noise and skew the metric. This change filters out the noise, fixing an incorrect assumption in r270615 which resulted in only the noise being recorded (and a failed DCHECK otherwise). Before r270615, the UMA included cases requiring a profile load as well as those that didn't. After r270615, only cases that required a profile load were being recorded. Note that ShowAppListInteractiveTest.ShowAppListFlag was updated in r281213 to include both cold and warm codepaths via StartupBrowserCreator, to cover the aforementioned DCHECK. BUG=382793 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282245

Patch Set 1 #

Patch Set 2 : rebase to master #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -6 lines) Patch
M chrome/browser/ui/app_list/app_list_service.cc View 1 4 chunks +36 lines, -5 lines 0 comments Download
M chrome/browser/ui/views/app_list/win/app_list_service_win.cc View 1 chunk +4 lines, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
Matt Giuca
LGTM! Nice. Again, please update the CL description (removing the [2/2]). Also, the last paragraph ...
6 years, 5 months ago (2014-07-04 04:44:41 UTC) #1
Matt Giuca
"not has a big impact on the metric" (from the CL description)?
6 years, 5 months ago (2014-07-08 00:30:53 UTC) #2
tapted
On 2014/07/08 00:30:53, Matt Giuca wrote: > "not has a big impact on the metric" ...
6 years, 5 months ago (2014-07-08 00:40:44 UTC) #3
tapted
The CQ bit was checked by tapted@chromium.org
6 years, 5 months ago (2014-07-10 00:10:15 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tapted@chromium.org/366233002/20001
6 years, 5 months ago (2014-07-10 00:12:54 UTC) #5
commit-bot: I haz the power
6 years, 5 months ago (2014-07-10 03:55:10 UTC) #6
Message was sent while issue was closed.
Change committed as 282245

Powered by Google App Engine
This is Rietveld 408576698