Add a histogram entry for same-site but cross-origin downloads

This CL adds a new bucket to Download.InitiatedByWindowOpener to
indicate that a download is initiated by a same-site but
cross-origin opener of that tab. E.g., if a tab
subdomain1.example.com opens another tab subdomain2.example.com and
then navigates it to a download, this CL will now record the download
as kSameSite instead of kCrossOrigin.

This is added because latest number from Canary for the kCrossOrigin
bucket is unexpectedly large. I suspect this is caused by sites serving
their downloads on subdomains. This would previously go to the
kCrossOrigin bucket but it now goes under kSameSite bucket.

Bug: 121259
Change-Id: Idf142448b052a342df057982cfb79b0669fbd229
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2065252
Auto-Submit: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743183}

content/browser/download/download_browsertest.cc

@@ -110,7 +110,12 @@ const int kPauseOffset = 100 * 1024;
 
 const char kOriginOne[] = "one.example";
 const char kOriginTwo[] = "two.example";
-const char kOriginThree[] = "example.com";
+const char kOrigin[] = "example.com";
+const char kOriginSubdomain[] = "subdomain.example.com";
+const char kOtherOrigin[] = "example.site";
+const char kBlogspotSite1[] = "a.blogspot.com";
+const char kBlogspotSite2[] = "b.blogspot.com";
+
 const char k404Response[] = "HTTP/1.1 404 Not found\r\n\r\n";
 
 void ExpectRequestNetworkIsolationKey(
@@ -909,7 +914,11 @@ class DownloadContentTest : public ContentBrowserTest {
         embedded_test_server()->host_port_pair().host();
     host_resolver()->AddRule(kOriginOne, real_host);
     host_resolver()->AddRule(kOriginTwo, real_host);
-    host_resolver()->AddRule(kOriginThree, real_host);
+    host_resolver()->AddRule(kOrigin, real_host);
+    host_resolver()->AddRule(kOriginSubdomain, real_host);
+    host_resolver()->AddRule(kOtherOrigin, real_host);
+    host_resolver()->AddRule(kBlogspotSite1, real_host);
+    host_resolver()->AddRule(kBlogspotSite2, real_host);
     host_resolver()->AddRule(SlowDownloadHttpResponse::kSlowResponseHostName,
                              real_host);
     host_resolver()->AddRule(TestDownloadHttpResponse::kTestDownloadHostName,
@@ -1446,39 +1455,18 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, MultiDownload) {
       file2, GetTestFilePath("download", "download-test.lib")));
 }
 
-#if BUILDFLAG(ENABLE_PLUGINS)
-// Content served with a MIME type of application/octet-stream should be
-// downloaded even when a plugin can be found that handles the file type.
-// See https://crbug.com/104331 for the details.
-IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadOctetStream) {
-  const char kTestPluginName[] = "TestPlugin";
-  const char kTestMimeType[] = "application/x-test-mime-type";
-  const char kTestFileType[] = "abc";
-
-  WebPluginInfo plugin_info;
-  plugin_info.name = base::ASCIIToUTF16(kTestPluginName);
-  plugin_info.mime_types.push_back(
-      WebPluginMimeType(kTestMimeType, kTestFileType, ""));
-  plugin_info.type = WebPluginInfo::PLUGIN_TYPE_PEPPER_IN_PROCESS;
-  PluginServiceImpl::GetInstance()->RegisterInternalPlugin(plugin_info, false);
-
-  // The following is served with a Content-Type of application/octet-stream.
-  NavigateToURLAndWaitForDownload(
-      shell(), embedded_test_server()->GetURL("/download/octet-stream.abc"),
-      download::DownloadItem::COMPLETE);
-}
-
 // Tests that metrics are recorded when a page opens a named window, navigates
 // it to a URL, then navigates it again to a download. The navigated URL is same
-// origin as the opener.
+// origin as the opener (example.com). The actual download URL doesn't matter.
 IN_PROC_BROWSER_TEST_F(DownloadContentTest,
                        InitiatedByWindowOpener_SameOrigin) {
-  EXPECT_TRUE(NavigateToURL(shell()->web_contents(),
-                            embedded_test_server()->GetURL("/empty.html")));
+  EXPECT_TRUE(
+      NavigateToURL(shell()->web_contents(),
+                    embedded_test_server()->GetURL(kOrigin, "/empty.html")));
 
   // From the initial tab, open a window named 'foo' and navigate it to a same
   // origin page.
-  const GURL url = embedded_test_server()->GetURL("/title1.html");
+  const GURL url = embedded_test_server()->GetURL(kOrigin, "/title1.html");
   const std::string script = "window.open('" + url.spec() + "', 'foo')";
   ShellAddedObserver new_shell_observer;
   EXPECT_TRUE(ExecuteScript(shell()->web_contents(), script));
@@ -1490,8 +1478,8 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
   // completion.
   base::HistogramTester histogram_tester;
   std::unique_ptr<DownloadTestObserver> observer(CreateWaiter(new_shell, 1));
-  const GURL download_url =
-      embedded_test_server()->GetURL("/download/download-test.lib");
+  const GURL download_url = embedded_test_server()->GetURL(
+      kOtherOrigin, "/download/download-test.lib");
   const std::string download_script =
       "window.open('" + download_url.spec() + "', 'foo')";
   EXPECT_TRUE(ExecuteScript(shell()->web_contents(), download_script));
@@ -1503,7 +1491,82 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
       static_cast<int>(InitiatedByWindowOpenerType::kSameOrigin), 1);
 }
 
-// Same as above, but the navigated URL is cross origin to the opener.
+// Same as InitiatedByWindowOpener_SameOrigin, but the navigated URL is same
+// site as the opener (example.com vs one.example.com).
+IN_PROC_BROWSER_TEST_F(DownloadContentTest, InitiatedByWindowOpener_SameSite) {
+  EXPECT_TRUE(
+      NavigateToURL(shell()->web_contents(),
+                    embedded_test_server()->GetURL(kOrigin, "/empty.html")));
+
+  // From the initial tab, open a window named 'foo' and navigate it to a
+  // subdomain. This is cross-origin but same site.
+  const GURL url =
+      embedded_test_server()->GetURL(kOriginSubdomain, "/title1.html");
+
+  const std::string script = "window.open('" + url.spec() + "', 'foo')";
+  ShellAddedObserver new_shell_observer;
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), script));
+  Shell* new_shell = new_shell_observer.GetShell();
+  ASSERT_TRUE(new_shell);
+  WaitForLoadStop(new_shell->web_contents());
+
+  // From the initial tab, navigate the 'foo' window to a download and wait for
+  // completion.
+  base::HistogramTester histogram_tester;
+  std::unique_ptr<DownloadTestObserver> observer(CreateWaiter(new_shell, 1));
+  const GURL download_url = embedded_test_server()->GetURL(
+      kOtherOrigin, "/download/download-test.lib");
+  const std::string download_script =
+      "window.open('" + download_url.spec() + "', 'foo')";
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), download_script));
+  observer->WaitForFinished();
+
+  histogram_tester.ExpectTotalCount("Download.InitiatedByWindowOpener", 1);
+  histogram_tester.ExpectUniqueSample(
+      "Download.InitiatedByWindowOpener",
+      static_cast<int>(InitiatedByWindowOpenerType::kSameSite), 1);
+}
+
+// The opener and the openee are under the same domain name blogspot.com, but
+// blogspot.com is a private registry according to the Public Suffix List, so
+// its subdomains are not considered same host.
+IN_PROC_BROWSER_TEST_F(DownloadContentTest,
+                       InitiatedByWindowOpener_PrivateRegistry) {
+  EXPECT_TRUE(NavigateToURL(
+      shell()->web_contents(),
+      embedded_test_server()->GetURL(kBlogspotSite1, "/empty.html")));
+
+  // From the initial tab, open a window named 'foo' and navigate it to another
+  // subdomain of blogspot.com.
+  const GURL url =
+      embedded_test_server()->GetURL(kBlogspotSite2, "/title1.html");
+
+  const std::string script = "window.open('" + url.spec() + "', 'foo')";
+  ShellAddedObserver new_shell_observer;
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), script));
+  Shell* new_shell = new_shell_observer.GetShell();
+  ASSERT_TRUE(new_shell);
+  WaitForLoadStop(new_shell->web_contents());
+
+  // From the initial tab, navigate the 'foo' window to a download and wait for
+  // completion.
+  base::HistogramTester histogram_tester;
+  std::unique_ptr<DownloadTestObserver> observer(CreateWaiter(new_shell, 1));
+  const GURL download_url = embedded_test_server()->GetURL(
+      kOtherOrigin, "/download/download-test.lib");
+  const std::string download_script =
+      "window.open('" + download_url.spec() + "', 'foo')";
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), download_script));
+  observer->WaitForFinished();
+
+  histogram_tester.ExpectTotalCount("Download.InitiatedByWindowOpener", 1);
+  histogram_tester.ExpectUniqueSample(
+      "Download.InitiatedByWindowOpener",
+      static_cast<int>(InitiatedByWindowOpenerType::kCrossOrigin), 1);
+}
+
+// Same as InitiatedByWindowOpener_SameOrigin, but the navigated URL is cross
+// origin to the opener (example.com vs example.site).
 IN_PROC_BROWSER_TEST_F(DownloadContentTest,
                        InitiatedByWindowOpener_CrossOrigin) {
   EXPECT_TRUE(NavigateToURL(shell()->web_contents(),
@@ -1511,9 +1574,10 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
 
   // From the initial tab, open a window named 'foo' and navigate it to a cross
   // origin page.
+  const GURL url = embedded_test_server()->GetURL(kOtherOrigin, "/title1.html");
   ShellAddedObserver new_shell_observer;
   EXPECT_TRUE(ExecuteScript(shell()->web_contents(),
-                            "window.open('http://a.com', 'foo')"));
+                            "window.open('" + url.spec() + "', 'foo')"));
   Shell* new_shell = new_shell_observer.GetShell();
   ASSERT_TRUE(new_shell);
   WaitForLoadStop(new_shell->web_contents());
@@ -1522,8 +1586,8 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
   // completion.
   base::HistogramTester histogram_tester;
   std::unique_ptr<DownloadTestObserver> observer(CreateWaiter(new_shell, 1));
-  const GURL download_url =
-      embedded_test_server()->GetURL("/download/download-test.lib");
+  const GURL download_url = embedded_test_server()->GetURL(
+      kOtherOrigin, "/download/download-test.lib");
   const std::string download_script =
       "window.open('" + download_url.spec() + "', 'foo')";
   EXPECT_TRUE(ExecuteScript(shell()->web_contents(), download_script));
@@ -1535,6 +1599,28 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
       static_cast<int>(InitiatedByWindowOpenerType::kCrossOrigin), 1);
 }
 
+#if BUILDFLAG(ENABLE_PLUGINS)
+// Content served with a MIME type of application/octet-stream should be
+// downloaded even when a plugin can be found that handles the file type.
+// See https://crbug.com/104331 for the details.
+IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadOctetStream) {
+  const char kTestPluginName[] = "TestPlugin";
+  const char kTestMimeType[] = "application/x-test-mime-type";
+  const char kTestFileType[] = "abc";
+
+  WebPluginInfo plugin_info;
+  plugin_info.name = base::ASCIIToUTF16(kTestPluginName);
+  plugin_info.mime_types.push_back(
+      WebPluginMimeType(kTestMimeType, kTestFileType, ""));
+  plugin_info.type = WebPluginInfo::PLUGIN_TYPE_PEPPER_IN_PROCESS;
+  PluginServiceImpl::GetInstance()->RegisterInternalPlugin(plugin_info, false);
+
+  // The following is served with a Content-Type of application/octet-stream.
+  NavigateToURLAndWaitForDownload(
+      shell(), embedded_test_server()->GetURL("/download/octet-stream.abc"),
+      download::DownloadItem::COMPLETE);
+}
+
 // Content served with a MIME type of application/octet-stream should be
 // downloaded even when a plugin can be found that handles the file type.
 // See https://crbug.com/104331 for the details.

content/browser/download/download_manager_impl.cc

@@ -71,6 +71,7 @@
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "net/base/elements_upload_data_stream.h"
 #include "net/base/load_flags.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/base/request_priority.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "services/metrics/public/cpp/ukm_source_id.h"
@@ -1209,8 +1210,16 @@ void DownloadManagerImpl::InterceptNavigationOnChecksComplete(
     if (opener) {
       if (opener->GetLastCommittedOrigin() !=
           render_frame_host->GetLastCommittedOrigin()) {
-        UMA_HISTOGRAM_ENUMERATION("Download.InitiatedByWindowOpener",
-                                  InitiatedByWindowOpenerType::kCrossOrigin);
+        if (net::registry_controlled_domains::SameDomainOrHost(
+                opener->GetLastCommittedOrigin(),
+                render_frame_host->GetLastCommittedOrigin(),
+                net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
+          UMA_HISTOGRAM_ENUMERATION("Download.InitiatedByWindowOpener",
+                                    InitiatedByWindowOpenerType::kSameSite);
+        } else {
+          UMA_HISTOGRAM_ENUMERATION("Download.InitiatedByWindowOpener",
+                                    InitiatedByWindowOpenerType::kCrossOrigin);
+        }
       } else {
         UMA_HISTOGRAM_ENUMERATION("Download.InitiatedByWindowOpener",
                                   InitiatedByWindowOpenerType::kSameOrigin);

content/browser/download/download_manager_impl.h

@@ -48,8 +48,12 @@ class DownloadItemImpl;
 // Entries should not be renumbered and numeric values should never be reused.
 enum class InitiatedByWindowOpenerType {
   kSameOrigin = 0,
+  // Download URL and the initiator are cross origin.
   kCrossOrigin = 1,
-  kMaxValue = kCrossOrigin
+  // Download URL and the initiator are cross origin but same site (i.e. same
+  // eTLD+1).
+  kSameSite = 2,
+  kMaxValue = kSameSite
 };
 
 namespace content {

tools/metrics/histograms/enums.xml

@@ -16336,6 +16336,10 @@ to ensure that the crash string is shown properly on the user-facing crash UI.
   <int value="1" label="Cross Origin">
     window.opener is cross origin to the top frame of this download.
   </int>
+  <int value="2" label="Same Site">
+    window.opener is cross origin but same site to the top frame of this
+    download (e.g. google.com vs subdomain.google.com).
+  </int>
 </enum>
 
 <enum name="DownloadInProgressDBCountType">