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

Issue 2955063005: [Cleanup] Migrate ChromeBrowserMainExtraPartsMetrics to use the Task Scheduler. (Closed)

Created:
1 year, 8 months ago by Ilya Sherman
Modified:
1 year, 8 months ago
CC:
chromium-reviews, asvitkine+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Cleanup] Migrate ChromeBrowserMainExtraPartsMetrics to use the Task Scheduler. This also changes the timing with which startup metrics are recorded: Previously they were recorded ~45 seconds after startup; now they're recorded whenever the TaskScheduler schedules them to be. This is not expected to change the values recorded to these metrics. BUG=667892, 741816 TEST=none R=asvitkine@chromium.org Review-Url: https://codereview.chromium.org/2955063005 Cr-Commit-Position: refs/heads/master@{#486114} Committed: https://chromium.googlesource.com/chromium/src/+/2d71d53f0142954869b02fb1a93d8c790e1743c3

Patch Set 1 #

Patch Set 2 : Update #includes #

Patch Set 3 : Rebase #

Patch Set 4 : Use SequenceTaskRunner to post Windows Mojo-bound task #

Patch Set 5 : Rebase #

Patch Set 6 : Try a COM thread #

Patch Set 7 : Rebase #

Patch Set 8 : Use COM thread for RecordStartupMetrics #

Patch Set 9 : Add base:: namespace koala-fication #

Patch Set 10 : For debugging: VLOG(1) -> LOG(ERROR) #

Patch Set 11 : Some more logging #

Patch Set 12 : Add some more logging #

Patch Set 13 : Try a timeout #

Patch Set 14 : The delay worked! #

Total comments: 4

Patch Set 15 : const #

Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -16 lines) Patch
M chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 7 chunks +29 lines, -16 lines 0 comments Download

Messages

Total messages: 70 (54 generated)
Ilya Sherman
1 year, 8 months ago (2017-06-27 19:46:27 UTC) #1
Alexei Svitkine (slow)
will wait for green bots (they're currently red)
1 year, 8 months ago (2017-06-27 20:13:36 UTC) #6
Ilya Sherman
+gab: I'm confused by the test failures, e.g. [1]. Is this likely caused by the ...
1 year, 8 months ago (2017-06-28 20:20:20 UTC) #12
gab
On 2017/06/28 20:20:20, Ilya Sherman wrote: > +gab: I'm confused by the test failures, e.g. ...
1 year, 8 months ago (2017-06-28 20:46:30 UTC) #13
Ilya Sherman
On 2017/06/28 20:46:30, gab (slow Tue-Wed) wrote: > On 2017/06/28 20:20:20, Ilya Sherman wrote: > ...
1 year, 8 months ago (2017-06-28 21:09:15 UTC) #16
Alexei Svitkine (slow)
lgtm but gab's the expert Also yay for moving random lengthy stuff from IO thread ...
1 year, 8 months ago (2017-06-29 19:17:22 UTC) #21
Ilya Sherman
On 2017/06/29 19:17:22, Alexei Svitkine (slow) wrote: > lgtm but gab's the expert > > ...
1 year, 8 months ago (2017-06-29 19:42:16 UTC) #22
gab
On 2017/06/29 19:42:16, Ilya Sherman wrote: > On 2017/06/29 19:17:22, Alexei Svitkine (slow) wrote: > ...
1 year, 8 months ago (2017-07-05 02:40:51 UTC) #25
gab
On 2017/07/05 02:40:51, gab (in-out til july17) wrote: > On 2017/06/29 19:42:16, Ilya Sherman wrote: ...
1 year, 8 months ago (2017-07-05 02:41:32 UTC) #26
Ilya Sherman
Alexei, mind taking another look as a sanity check? I think I finally nailed down ...
1 year, 8 months ago (2017-07-12 05:07:14 UTC) #57
Alexei Svitkine (slow)
lgtm https://codereview.chromium.org/2955063005/diff/260001/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc File chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc (right): https://codereview.chromium.org/2955063005/diff/260001/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc#newcode513 chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc:513: base::TaskTraits background_task_traits = { Nit: const https://codereview.chromium.org/2955063005/diff/260001/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc#newcode546 chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc:546: ...
1 year, 8 months ago (2017-07-12 15:12:57 UTC) #60
Ilya Sherman
https://codereview.chromium.org/2955063005/diff/260001/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc File chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc (right): https://codereview.chromium.org/2955063005/diff/260001/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc#newcode513 chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc:513: base::TaskTraits background_task_traits = { On 2017/07/12 15:12:56, Alexei Svitkine ...
1 year, 8 months ago (2017-07-12 19:39:47 UTC) #62
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/2955063005/280001
1 year, 8 months ago (2017-07-12 19:40:42 UTC) #65
commit-bot: I haz the power
Committed patchset #15 (id:280001) as https://chromium.googlesource.com/chromium/src/+/2d71d53f0142954869b02fb1a93d8c790e1743c3
1 year, 8 months ago (2017-07-12 21:55:12 UTC) #68
chromium-reviews
Le mer. 12 juil. 2017 08:12, <asvitkine@chromium.org> a écrit : > lgtm > > > ...
1 year, 8 months ago (2017-07-13 00:37:40 UTC) #69
Ilya Sherman
1 year, 8 months ago (2017-07-14 05:54:49 UTC) #70
Message was sent while issue was closed.
On 2017/07/13 00:37:40, chromium-reviews wrote:
> constexpr is even better for TaskTraits

(a) https://chromium-review.googlesource.com/c/570732/
(b) Pourquoi?

Powered by Google App Engine
This is Rietveld 408576698