Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2342993002:
Rearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics (Closed)
DescriptionRearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics
This allows for the Browser Task Scheduler to setup and redirect after
SetupFieldTrials to get variations data but before SetupMetrics, which
posts tasks to the blocking pool.
The corresponding iOS change will be in a later CL
BUG=636518
Committed: https://crrev.com/ce2e7d31800a187321977d0dce1115c862abc78c
Cr-Commit-Position: refs/heads/master@{#419511}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Self Review #Patch Set 3 : CR Feedback #
Dependent Patchsets: Messages
Total messages: 28 (21 generated)
Description was changed from ========== Rearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics This allows for the Browser Task Scheduler to setup and redirect after SetupFieldTrials to get variations data but before SetupMetrics, which posts tasks to the blocking pool. BUG=636518 ========== to ========== Rearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics This allows for the Browser Task Scheduler to setup and redirect after SetupFieldTrials to get variations data but before SetupMetrics, which posts tasks to the blocking pool. BUG=636518 ==========
robliao@chromium.org changed reviewers: + asvitkine@chromium.org
The CQ bit was checked by robliao@chromium.org to run a CQ dry run
asvitkine: Please review this CL. Thanks!
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:1) has been deleted
The CQ bit was checked by robliao@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm % comments Thanks! https://codereview.chromium.org/2342993002/diff/20001/chrome/browser/chrome_b... File chrome/browser/chrome_browser_main.h (right): https://codereview.chromium.org/2342993002/diff/20001/chrome/browser/chrome_b... chrome/browser/chrome_browser_main.h:105: // Methods for |SetupFieldTrials()| and |SetupMetrics()|---------------------- I would just remove this line and the blank line below it. https://codereview.chromium.org/2342993002/diff/20001/chrome/browser/chrome_b... chrome/browser/chrome_browser_main.h:107: // Constructs metrics service and does related initialization, including Remove "Constructs metrics service" https://codereview.chromium.org/2342993002/diff/20001/chrome/browser/chrome_b... chrome/browser/chrome_browser_main.h:147: // Statistical testing infrastructure for the entire browser. NULL until Nit: NULL -> Null
robliao@chromium.org changed reviewers: + brettw@chromium.org
asvitkine: Done. Thanks! brettw: Please review this CL. Thanks! https://codereview.chromium.org/2342993002/diff/20001/chrome/browser/chrome_b... File chrome/browser/chrome_browser_main.h (right): https://codereview.chromium.org/2342993002/diff/20001/chrome/browser/chrome_b... chrome/browser/chrome_browser_main.h:105: // Methods for |SetupFieldTrials()| and |SetupMetrics()|---------------------- On 2016/09/15 21:50:57, Alexei Svitkine (very slow) wrote: > I would just remove this line and the blank line below it. Done. https://codereview.chromium.org/2342993002/diff/20001/chrome/browser/chrome_b... chrome/browser/chrome_browser_main.h:107: // Constructs metrics service and does related initialization, including On 2016/09/15 21:50:57, Alexei Svitkine (very slow) wrote: > Remove "Constructs metrics service" Done. https://codereview.chromium.org/2342993002/diff/20001/chrome/browser/chrome_b... chrome/browser/chrome_browser_main.h:147: // Statistical testing infrastructure for the entire browser. NULL until On 2016/09/15 21:50:57, Alexei Svitkine (very slow) wrote: > Nit: NULL -> Null Went with nullptr.
The CQ bit was checked by robliao@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Rearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics This allows for the Browser Task Scheduler to setup and redirect after SetupFieldTrials to get variations data but before SetupMetrics, which posts tasks to the blocking pool. BUG=636518 ========== to ========== Rearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics This allows for the Browser Task Scheduler to setup and redirect after SetupFieldTrials to get variations data but before SetupMetrics, which posts tasks to the blocking pool. The corresponding iOS change will be in a later CL BUG=636518 ==========
The CQ bit was checked by robliao@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by robliao@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from asvitkine@chromium.org Link to the patchset: https://codereview.chromium.org/2342993002/#ps60001 (title: "CR Feedback")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Rearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics This allows for the Browser Task Scheduler to setup and redirect after SetupFieldTrials to get variations data but before SetupMetrics, which posts tasks to the blocking pool. The corresponding iOS change will be in a later CL BUG=636518 ========== to ========== Rearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics This allows for the Browser Task Scheduler to setup and redirect after SetupFieldTrials to get variations data but before SetupMetrics, which posts tasks to the blocking pool. The corresponding iOS change will be in a later CL BUG=636518 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Rearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics This allows for the Browser Task Scheduler to setup and redirect after SetupFieldTrials to get variations data but before SetupMetrics, which posts tasks to the blocking pool. The corresponding iOS change will be in a later CL BUG=636518 ========== to ========== Rearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics This allows for the Browser Task Scheduler to setup and redirect after SetupFieldTrials to get variations data but before SetupMetrics, which posts tasks to the blocking pool. The corresponding iOS change will be in a later CL BUG=636518 Committed: https://crrev.com/ce2e7d31800a187321977d0dce1115c862abc78c Cr-Commit-Position: refs/heads/master@{#419511} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/ce2e7d31800a187321977d0dce1115c862abc78c Cr-Commit-Position: refs/heads/master@{#419511} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
