Add SignedExchange.LoadResult UMA histogram

Design doc: https://docs.google.com/document/d/1kzQRLybaUk12UgG8YTyACwNZ7tpcXwdjzPXU3CV25J8/edit

Bug: 863305
Change-Id: Ibdfbd06dc354894c2ef92db878828d171adaa75f
Reviewed-on: https://chromium-review.googlesource.com/1226758Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592659}

content/browser/web_package/signed_exchange_loader.cc

@@ -8,6 +8,7 @@
 
 #include "base/callback.h"
 #include "base/feature_list.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "content/browser/loader/data_pipe_to_source_stream.h"
 #include "content/browser/loader/source_stream_to_data_pipe.h"
@@ -29,6 +30,8 @@ namespace content {
 
 namespace {
 
+constexpr char kLoadResultHistogram[] = "SignedExchange.LoadResult";
+
 net::RedirectInfo CreateRedirectInfo(const GURL& new_url,
                                      const GURL& outer_request_url) {
   net::RedirectInfo redirect_info;
@@ -129,6 +132,8 @@ SignedExchangeLoader::SignedExchangeLoader(
   // transport layer, and MUST NOT accept exchanges transferred over plain HTTP
   // without TLS. [spec text]
   if (!IsOriginSecure(outer_request_url)) {
+    UMA_HISTOGRAM_ENUMERATION(kLoadResultHistogram,
+                              SignedExchangeLoadResult::kSXGServedFromNonHTTPS);
     devtools_proxy_->ReportError(
         "Signed exchange response from non secure origin is not supported.",
         base::nullopt /* error_field */);
@@ -272,6 +277,8 @@ void SignedExchangeLoader::OnHTTPExchangeFound(
     const std::string& request_method,
     const network::ResourceResponseHead& resource_response,
     std::unique_ptr<net::SourceStream> payload_stream) {
+  UMA_HISTOGRAM_ENUMERATION(kLoadResultHistogram, result);
+
   if (error) {
     if (error != net::ERR_INVALID_SIGNED_EXCHANGE ||
         !should_redirect_on_failure_ || !request_url.is_valid()) {

content/browser/web_package/signed_exchange_request_handler_browsertest.cc

@@ -6,6 +6,7 @@
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
@@ -137,6 +138,7 @@ class SignedExchangeRequestHandlerBrowserTest : public CertVerifierBrowserTest {
   }
 
   base::test::ScopedFeatureList feature_list_;
+  const base::HistogramTester histogram_tester_;
 
  private:
   static void InstallMockInterceptors(const GURL& url,
@@ -217,6 +219,8 @@ IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerBrowserTest, Simple) {
       net::X509Certificate::CalculateFingerprint256(
           original_cert->cert_buffer());
   EXPECT_EQ(original_fingerprint, fingerprint);
+  histogram_tester_.ExpectUniqueSample("SignedExchange.LoadResult",
+                                       SignedExchangeLoadResult::kSuccess, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerBrowserTest,
@@ -250,6 +254,9 @@ IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerBrowserTest,
   NavigateToURL(shell(), url);
   EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
   EXPECT_EQ(303, redirect_observer.response_code());
+  histogram_tester_.ExpectUniqueSample(
+      "SignedExchange.LoadResult", SignedExchangeLoadResult::kVersionMismatch,
+      1);
 }
 
 IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerBrowserTest,
@@ -276,6 +283,13 @@ IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerBrowserTest,
     NavigateToURL(shell(), url);
     EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
   }
+  histogram_tester_.ExpectTotalCount("SignedExchange.LoadResult", 2);
+  histogram_tester_.ExpectBucketCount(
+      "SignedExchange.LoadResult", SignedExchangeLoadResult::kVersionMismatch,
+      1);
+  histogram_tester_.ExpectBucketCount(
+      "SignedExchange.LoadResult", SignedExchangeLoadResult::kHeaderParseError,
+      1);
 }
 
 IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerBrowserTest, CertNotFound) {
@@ -292,6 +306,9 @@ IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerBrowserTest, CertNotFound) {
   TitleWatcher title_watcher(shell()->web_contents(), title);
   NavigateToURL(shell(), url);
   EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
+  histogram_tester_.ExpectUniqueSample(
+      "SignedExchange.LoadResult", SignedExchangeLoadResult::kCertFetchError,
+      1);
 }
 
 class SignedExchangeRequestHandlerRealCertVerifierBrowserTest
@@ -329,23 +346,13 @@ IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerRealCertVerifierBrowserTest,
   // TODO(https://crbug.com/815024): Make this test pass the OCSP check. We'll
   // need to either generate an OCSP response on the fly, or override the OCSP
   // verification time.
-  content::ConsoleObserverDelegate console_observer(shell()->web_contents(),
-                                                    "*OCSP*");
-  shell()->web_contents()->SetDelegate(&console_observer);
-
   base::string16 title = base::ASCIIToUTF16("Fallback URL response");
   TitleWatcher title_watcher(shell()->web_contents(), title);
   NavigateToURL(shell(), url);
   EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
-
   // Verify that it failed at the OCSP check step.
-  // TODO(https://crbug.com/803774): Find a better way than matching against the
-  // error message. We can probably make DevToolsProxy derive some context from
-  // StoragePartition so that we can record and extract the detailed error
-  // status for testing via that.
-  console_observer.Wait();
-  EXPECT_TRUE(base::StartsWith(console_observer.message(), "OCSP check failed:",
-                               base::CompareCase::SENSITIVE));
+  histogram_tester_.ExpectUniqueSample("SignedExchange.LoadResult",
+                                       SignedExchangeLoadResult::kOCSPError, 1);
 }
 
 }  // namespace content

tools/metrics/histograms/enums.xml

@@ -44822,6 +44822,22 @@ Called by update_net_trust_anchors.py.-->
   <int value="2" label="Dismiss"/>
 </enum>
 
+<enum name="SignedExchangeLoadResult">
+  <int value="0" label="Loaded successfully"/>
+  <int value="1" label="Served from non-secure origin"/>
+  <int value="2" label="Parse Error (no fallback URL)"/>
+  <int value="3" label="Unsupported SXG version (has fallback URL)"/>
+  <int value="4" label="Parse Error (has fallback URL)"/>
+  <int value="5" label="Network error"/>
+  <int value="6" label="Cert fetch error"/>
+  <int value="7" label="Cert parse error"/>
+  <int value="8" label="Signature verification error"/>
+  <int value="9" label="Cert verification error"/>
+  <int value="10" label="CT verification error"/>
+  <int value="11" label="OCSP error"/>
+  <int value="12" label="Certificate requirements not met"/>
+</enum>
+
 <enum name="SigninAccessPoint">
   <int value="0" label="Start page"/>
   <int value="1" label="NTP Link"/>

tools/metrics/histograms/histograms.xml

@@ -96757,6 +96757,14 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="SignedExchange.LoadResult" enum="SignedExchangeLoadResult">
+  <owner>ksakamoto@chromium.org</owner>
+  <summary>
+    Records the result of loading a resource from Signed HTTP Exchange. Emitted
+    each time a response is handled as Signed Exchange.
+  </summary>
+</histogram>
+
 <histogram name="Signin" enum="SigninHelperFlow">
   <owner>mlerman@chromium.org</owner>
   <summary>