Allow direct and indirect navigation values to be varied via field trial.

chrome/browser/banners/app_banner_settings_helper.cc

 // Copyright 2014 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 "chrome/browser/banners/app_banner_settings_helper.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
 #include "chrome/browser/banners/app_banner_data_fetcher.h"
 #include "chrome/browser/banners/app_banner_metrics.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
 #include "components/rappor/rappor_utils.h"
 #include "content/public/browser/web_contents.h"
 #include "net/base/escape.h"
@@ skipping 443 matching lines @@
   // midnight with the resulting truncated number added.
   base::Time::Exploded exploded;
   time.LocalExplode(&exploded);
   int total_minutes = exploded.hour * 60 + exploded.minute;
 
   // Use truncating integer division here.
   return time.LocalMidnight() +
          base::TimeDelta::FromMinutes((total_minutes / minutes) * minutes);
 }
 
+void AppBannerSettingsHelper::UpdateEngagementWeights() {

[benwells, 2015/08/26 06:50:06]

UpdateEngagementWeights and UpdateMinutesBetweenVisits don't need to be exposed.
They could be in the anon namespace.

[dominickn, 2015/08/26 07:04:23]

Done.

+  // Expect a field trial value of "X:Y", where X is the direct engagement
+  // value and Y is the indirect engagement value.
+  std::string weights =
+      base::FieldTrialList::FindFullName("AppBannersEngagementWeights");

[benwells, 2015/08/26 06:50:06]

Nit: do you think it is worthwhile adding something like:

if (weights.empty())
  return;

?

(and in UpdateMinutesBetweenVisits).

[dominickn, 2015/08/26 07:04:23]

Done.

+  std::vector<base::StringPiece> tokens = base::SplitStringPiece(

[benwells, 2015/08/26 06:50:06]

Could you just use SplitString? Since you are calling as_string below I'm not
sure if there is an advantage to using StringPiece (correct me if I'm wrong).

[dominickn, 2015/08/26 07:04:23]

I believe there's a memory advantage to SplitStringPiece, but that may be
nullified by as_string(), so done.

+      weights, ":", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  if (tokens.size() == 2) {
+    double direct_engagement = -1;
+    double indirect_engagement = -1;
+
+    // Ensure that we get valid doubles from the field trial, and that both
+    // values are greater than or equal to zero and less than or equal to the
+    // total engagement required to trigger the banner.
+    if (base::StringToDouble(tokens[0].as_string(), &direct_engagement) &&
+        base::StringToDouble(tokens[1].as_string(), &indirect_engagement) &&
+        direct_engagement >= 0 && indirect_engagement >= 0 &&
+        direct_engagement <= kTotalEngagementToTrigger &&
+        indirect_engagement <= kTotalEngagementToTrigger) {
+      SetEngagementWeights(direct_engagement, indirect_engagement);
+    }
+  }
+}
+
 void AppBannerSettingsHelper::UpdateMinutesBetweenVisits() {
   std::string minutes_between_visits =
       base::FieldTrialList::FindFullName("AppBannersMinutesBetweenVisits");
   int minimum_minutes = 0;
   if (base::StringToInt(minutes_between_visits, &minimum_minutes))
-    AppBannerSettingsHelper::SetMinimumMinutesBetweenVisits(minimum_minutes);
+    SetMinimumMinutesBetweenVisits(minimum_minutes);
 }
+
+void AppBannerSettingsHelper::UpdateFromFieldTrial() {
+  UpdateEngagementWeights();
+  UpdateMinutesBetweenVisits();
+}