Background tracing: Added trace_chance

content/browser/tracing/background_tracing_rule.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 #include "content/browser/tracing/background_tracing_rule.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/rand_util.h"
 #include "base/strings/safe_sprintf.h"
 #include "base/values.h"
 #include "components/tracing/tracing_messages.h"
 #include "content/browser/tracing/background_tracing_manager_impl.h"
 #include "content/browser/tracing/trace_message_filter.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace {
 
 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";
 const char kConfigRuleHistogramValue1Key[] = "histogram_lower_value";
 const char kConfigRuleHistogramValue2Key[] = "histogram_upper_value";
 const char kConfigRuleHistogramRepeatKey[] = "histogram_repeat";
 
 const char kConfigRuleRandomIntervalTimeoutMin[] = "timeout_min";
 const char kConfigRuleRandomIntervalTimeoutMax[] = "timeout_max";
 
@@ skipping 13 matching lines @@
     "ReactiveTraceAtRandomIntervals";
 
 const int kReactiveConfigNavigationTimeout = 30;
 const int kReactiveTraceRandomStartTimeMin = 60;
 const int kReactiveTraceRandomStartTimeMax = 120;
 
 }  // namespace
 
 namespace content {
 
-BackgroundTracingRule::BackgroundTracingRule() {}
+BackgroundTracingRule::BackgroundTracingRule() : trigger_chance_(1.0) {}
 
 BackgroundTracingRule::~BackgroundTracingRule() {}
 
 bool BackgroundTracingRule::ShouldTriggerNamedEvent(
     const std::string& named_event) const {
   return false;
 }
 
 BackgroundTracingConfigImpl::CategoryPreset
 BackgroundTracingRule::GetCategoryPreset() const {
   return BackgroundTracingConfigImpl::BENCHMARK;
 }
 
 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 {
  private:
   NamedTriggerRule(const std::string& named_event)
       : named_event_(named_event) {}
 
  public:
   static scoped_ptr<BackgroundTracingRule> Create(
       const base::DictionaryValue* dict) {
     std::string trigger_name;
     if (!dict->GetString(kConfigRuleTriggerNameKey, &trigger_name))
       return nullptr;
 
     return scoped_ptr<BackgroundTracingRule>(
         new NamedTriggerRule(trigger_name));
   }
 
   void IntoDict(base::DictionaryValue* dict) const override {
     DCHECK(dict);
+    BackgroundTracingRule::IntoDict(dict);
     dict->SetString(kConfigRuleKey, kPreemptiveConfigRuleMonitorNamed);
     dict->SetString(kConfigRuleTriggerNameKey, named_event_.c_str());
   }
 
   bool ShouldTriggerNamedEvent(const std::string& named_event) const override {
     return named_event == named_event_;
   }
 
  private:
   std::string named_event_;
@@ skipping 60 matching lines @@
         histogram_name_,
         base::Bind(&HistogramRule::OnHistogramChangedCallback,
                    base::Unretained(this), histogram_name_,
                    histogram_lower_value_, histogram_upper_value_, repeat_));
 
     TracingControllerImpl::GetInstance()->AddTraceMessageFilterObserver(this);
   }
 
   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_);
     dict->SetInteger(kConfigRuleHistogramValue2Key, histogram_upper_value_);
     dict->SetBoolean(kConfigRuleHistogramRepeatKey, repeat_);
     if (trigger_delay_ != -1)
       dict->SetInteger(kConfigRuleTriggerDelay, trigger_delay_);
   }
 
   void OnHistogramTrigger(const std::string& histogram_name) const override {
@@ skipping 72 matching lines @@
       return nullptr;
 
     return scoped_ptr<BackgroundTracingRule>(
         new ReactiveTraceForNSOrTriggerOrFullRule(trigger_name,
                                                   category_preset));
   }
 
   // BackgroundTracingRule implementation
   void IntoDict(base::DictionaryValue* dict) const override {
     DCHECK(dict);
+    BackgroundTracingRule::IntoDict(dict);
     dict->SetString(
         kConfigCategoryKey,
         BackgroundTracingConfigImpl::CategoryPresetToString(category_preset_));
     dict->SetString(kConfigRuleKey,
                     kReactiveConfigRuleTraceOnNavigationUntilTriggerOrFull);
     dict->SetString(kConfigRuleTriggerNameKey, named_event_.c_str());
   }
 
   bool ShouldTriggerNamedEvent(const std::string& named_event) const override {
     return named_event == named_event_;
@@ skipping 41 matching lines @@
       return nullptr;
 
     return scoped_ptr<BackgroundTracingRule>(
         new ReactiveTraceAtRandomIntervalsRule(category_preset, timeout_min,
                                                timeout_max));
   }
   ~ReactiveTraceAtRandomIntervalsRule() override {}
 
   void IntoDict(base::DictionaryValue* dict) const override {
     DCHECK(dict);
+    BackgroundTracingRule::IntoDict(dict);
     dict->SetString(
         kConfigCategoryKey,
         BackgroundTracingConfigImpl::CategoryPresetToString(category_preset_));
     dict->SetString(kConfigRuleKey, kReactiveConfigRuleTraceAtRandomIntervals);
     dict->SetInteger(kConfigRuleRandomIntervalTimeoutMin, timeout_min_);
     dict->SetInteger(kConfigRuleRandomIntervalTimeoutMax, timeout_max_);
   }
 
   void Install() override {
     handle_ = BackgroundTracingManagerImpl::GetInstance()->RegisterTriggerType(
@@ skipping 58 matching lines @@
 }  // namespace
 
 scoped_ptr<BackgroundTracingRule> BackgroundTracingRule::PreemptiveRuleFromDict(
     const base::DictionaryValue* dict) {
   DCHECK(dict);
 
   std::string type;
   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(
     const base::DictionaryValue* dict,
     BackgroundTracingConfigImpl::CategoryPreset category_preset) {
   DCHECK(dict);
 
   std::string type;
   if (!dict->GetString(kConfigRuleKey, &type))
     return nullptr;
 
-  if (type == kReactiveConfigRuleTraceOnNavigationUntilTriggerOrFull)
-    return ReactiveTraceForNSOrTriggerOrFullRule::Create(dict, category_preset);
+  scoped_ptr<BackgroundTracingRule> tracing_rule;
 
-  if (type == kReactiveConfigRuleTraceAtRandomIntervals)
-    return ReactiveTraceAtRandomIntervalsRule::Create(dict, category_preset);
+  if (type == kReactiveConfigRuleTraceOnNavigationUntilTriggerOrFull) {
+    tracing_rule =
+        ReactiveTraceForNSOrTriggerOrFullRule::Create(dict, category_preset);
+  } else if (type == kReactiveConfigRuleTraceAtRandomIntervals) {
+    tracing_rule =
+        ReactiveTraceAtRandomIntervalsRule::Create(dict, category_preset);
+  }
 
-  return nullptr;
+  if (tracing_rule)
+    tracing_rule->FromDict(dict);
+
+  return tracing_rule;
 }
 
 }  // namespace content