Background tracing: Added trace_chance

content/browser/tracing/background_tracing_rule.cc

Index: content/browser/tracing/background_tracing_rule.cc
diff --git a/content/browser/tracing/background_tracing_rule.cc b/content/browser/tracing/background_tracing_rule.cc
index cd78b1b10940a7d4c1b4cd3d83528335fef51e37..0487a733493e262eac6f4e0aa9bba45335168225 100644
--- a/content/browser/tracing/background_tracing_rule.cc
+++ b/content/browser/tracing/background_tracing_rule.cc
@@ -22,6 +22,7 @@ const char kConfigRuleKey[] = "rule";
 const char kConfigCategoryKey[] = "category";
 const char kConfigRuleTriggerNameKey[] = "trigger_name";
 const char kConfigRuleTriggerDelay[] = "trigger_delay";
+const char kConfigRuleTriggerChance[] = "trigger_chance";

[shatch, 2015/10/30 15:50:28]

Add unit tests.

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

Done.

 
 const char kConfigRuleHistogramNameKey[] = "histogram_name";
 const char kConfigRuleHistogramValueOldKey[] = "histogram_value";
@@ -55,7 +56,7 @@ const int kReactiveTraceRandomStartTimeMax = 120;
 
 namespace content {
 
-BackgroundTracingRule::BackgroundTracingRule() {}
+BackgroundTracingRule::BackgroundTracingRule() : trigger_chance_(1.0) {}
 
 BackgroundTracingRule::~BackgroundTracingRule() {}
 
@@ -73,6 +74,18 @@ int BackgroundTracingRule::GetTraceTimeout() const {
   return -1;
 }
 
+void BackgroundTracingRule::IntoDict(base::DictionaryValue* dict) const {
+  DCHECK(dict);
+  if (trigger_chance_ < 1.0)
+    dict->SetDouble(kConfigRuleTriggerChance, trigger_chance_);
+}
+
+void BackgroundTracingRule::FromDict(const base::DictionaryValue* dict) {
+  double trigger_chance;

[shatch, 2015/10/30 15:50:28]

nit: Can't you just pass trigger_chance_ in directly? If the get fails, it
doesn't output anything.

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

Done.

+  if (dict->GetDouble(kConfigRuleTriggerChance, &trigger_chance))
+    trigger_chance_ = trigger_chance;
+}
+
 namespace {
 
 class NamedTriggerRule : public BackgroundTracingRule {
@@ -93,6 +106,7 @@ class NamedTriggerRule : public BackgroundTracingRule {
 
   void IntoDict(base::DictionaryValue* dict) const override {
     DCHECK(dict);
+    BackgroundTracingRule::IntoDict(dict);
     dict->SetString(kConfigRuleKey, kPreemptiveConfigRuleMonitorNamed);
     dict->SetString(kConfigRuleTriggerNameKey, named_event_.c_str());
   }
@@ -173,6 +187,7 @@ class HistogramRule : public BackgroundTracingRule,
 
   void IntoDict(base::DictionaryValue* dict) const override {
     DCHECK(dict);
+    BackgroundTracingRule::IntoDict(dict);
     dict->SetString(kConfigRuleKey, kPreemptiveConfigRuleMonitorHistogram);
     dict->SetString(kConfigRuleHistogramNameKey, histogram_name_.c_str());
     dict->SetInteger(kConfigRuleHistogramValue1Key, histogram_lower_value_);
@@ -265,6 +280,7 @@ class ReactiveTraceForNSOrTriggerOrFullRule : public BackgroundTracingRule {
   // BackgroundTracingRule implementation
   void IntoDict(base::DictionaryValue* dict) const override {
     DCHECK(dict);
+    BackgroundTracingRule::IntoDict(dict);
     dict->SetString(
         kConfigCategoryKey,
         BackgroundTracingConfigImpl::CategoryPresetToString(category_preset_));
@@ -326,6 +342,7 @@ class ReactiveTraceAtRandomIntervalsRule : public BackgroundTracingRule {
 
   void IntoDict(base::DictionaryValue* dict) const override {
     DCHECK(dict);
+    BackgroundTracingRule::IntoDict(dict);
     dict->SetString(
         kConfigCategoryKey,
         BackgroundTracingConfigImpl::CategoryPresetToString(category_preset_));
@@ -404,13 +421,16 @@ scoped_ptr<BackgroundTracingRule> BackgroundTracingRule::PreemptiveRuleFromDict(
   if (!dict->GetString(kConfigRuleKey, &type))
     return nullptr;
 
+  scoped_ptr<BackgroundTracingRule> tracing_rule;
   if (type == kPreemptiveConfigRuleMonitorNamed)
-    return NamedTriggerRule::Create(dict);
+    tracing_rule = NamedTriggerRule::Create(dict);
+  else if (type == kPreemptiveConfigRuleMonitorHistogram)
+    tracing_rule = HistogramRule::Create(dict);
 
-  if (type == kPreemptiveConfigRuleMonitorHistogram)
-    return HistogramRule::Create(dict);
+  if (tracing_rule)
+    tracing_rule->FromDict(dict);

[shatch, 2015/10/30 15:50:28]

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.

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

Renamed to "Setup".

 
-  return nullptr;
+  return tracing_rule;
 }
 
 scoped_ptr<BackgroundTracingRule> BackgroundTracingRule::ReactiveRuleFromDict(
@@ -422,13 +442,20 @@ scoped_ptr<BackgroundTracingRule> BackgroundTracingRule::ReactiveRuleFromDict(
   if (!dict->GetString(kConfigRuleKey, &type))
     return nullptr;
 
-  if (type == kReactiveConfigRuleTraceOnNavigationUntilTriggerOrFull)
-    return ReactiveTraceForNSOrTriggerOrFullRule::Create(dict, category_preset);
+  scoped_ptr<BackgroundTracingRule> tracing_rule;
+
+  if (type == kReactiveConfigRuleTraceOnNavigationUntilTriggerOrFull) {
+    tracing_rule =
+        ReactiveTraceForNSOrTriggerOrFullRule::Create(dict, category_preset);
+  } else if (type == kReactiveConfigRuleTraceAtRandomIntervals) {
+    tracing_rule =
+        ReactiveTraceAtRandomIntervalsRule::Create(dict, category_preset);
+  }
 
-  if (type == kReactiveConfigRuleTraceAtRandomIntervals)
-    return ReactiveTraceAtRandomIntervalsRule::Create(dict, category_preset);
+  if (tracing_rule)
+    tracing_rule->FromDict(dict);
 
-  return nullptr;
+  return tracing_rule;
 }
 
 }  // namespace content