Background tracing: Added trace_chance

Background tracing: Added trace_chance

This allows trigger rules to be configured to only successfully trigger a certain percentage of the time.

R=shatch
BUG=546193
Committed: https://crrev.com/b6461a2c4357e6a47f525e1926d35149fbbaf3fa
Cr-Commit-Position: refs/heads/master@{#357204}

[oystein (OOO til 10th of July), 2015-10-29 21:21:08]

Description was changed from

==========
WIP


git cl format


Tests work


git cl format


git cl format


git cl format


Pre tests

BUG=
==========

to

==========
Background tracing: Added trace_chance

This allows trigger rules to be configured to only successfully trigger a
certain percentage of the time.

R=shatch
BUG=546193
==========

[oystein (OOO til 10th of July), 2015-10-29 21:21:08]

oysteine@chromium.org changed reviewers:
+ simonhatch@chromium.org

[oystein (OOO til 10th of July), 2015-10-29 21:21:19]

ptal!

[oystein (OOO til 10th of July), 2015-10-29 21:28:13]

The CQ bit was checked by oysteine@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-29 21:29:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427873003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427873003/1

[commit-bot: I haz the power, 2015-10-29 22:50:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-29 22:50:18]

Dry run: This issue passed the CQ dry run.

[shatch, 2015-10-30 15:50:29]

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
File content/browser/tracing/background_tracing_manager_browsertest.cc (right):

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_manager_browsertest.cc:630: // This
tests that no preemptive trace is triggered with 0 chance set.
nit: reactive

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
File content/browser/tracing/background_tracing_rule.cc (right):

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_rule.cc:25: const char
kConfigRuleTriggerChance[] = "trigger_chance";
Add unit tests.

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_rule.cc:84: double trigger_chance;
nit: Can't you just pass trigger_chance_ in directly? If the get fails, it
doesn't output anything.

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_rule.cc:431:
tracing_rule->FromDict(dict);
nit: Maybe rename this? "FromDict" kinda implies you're making the rule from the
dict, but you already created it from the dict earlier, it's just missing some
parameters that are common to all rules.

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
File content/browser/tracing/background_tracing_rule.h (right):

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_rule.h:24: virtual void
FromDict(const base::DictionaryValue* dict);
Does this need to be virtual?

[oystein (OOO til 10th of July), 2015-10-30 19:11:44]

Thanks!

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
File content/browser/tracing/background_tracing_manager_browsertest.cc (right):

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_manager_browsertest.cc:630: // This
tests that no preemptive trace is triggered with 0 chance set.
On 2015/10/30 15:50:28, shatch wrote:
> nit: reactive

Done.

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
File content/browser/tracing/background_tracing_rule.cc (right):

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_rule.cc:25: const char
kConfigRuleTriggerChance[] = "trigger_chance";
On 2015/10/30 15:50:28, shatch wrote:
> Add unit tests.

Done.

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_rule.cc:84: double trigger_chance;
On 2015/10/30 15:50:28, shatch wrote:
> nit: Can't you just pass trigger_chance_ in directly? If the get fails, it
> doesn't output anything.

Done.

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_rule.cc:431:
tracing_rule->FromDict(dict);
On 2015/10/30 15:50:28, shatch wrote:
> nit: Maybe rename this? "FromDict" kinda implies you're making the rule from
the
> dict, but you already created it from the dict earlier, it's just missing some
> parameters that are common to all rules.

Renamed to "Setup".

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
File content/browser/tracing/background_tracing_rule.h (right):

https://codereview.chromium.org/1427873003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_rule.h:24: virtual void
FromDict(const base::DictionaryValue* dict);
On 2015/10/30 15:50:28, shatch wrote:
> Does this need to be virtual?

Done.

[shatch, 2015-10-30 19:17:06]

lgtm w/ nit

https://codereview.chromium.org/1427873003/diff/20001/content/browser/tracing...
File content/browser/tracing/background_tracing_manager_impl.cc (right):

https://codereview.chromium.org/1427873003/diff/20001/content/browser/tracing...
content/browser/tracing/background_tracing_manager_impl.cc:319: if
(trigger_chance < 1.0f && base::RandDouble() > trigger_chance) {
nit: consistent using float or double?

[oystein (OOO til 10th of July), 2015-10-30 19:24:47]

https://codereview.chromium.org/1427873003/diff/20001/content/browser/tracing...
File content/browser/tracing/background_tracing_manager_impl.cc (right):

https://codereview.chromium.org/1427873003/diff/20001/content/browser/tracing...
content/browser/tracing/background_tracing_manager_impl.cc:319: if
(trigger_chance < 1.0f && base::RandDouble() > trigger_chance) {
On 2015/10/30 19:17:06, shatch wrote:
> nit: consistent using float or double?

Done.

[oystein (OOO til 10th of July), 2015-10-30 19:24:49]

The CQ bit was checked by oysteine@chromium.org

[oystein (OOO til 10th of July), 2015-10-30 19:24:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
simonhatch@chromium.org
Link to the patchset: https://codereview.chromium.org/1427873003/#ps40001
(title: "Nit fix")

[commit-bot: I haz the power, 2015-10-30 19:26:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427873003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427873003/40001

[oystein (OOO til 10th of July), 2015-10-30 20:35:21]

The CQ bit was unchecked by oysteine@chromium.org

[oystein (OOO til 10th of July), 2015-10-30 20:35:47]

The CQ bit was checked by oysteine@chromium.org

[oystein (OOO til 10th of July), 2015-10-30 20:35:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
simonhatch@chromium.org
Link to the patchset: https://codereview.chromium.org/1427873003/#ps60001
(title: "Oops")

[commit-bot: I haz the power, 2015-10-30 20:36:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427873003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427873003/60001

[commit-bot: I haz the power, 2015-10-30 21:50:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-30 21:50:43]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oystein (OOO til 10th of July), 2015-10-30 21:56:45]

The CQ bit was checked by oysteine@chromium.org

[commit-bot: I haz the power, 2015-10-30 21:56:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427873003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427873003/60001

[commit-bot: I haz the power, 2015-10-30 22:36:01]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-30 22:37:23]

Patchset 4 (id:??) landed as
https://crrev.com/b6461a2c4357e6a47f525e1926d35149fbbaf3fa
Cr-Commit-Position: refs/heads/master@{#357204}