Add coarse score to site engagement UKM

This adds to the SiteEngagement UKM collection to track the final
engagement score, rounded down to the nearest multiple of 10 to reduce
the granularity of the collection.

Additionally adds a new metrics utility function to perform linear
bucketing (similar to the existing exponential bucketing function).

Bug: 803544
Change-Id: I1c3031e94d4cce28995ab53095c6a4df3e39f65e
Reviewed-on: https://chromium-review.googlesource.com/939876
Commit-Queue: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540943}

chrome/browser/page_load_metrics/observers/security_state_page_load_metrics_observer.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/ssl/security_state_tab_helper.h"
 #include "components/security_state/core/security_state.h"
 #include "content/public/browser/navigation_handle.h"
+#include "services/metrics/public/cpp/metrics_utils.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
 
@@ -170,12 +171,17 @@ void SecurityStatePageLoadMetricsObserver::OnComplete(
   if (engagement_service_) {
     double final_engagement_score =
         engagement_service_->GetScore(extra_info.url);
+    // Round the engagement score down to the closest multiple of 10 to decrease
+    // the granularity of the UKM collection.
+    int64_t coarse_engagement_score =
+        ukm::GetLinearBucketMin(final_engagement_score, 10);
 
     ukm::UkmRecorder* ukm_recorder = ukm::UkmRecorder::Get();
     ukm::builders::Security_SiteEngagement(source_id_)
         .SetInitialSecurityLevel(initial_security_level_)
         .SetFinalSecurityLevel(current_security_level_)
         .SetScoreDelta(final_engagement_score - initial_engagement_score_)
+        .SetScoreFinal(coarse_engagement_score)
         .Record(ukm_recorder);
 
     // Get the change in Site Engagement score and transform it into the range

services/BUILD.gn

@@ -20,6 +20,7 @@ service_test("services_unittests") {
   # section below. If you are unsure, contact blundell@chromium.org.
   deps = [
     "//services/identity:tests",
+    "//services/metrics/public/cpp:tests",
     "//services/network:tests",
     "//services/network/public/cpp:tests",
   ]

services/metrics/public/cpp/BUILD.gn

@@ -37,6 +37,20 @@ component("metrics_cpp") {
   ]
 }
 
+source_set("tests") {
+  testonly = true
+
+  sources = [
+    "metrics_utils_unittest.cc",
+  ]
+
+  deps = [
+    ":metrics_cpp",
+    "//base",
+    "//testing/gtest",
+  ]
+}
+
 action("gen_ukm_builders") {
   script = "//tools/metrics/ukm/gen_builders.py"
 

services/metrics/public/cpp/metrics_utils.cc

@@ -4,10 +4,11 @@
 
 #include "services/metrics/public/cpp/metrics_utils.h"
 
-#include <math.h>
-
 #include <cmath>
 
+#include "base/logging.h"
+#include "base/numerics/safe_conversions.h"
+
 namespace ukm {
 
 int64_t GetExponentialBucketMin(int64_t sample, double bucket_spacing) {
@@ -21,4 +22,22 @@ int64_t GetExponentialBucketMin(int64_t sample, double bucket_spacing) {
       bucket_spacing, std::floor(std::log(sample) / std::log(bucket_spacing))));
 }
 
+int64_t GetLinearBucketMin(int64_t sample, int32_t bucket_size) {
+  DCHECK(bucket_size > 0);
+  // Round down to the nearest multiple of |bucket_size| (for negative samples,
+  // this rounds away from zero).
+  int64_t remainder = sample % bucket_size;
+  if (remainder < 0)
+    return sample - (remainder + bucket_size);
+  return sample - remainder;
+}
+
+int64_t GetLinearBucketMin(double sample, int32_t bucket_size) {
+  int64_t val = GetLinearBucketMin(
+      base::saturated_cast<int64_t>(std::floor(sample)), bucket_size);
+  // Ensure that |sample| can't get put into a bucket higher than itself.
+  DCHECK(val <= sample);
+  return val;
+}
+
 }  // namespace ukm

services/metrics/public/cpp/metrics_utils.h

@@ -17,6 +17,14 @@ namespace ukm {
 int64_t METRICS_EXPORT GetExponentialBucketMin(int64_t sample,
                                                double bucket_spacing);
 
+// Calculates the linear bucket |sample| falls in and returns the lower
+// threshold of that bucket (i.e., rounding down to the nearest multiple of
+// |bucket_size|). Negative sample values will be rounded down as well (away
+// from zero). |bucket_size| is the size of each bucket, and must be a non-zero
+// positive integer.
+int64_t METRICS_EXPORT GetLinearBucketMin(int64_t sample, int32_t bucket_size);
+int64_t METRICS_EXPORT GetLinearBucketMin(double sample, int32_t bucket_size);
+
 }  // namespace ukm
 
 #endif  // SERVICES_METRICS_PUBLIC_CPP_METRICS_UTILS_H_

services/metrics/public/cpp/metrics_utils_unittest.cc

+// Copyright 2018 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 "services/metrics/public/cpp/metrics_utils.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(MetricsUtilsTest, GetLinearBucketMin) {
+  struct {
+    int64_t expected_result;
+    int64_t sample;
+    int32_t bucket_size;
+  } int_test_cases[] = {
+      // Typical positive cases.
+      {35, 38, 5},
+      {50, 51, 50},
+      {50, 99, 50},
+      {20, 25, 10},
+      // Negative samples.
+      {-50, -45, 10},
+      {-50, -48, 10},
+      {-50, -41, 10},
+      {-42, -41, 2},
+      // Zero samples.
+      {0, 0, 1},
+      {0, 0, 10},
+  };
+
+  struct {
+    int64_t expected_result;
+    double sample;
+    int32_t bucket_size;
+  } double_test_cases[] = {
+      // Typical positive cases.
+      {35, 38.0, 5},
+      {50, 50.5, 50},
+      {50, 99.5, 50},
+      {20, 25.0, 10},
+      // Negative samples.
+      {-50, -45.0, 10},
+      {-42, -41.2, 2},
+      {-42, -40.8, 2},
+      // Test that a double close to the next bucker never rounds up.
+      {5, 9.95, 5},
+  };
+
+  // Test int64_t sample cases.
+  for (const auto& test : int_test_cases) {
+    EXPECT_EQ(test.expected_result,
+              ukm::GetLinearBucketMin(test.sample, test.bucket_size))
+        << "For sample: " << test.sample
+        << " with bucket_size: " << test.bucket_size;
+  }
+
+  // Test double sample cases.
+  for (const auto& test : double_test_cases) {
+    EXPECT_EQ(test.expected_result,
+              ukm::GetLinearBucketMin(test.sample, test.bucket_size))
+        << "For sample: " << test.sample
+        << " with bucket_size: " << test.bucket_size;
+  }
+}

tools/metrics/ukm/ukm.xml

@@ -2297,6 +2297,13 @@ be describing additional metrics about the same event.
       100.0. Rounded to the nearest integer.
     </summary>
   </metric>
+  <metric name="ScoreFinal">
+    <summary>
+      The final Site Engagement score for the page before it is closed or
+      another navigation occurs. Score is between 0.0 and 100.0; this metric is
+      rounded down to a multiple of 10 to limit granularity.
+    </summary>
+  </metric>
 </event>
 
 <event name="SiteIsolation.XSD.Browser.Blocked">