Enable the sharing of non-HTTPS canonical URLs.

This CL enables the sharing of non-HTTPS canonical URLs if the
display URL is HTTPS, due to a requirements change. It also
updates the logging to treat HTTP canonical URLs as a success
case.

Bug: 794768
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I741949980f2d6d804a673ac82d950cdf735021df
Reviewed-on: https://chromium-review.googlesource.com/826051Reviewed-by: Peter Lee <pkl@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Gregory Chatzinoff <gchatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524433}

ios/chrome/browser/ui/activity_services/canonical_url_retriever.mm

@@ -59,15 +59,9 @@ GURL UrlFromValue(const base::Value* value) {
     // Log result if an invalid canonical URL is found.
     LogCanonicalUrlResultHistogram(
         activity_services::FAILED_CANONICAL_URL_INVALID);
-  } else if (!canonical_url.SchemeIsCryptographic()) {
-    // Logs result if the found canonical URL is not HTTPS.
-    LogCanonicalUrlResultHistogram(
-        activity_services::FAILED_CANONICAL_URL_NOT_HTTPS);
   }
 
-  return canonical_url.is_valid() && canonical_url.SchemeIsCryptographic()
-             ? canonical_url
-             : GURL::EmptyGURL();
+  return canonical_url.is_valid() ? canonical_url : GURL::EmptyGURL();
 }
 }  // namespace
 
@@ -90,11 +84,17 @@ void RetrieveCanonicalUrl(web::WebState* web_state,
         // If the canonical URL is not empty, then the retrieval was successful,
         // and the success can be logged.
         if (!canonical_url.is_empty()) {
+          // Log whether the success occurred with an HTTP canonical URL, a
+          // canonical URL that is the same as the visible URL, or a canonical
+          // URL that is different from the visible URL.
           LogCanonicalUrlResultHistogram(
-              visible_url == canonical_url
-                  ? activity_services::SUCCESS_CANONICAL_URL_SAME_AS_VISIBLE
-                  : activity_services::
-                        SUCCESS_CANONICAL_URL_DIFFERENT_FROM_VISIBLE);
+              !canonical_url.SchemeIsCryptographic()
+                  ? activity_services::SUCCESS_CANONICAL_URL_NOT_HTTPS
+                  : visible_url == canonical_url
+                        ? activity_services::
+                              SUCCESS_CANONICAL_URL_SAME_AS_VISIBLE
+                        : activity_services::
+                              SUCCESS_CANONICAL_URL_DIFFERENT_FROM_VISIBLE);
         }
 
         completion(canonical_url);

ios/chrome/browser/ui/activity_services/canonical_url_retriever_private.h

@@ -14,9 +14,8 @@ enum CanonicalURLResult {
   // The canonical URL retrieval failed because the visible URL is not HTTPS.
   FAILED_VISIBLE_URL_NOT_HTTPS = 0,
 
-  // The canonical URL retrieval failed because the canonical URL is not HTTPS
-  // (but the visible URL is).
-  FAILED_CANONICAL_URL_NOT_HTTPS,
+  // Deprecated.
+  FAILED_CANONICAL_URL_NOT_HTTPS_DEPRECATED,
 
   // The canonical URL retrieval failed because the page did not define one.
   FAILED_NO_CANONICAL_URL_DEFINED,
@@ -33,6 +32,10 @@ enum CanonicalURLResult {
   // same as the visible URL.
   SUCCESS_CANONICAL_URL_SAME_AS_VISIBLE,
 
+  // The canonical URL retrieval succeeded. The canonical URL is not HTTPS
+  // (but the visible URL is).
+  SUCCESS_CANONICAL_URL_NOT_HTTPS,
+
   // The count of canonical URL results. This must be the last item in the enum.
   CANONICAL_URL_RESULT_COUNT
 };

ios/chrome/browser/ui/activity_services/canonical_url_retriever_unittest.mm

@@ -165,8 +165,8 @@ TEST_F(CanonicalURLRetrieverTest, TestCanonicalURLHTTPSUpgrade) {
       activity_services::FAILED_VISIBLE_URL_NOT_HTTPS, 1);
 }
 
-// Validates that if the visible URL is HTTPS but the canonical URL is HTTP, an
-// empty GURL is given to the completion block.
+// Validates that if the visible URL is HTTPS but the canonical URL is HTTP, it
+// is found and given to the completion block.
 TEST_F(CanonicalURLRetrieverTest, TestCanonicalLinkHTTPSDowngrade) {
   LoadHtml(@"<link rel=\"canonical\" href=\"http://chromium.test\">",
            GURL("https://m.chromium.test/"));
@@ -175,8 +175,8 @@ TEST_F(CanonicalURLRetrieverTest, TestCanonicalLinkHTTPSDowngrade) {
   bool success = RetrieveCanonicalUrl(&url);
 
   ASSERT_TRUE(success);
-  EXPECT_TRUE(url.is_empty());
+  EXPECT_EQ("http://chromium.test/", url);
   histogram_tester_.ExpectUniqueSample(
       activity_services::kCanonicalURLResultHistogram,
-      activity_services::FAILED_CANONICAL_URL_NOT_HTTPS, 1);
+      activity_services::SUCCESS_CANONICAL_URL_NOT_HTTPS, 1);
 }

tools/metrics/histograms/enums.xml

@@ -4271,14 +4271,17 @@ uploading your change for review.  These are checked by presubmit scripts.
 <enum name="CanonicalURLResult">
   <int value="0" label="Canonical URL not retrieved: Visible URL not HTTPS"/>
   <int value="1"
-      label="Canonical URL not retrieved: Canonical URL not HTTPS (but
-             visible URL is)"/>
+      label="(obsolete) Canonical URL not retrieved: Canonical URL not HTTPS
+             (but visible URL is)"/>
   <int value="2"
       label="Canonical URL not retrieved: Page had no canonical URL defined"/>
   <int value="3"
       label="Canonical URL not retrieved: Page's canonical URL was invalid"/>
   <int value="4" label="Canonical URL retrieved: Differed from visible URL"/>
   <int value="5" label="Canonical URL retrieved: Same as visible URL"/>
+  <int value="6"
+      label="Canonical URL retrieved: Canonical URL not HTTPS (but visible
+             URL is)"/>
 </enum>
 
 <enum name="CanvasContextType">