Use the navigation IsolationInfo for downloading an opened PDF

What: When downloading a PDF that is already opened, use the original
navigation isolation info.

Why: When a subframe PDF is opened and then is manually downloaded, the
network isolation key for the 2 requests are inconsistent. The
navigation request is using <top frame origin, subframe origin>, whereas
the download request is using the <pdf url origin (== subframe origin),
pdf url origin>.

This not only has the typical performance & privacy problem, but also
could give the user an unexpected cached PDF instead of the latest
PDF opened -- if there are 2 PDFs opened with the same URL but different
contents, and we try to save the latest one. (Note that the fundamental
issue is not solvable by fixing the isolation info per
https://bugs.chromium.org/p/chromium/issues/detail?id=1171524#c20,
but making it download the latest opened PDF is still more desirable
than making it download an old one).

How: Add a |rfh| parameter in WebContents::SaveFrame(WithHeaders), to
denote the original navigation targeted frame, so that we can recover
the origin navigation isolation info from there. Fill in this info in
the caller sites:
1. PDFWebContentsHelper::SaveUrlAs
2. RenderViewContextMenu::ExecSaveAs
3. WebContentsImpl::OnSavePage

Specifically, if we have an inner-web-contents PDF, we will pass its
outer-web-contents-frame; otherwise, pass the current frame host.

In download_utils.cc::CreateResourceRequest, if the isolation_info is
already filled in, use the given isolation info & site-for-cookies;
otherwise, keep the existing behavior i.e. use <url origin, url origin>
as the isolation key.

Bug: 1174216
Change-Id: Ibba85358a4da9efedaa5510b70f7279412685d56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2689861
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: K. Moon <kmoon@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#857983}

chrome/browser/download/download_browsertest.cc

@@ -93,6 +93,7 @@
 #include "components/infobars/core/confirm_infobar_delegate.h"
 #include "components/infobars/core/infobar.h"
 #include "components/metrics/content/subprocess_metrics_provider.h"
+#include "components/pdf/browser/pdf_web_contents_helper.h"
 #include "components/permissions/permission_request_manager.h"
 #include "components/prefs/pref_service.h"
 #include "components/reputation/core/safety_tip_test_utils.h"
@@ -132,6 +133,7 @@
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/scoped_ignore_content_verifier_for_test.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "net/base/features.h"
 #include "net/base/filename_util.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -170,6 +172,24 @@ using net::test_server::EmbeddedTestServer;
 
 namespace {
 
+class InnerWebContentsAttachedWaiter : public content::WebContentsObserver {
+ public:
+  // Observes navigation for the specified |web_contents|.
+  explicit InnerWebContentsAttachedWaiter(WebContents* web_contents)
+      : content::WebContentsObserver(web_contents) {}
+
+  void InnerWebContentsAttached(WebContents* inner_web_contents,
+                                content::RenderFrameHost* render_frame_host,
+                                bool is_full_page) override {
+    run_loop_.Quit();
+  }
+
+  void Wait() { run_loop_.Run(); }
+
+ private:
+  base::RunLoop run_loop_{base::RunLoop::Type::kNestableTasksAllowed};
+};
+
 const char kDownloadTest1Path[] = "download-test1.lib";
 
 void VerifyNewDownloadId(uint32_t expected_download_id, uint32_t download_id) {
@@ -2621,6 +2641,347 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, TransientDownload) {
   ASSERT_FALSE(downloads[0]->IsTemporary());
 }
 
+class DownloadTestSplitCacheEnabled : public DownloadTest {
+ public:
+  DownloadTestSplitCacheEnabled() {
+    feature_list_.InitWithFeatures(
+        {net::features::kSplitCacheByNetworkIsolationKey}, {});
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(DownloadTestSplitCacheEnabled,
+                       SaveMainFramePdfFromContextMenu_IsolationInfo) {
+  embedded_test_server()->ServeFilesFromDirectory(GetTestDataDirectory());
+  ASSERT_TRUE(embedded_test_server()->Start());
+  EnableFileChooser(true);
+
+  net::SiteForCookies expected_site_for_cookies =
+      net::SiteForCookies::FromOrigin(
+          url::Origin::Create(embedded_test_server()->GetURL("foo.com", "/")));
+
+  net::IsolationInfo expected_isolation_info = net::IsolationInfo::Create(
+      net::IsolationInfo::RequestType::kMainFrame,
+      url::Origin::Create(embedded_test_server()->GetURL("foo.com", "/")),
+      url::Origin::Create(embedded_test_server()->GetURL("foo.com", "/")),
+      expected_site_for_cookies, std::set<net::SchemefulSite>());
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Set up a PDF page.
+  GURL url = embedded_test_server()->GetURL("foo.com", "/pdf/test.pdf");
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  std::vector<content::WebContents*> inner_web_contents_vector =
+      web_contents->GetInnerWebContents();
+  ASSERT_EQ(1u, inner_web_contents_vector.size());
+  content::WebContents* inner_web_contents = inner_web_contents_vector.front();
+
+  // Wait for the page to finish loading.
+  if (inner_web_contents->IsLoading()) {
+    content::TestNavigationObserver inner_navigation_waiter(inner_web_contents);
+    inner_navigation_waiter.Wait();
+    ASSERT_TRUE(!inner_web_contents->IsLoading());
+  }
+
+  // Stop the server. This makes sure we really are pulling from the cache for
+  // the download request.
+  ASSERT_TRUE(embedded_test_server()->ShutdownAndWaitUntilComplete());
+
+  base::Optional<network::ResourceRequest::TrustedParams> trusted_params;
+  net::SiteForCookies site_for_cookies;
+
+  base::RunLoop request_waiter;
+  URLLoaderInterceptor request_listener(base::BindLambdaForTesting(
+      [&](URLLoaderInterceptor::RequestParams* params) {
+        if (params->url_request.url == url) {
+          trusted_params = params->url_request.trusted_params;
+          site_for_cookies = params->url_request.site_for_cookies;
+          request_waiter.Quit();
+        }
+        return false;
+      }));
+
+  std::unique_ptr<content::DownloadTestObserver> download_waiter(
+      CreateWaiter(browser(), 1));
+
+  // Simulate saving the PDF from the context menu "Save As...".
+  content::ContextMenuParams context_menu_params;
+  context_menu_params.media_type =
+      blink::mojom::ContextMenuDataMediaType::kPlugin;
+  context_menu_params.src_url = url;
+  context_menu_params.page_url = inner_web_contents->GetLastCommittedURL();
+  TestRenderViewContextMenu menu(inner_web_contents->GetMainFrame(),
+                                 context_menu_params);
+  menu.Init();
+  menu.ExecuteCommand(IDC_SAVE_PAGE, 0);
+
+  request_waiter.Run();
+
+  EXPECT_TRUE(trusted_params.has_value());
+  EXPECT_TRUE(trusted_params->isolation_info.IsEqualForTesting(
+      expected_isolation_info));
+  EXPECT_TRUE(site_for_cookies.IsEquivalent(expected_site_for_cookies));
+
+  download_waiter->WaitForFinished();
+
+  EXPECT_EQ(1u,
+            download_waiter->NumDownloadsSeenInState(DownloadItem::COMPLETE));
+  CheckDownloadStates(1, DownloadItem::COMPLETE);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTestSplitCacheEnabled,
+                       SaveSubframePdfFromPdfUI_IsolationInfo) {
+  embedded_test_server()->ServeFilesFromDirectory(GetTestDataDirectory());
+  ASSERT_TRUE(embedded_test_server()->Start());
+  EnableFileChooser(true);
+
+  net::SiteForCookies expected_site_for_cookies =
+      net::SiteForCookies::FromOrigin(
+          url::Origin::Create(embedded_test_server()->GetURL("foo.com", "/")));
+
+  net::IsolationInfo expected_isolation_info = net::IsolationInfo::Create(
+      net::IsolationInfo::RequestType::kSubFrame,
+      url::Origin::Create(embedded_test_server()->GetURL("foo.com", "/")),
+      url::Origin::Create(embedded_test_server()->GetURL("bar.com", "/")),
+      expected_site_for_cookies, std::set<net::SchemefulSite>());
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Set up a page with a cross-origin iframe hosting a PDF.
+  GURL url = embedded_test_server()->GetURL("foo.com", "/iframe.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  GURL subframe_url(embedded_test_server()->GetURL("bar.com", "/pdf/test.pdf"));
+
+  InnerWebContentsAttachedWaiter waiter(web_contents);
+  content::BeginNavigateIframeToURL(web_contents,
+                                    /*iframe_id=*/"test", subframe_url);
+  waiter.Wait();
+
+  std::vector<content::WebContents*> inner_web_contents_vector =
+      web_contents->GetInnerWebContents();
+  ASSERT_EQ(1u, inner_web_contents_vector.size());
+  content::WebContents* inner_web_contents = inner_web_contents_vector.front();
+
+  // Wait for the page to finish loading.
+  if (inner_web_contents->IsLoading()) {
+    content::TestNavigationObserver inner_navigation_waiter(inner_web_contents);
+    inner_navigation_waiter.Wait();
+    ASSERT_TRUE(!inner_web_contents->IsLoading());
+  }
+
+  // Stop the server. This makes sure we really are pulling from the cache for
+  // the download request.
+  ASSERT_TRUE(embedded_test_server()->ShutdownAndWaitUntilComplete());
+
+  base::Optional<network::ResourceRequest::TrustedParams> trusted_params;
+  net::SiteForCookies site_for_cookies;
+
+  base::RunLoop request_waiter;
+  URLLoaderInterceptor request_listener(base::BindLambdaForTesting(
+      [&](URLLoaderInterceptor::RequestParams* params) {
+        if (params->url_request.url == subframe_url) {
+          trusted_params = params->url_request.trusted_params;
+          site_for_cookies = params->url_request.site_for_cookies;
+          request_waiter.Quit();
+        }
+        return false;
+      }));
+
+  std::unique_ptr<content::DownloadTestObserver> download_waiter(
+      CreateWaiter(browser(), 1));
+
+  // Simulate saving the PDF from the UI.
+  pdf::PDFWebContentsHelper* pdf_helper =
+      pdf::PDFWebContentsHelper::FromWebContents(inner_web_contents);
+  blink::mojom::ReferrerPtr referrer = blink::mojom::Referrer::New();
+  referrer->url = subframe_url;
+  referrer->policy =
+      network::mojom::ReferrerPolicy::kStrictOriginWhenCrossOrigin;
+  static_cast<pdf::mojom::PdfService*>(pdf_helper)
+      ->SaveUrlAs(subframe_url, std::move(referrer));
+
+  request_waiter.Run();
+
+  EXPECT_TRUE(trusted_params.has_value());
+  EXPECT_TRUE(trusted_params->isolation_info.IsEqualForTesting(
+      expected_isolation_info));
+  EXPECT_TRUE(site_for_cookies.IsEquivalent(expected_site_for_cookies));
+
+  download_waiter->WaitForFinished();
+  EXPECT_EQ(1u,
+            download_waiter->NumDownloadsSeenInState(DownloadItem::COMPLETE));
+  CheckDownloadStates(1, DownloadItem::COMPLETE);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTestSplitCacheEnabled,
+                       SaveSubframeImageFromContextMenu_IsolationInfo) {
+  embedded_test_server()->ServeFilesFromDirectory(GetTestDataDirectory());
+  ASSERT_TRUE(embedded_test_server()->Start());
+  EnableFileChooser(true);
+
+  net::SiteForCookies expected_site_for_cookies =
+      net::SiteForCookies::FromOrigin(
+          url::Origin::Create(embedded_test_server()->GetURL("foo.com", "/")));
+
+  net::IsolationInfo expected_isolation_info = net::IsolationInfo::Create(
+      net::IsolationInfo::RequestType::kSubFrame,
+      url::Origin::Create(embedded_test_server()->GetURL("foo.com", "/")),
+      url::Origin::Create(embedded_test_server()->GetURL("bar.com", "/")),
+      expected_site_for_cookies, std::set<net::SchemefulSite>());
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Set up a page with a cross-origin iframe hosting a PDF.
+  GURL url = embedded_test_server()->GetURL("foo.com", "/iframe.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  GURL subframe_url(
+      embedded_test_server()->GetURL("bar.com", "/downloads/image.jpg"));
+  content::NavigateIframeToURL(web_contents,
+                               /*iframe_id=*/"test", subframe_url);
+
+  // Stop the server. This makes sure we really are pulling from the cache for
+  // the download request.
+  ASSERT_TRUE(embedded_test_server()->ShutdownAndWaitUntilComplete());
+
+  base::Optional<network::ResourceRequest::TrustedParams> trusted_params;
+  net::SiteForCookies site_for_cookies;
+
+  base::RunLoop request_waiter;
+  URLLoaderInterceptor request_listener(base::BindLambdaForTesting(
+      [&](URLLoaderInterceptor::RequestParams* params) {
+        if (params->url_request.url == subframe_url) {
+          trusted_params = params->url_request.trusted_params;
+          site_for_cookies = params->url_request.site_for_cookies;
+          request_waiter.Quit();
+        }
+        return false;
+      }));
+
+  std::unique_ptr<content::DownloadTestObserver> download_waiter(
+      CreateWaiter(browser(), 1));
+
+  // Simulate saving the image from the context menu "Save As..."
+  content::ContextMenuParams context_menu_params;
+  context_menu_params.media_type =
+      blink::mojom::ContextMenuDataMediaType::kImage;
+  context_menu_params.src_url = subframe_url;
+  context_menu_params.page_url =
+      content::ChildFrameAt(web_contents->GetMainFrame(), 0)
+          ->GetLastCommittedURL();
+  TestRenderViewContextMenu menu(
+      content::ChildFrameAt(web_contents->GetMainFrame(), 0),
+      context_menu_params);
+  menu.Init();
+  menu.ExecuteCommand(IDC_CONTENT_CONTEXT_SAVEIMAGEAS, 0);
+
+  request_waiter.Run();
+
+  EXPECT_TRUE(trusted_params.has_value());
+  EXPECT_TRUE(trusted_params->isolation_info.IsEqualForTesting(
+      expected_isolation_info));
+  EXPECT_TRUE(site_for_cookies.IsEquivalent(expected_site_for_cookies));
+
+  download_waiter->WaitForFinished();
+
+  EXPECT_EQ(1u,
+            download_waiter->NumDownloadsSeenInState(DownloadItem::COMPLETE));
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTestSplitCacheEnabled,
+                       SaveSubframePdfFromContextMenu_IsolationInfo) {
+  embedded_test_server()->ServeFilesFromDirectory(GetTestDataDirectory());
+  ASSERT_TRUE(embedded_test_server()->Start());
+  EnableFileChooser(true);
+
+  net::SiteForCookies expected_site_for_cookies =
+      net::SiteForCookies::FromOrigin(
+          url::Origin::Create(embedded_test_server()->GetURL("foo.com", "/")));
+
+  net::IsolationInfo expected_isolation_info = net::IsolationInfo::Create(
+      net::IsolationInfo::RequestType::kSubFrame,
+      url::Origin::Create(embedded_test_server()->GetURL("foo.com", "/")),
+      url::Origin::Create(embedded_test_server()->GetURL("bar.com", "/")),
+      expected_site_for_cookies, std::set<net::SchemefulSite>());
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Set up a page with a cross-origin iframe hosting a PDF.
+  GURL url = embedded_test_server()->GetURL("foo.com", "/iframe.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  GURL subframe_url(embedded_test_server()->GetURL("bar.com", "/pdf/test.pdf"));
+
+  InnerWebContentsAttachedWaiter waiter(web_contents);
+  content::BeginNavigateIframeToURL(web_contents,
+                                    /*iframe_id=*/"test", subframe_url);
+  waiter.Wait();
+
+  std::vector<content::WebContents*> inner_web_contents_vector =
+      web_contents->GetInnerWebContents();
+  ASSERT_EQ(1u, inner_web_contents_vector.size());
+  content::WebContents* inner_web_contents = inner_web_contents_vector.front();
+
+  // Wait for the page to finish loading.
+  if (inner_web_contents->IsLoading()) {
+    content::TestNavigationObserver inner_navigation_waiter(inner_web_contents);
+    inner_navigation_waiter.Wait();
+    ASSERT_TRUE(!inner_web_contents->IsLoading());
+  }
+
+  // Stop the server. This makes sure we really are pulling from the cache for
+  // the download request.
+  ASSERT_TRUE(embedded_test_server()->ShutdownAndWaitUntilComplete());
+
+  base::Optional<network::ResourceRequest::TrustedParams> trusted_params;
+  net::SiteForCookies site_for_cookies;
+
+  base::RunLoop request_waiter;
+  URLLoaderInterceptor request_listener(base::BindLambdaForTesting(
+      [&](URLLoaderInterceptor::RequestParams* params) {
+        if (params->url_request.url == subframe_url) {
+          trusted_params = params->url_request.trusted_params;
+          site_for_cookies = params->url_request.site_for_cookies;
+          request_waiter.Quit();
+        }
+        return false;
+      }));
+
+  std::unique_ptr<content::DownloadTestObserver> download_waiter(
+      CreateWaiter(browser(), 1));
+
+  // Simulate saving the PDF from the context menu "Save As..."
+  content::ContextMenuParams context_menu_params;
+  context_menu_params.media_type =
+      blink::mojom::ContextMenuDataMediaType::kPlugin;
+  context_menu_params.src_url = subframe_url;
+  context_menu_params.page_url = inner_web_contents->GetLastCommittedURL();
+  TestRenderViewContextMenu menu(inner_web_contents->GetMainFrame(),
+                                 context_menu_params);
+  menu.Init();
+  menu.ExecuteCommand(IDC_CONTENT_CONTEXT_SAVEAVAS, 0);
+
+  request_waiter.Run();
+
+  EXPECT_TRUE(trusted_params.has_value());
+  EXPECT_TRUE(trusted_params->isolation_info.IsEqualForTesting(
+      expected_isolation_info));
+  EXPECT_TRUE(site_for_cookies.IsEquivalent(expected_site_for_cookies));
+
+  download_waiter->WaitForFinished();
+
+  EXPECT_EQ(1u,
+            download_waiter->NumDownloadsSeenInState(DownloadItem::COMPLETE));
+}
+
 class DownloadTestWithHistogramTester : public DownloadTest {
  public:
   void SetUp() override {