electron · jkleinsc · Aug 9, 2022 · Aug 9, 2022 · Aug 9, 2022 · Aug 9, 2022
@@ -129,4 +129,5 @@ add_maximized_parameter_to_linuxui_getwindowframeprovider.patch
 cherry-pick-94a8bdafc8c6.patch
 fix_mac_build_with_enable_plugins_false.patch
 fix_windows_build_with_enable_plugins_false.patch
+cherry-pick-bd9724c9fe63.patch
 cherry-pick-c643d18a078d.patch
@@ -0,0 +1,203 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Min Qin <qinmin@chromium.org>
+Date: Tue, 28 Jun 2022 16:27:43 +0000
+Subject: passing update_first_party_url_on_redirect=false for fetch
+
+Background fetch doesn't work like regular download as it is not
+considered a top frame navigation. This CL let background fetch to
+pass update_first_party_url_on_redirect=false to DownloadURLParameters,
+and handle it properly w.r.t samesite cookies.
+
+BUG=1268580
+
+(cherry picked from commit bf1e93c6af21dad12088b615feda07a90a85c158)
+
+Change-Id: I3a1cc33be8578d5d8c796dbbb21fa35a47bdda36
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3712307
+Reviewed-by: Rayan Kanso <rayankans@chromium.org>
+Commit-Queue: Min Qin <qinmin@chromium.org>
+Cr-Original-Commit-Position: refs/heads/main@{#1016316}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3727786
+Cr-Commit-Position: refs/branch-heads/5112@{#397}
+Cr-Branched-From: b13d3fe7b3c47a56354ef54b221008afa754412e-refs/heads/main@{#1012729}
+
+diff --git a/components/background_fetch/background_fetch_delegate_base.cc b/components/background_fetch/background_fetch_delegate_base.cc
+index 5f88192eb4e92d4c41c79db188b8b9982f005258..26f7716ae83b70cdcf7cdb23652c2c56d7dc27bf 100644
+--- a/components/background_fetch/background_fetch_delegate_base.cc
++++ b/components/background_fetch/background_fetch_delegate_base.cc
+@@ -101,6 +101,7 @@ void BackgroundFetchDelegateBase::DownloadUrl(
+                           weak_ptr_factory_.GetWeakPtr());
+   params.traffic_annotation =
+       net::MutableNetworkTrafficAnnotationTag(traffic_annotation);
++  params.request_params.update_first_party_url_on_redirect = false;
+
+   JobDetails* job_details = GetJobDetails(job_id);
+   if (job_details->job_state == JobDetails::State::kPendingWillStartPaused ||
+diff --git a/components/download/content/internal/download_driver_impl.cc b/components/download/content/internal/download_driver_impl.cc
+index 2b2da55b06e3b4859f439d5b3bf7013b981b75a0..b207ce5c6bd3c332a34885e1dc58cc11d91fe31c 100644
+--- a/components/download/content/internal/download_driver_impl.cc
++++ b/components/download/content/internal/download_driver_impl.cc
+@@ -229,6 +229,8 @@ void DownloadDriverImpl::Start(
+     download_url_params->set_isolation_info(
+         request_params.isolation_info.value());
+   }
++  download_url_params->set_update_first_party_url_on_redirect(
++      request_params.update_first_party_url_on_redirect);
+
+   download_manager_coordinator_->DownloadUrl(std::move(download_url_params));
+ }
+diff --git a/components/download/internal/common/download_utils.cc b/components/download/internal/common/download_utils.cc
+index 11f524861320eae8ab694022d97a6fe2ac7366ce..454326720b5c88cc61e4527790797fd7f522014d 100644
+--- a/components/download/internal/common/download_utils.cc
++++ b/components/download/internal/common/download_utils.cc
+@@ -386,8 +386,10 @@ std::unique_ptr<network::ResourceRequest> CreateResourceRequest(
+     // cross-site URL has been visited before.
+     url::Origin origin = url::Origin::Create(params->url());
+     request->trusted_params->isolation_info = net::IsolationInfo::Create(
+-        net::IsolationInfo::RequestType::kMainFrame, origin, origin,
+-        net::SiteForCookies::FromOrigin(origin));
++        params->update_first_party_url_on_redirect()
++            ? net::IsolationInfo::RequestType::kMainFrame
++            : net::IsolationInfo::RequestType::kOther,
++        origin, origin, net::SiteForCookies::FromOrigin(origin));
+     request->site_for_cookies = net::SiteForCookies::FromUrl(params->url());
+   }
+
+@@ -395,7 +397,8 @@ std::unique_ptr<network::ResourceRequest> CreateResourceRequest(
+   request->referrer = params->referrer();
+   request->referrer_policy = params->referrer_policy();
+   request->is_outermost_main_frame = true;
+-  request->update_first_party_url_on_redirect = true;
++  request->update_first_party_url_on_redirect =
++      params->update_first_party_url_on_redirect();
+
+   // Downloads should be treated as navigations from Fetch spec perspective.
+   // See also:
+diff --git a/components/download/public/background_service/download_params.h b/components/download/public/background_service/download_params.h
+index 8b911276e757ee889ed05a3fd60cc365ad8f1982..a976d470094462be735e81cf097f32b34c258270 100644
+--- a/components/download/public/background_service/download_params.h
++++ b/components/download/public/background_service/download_params.h
+@@ -126,6 +126,12 @@ struct RequestParams {
+   // be invalidate during download resumption in new browser session. Not
+   // supported on iOS.
+   absl::optional<net::IsolationInfo> isolation_info;
++
++  // First-party URL redirect policy: During server redirects, whether the
++  // first-party URL for cookies will need to be changed. Download is normally
++  // considered a main frame navigation. However, this is not true for
++  // background fetch.
++  bool update_first_party_url_on_redirect = true;
+ };
+
+ // The parameters that describe a download request made to the DownloadService.
+diff --git a/components/download/public/common/download_url_parameters.cc b/components/download/public/common/download_url_parameters.cc
+index 3dec7148d86cd3892670df8b95dcb78d49616e89..25ea6f13e0df9f23364b75c48b870d8ea6a53e92 100644
+--- a/components/download/public/common/download_url_parameters.cc
++++ b/components/download/public/common/download_url_parameters.cc
+@@ -34,7 +34,8 @@ DownloadUrlParameters::DownloadUrlParameters(
+       traffic_annotation_(traffic_annotation),
+       download_source_(DownloadSource::UNKNOWN),
+       require_safety_checks_(true),
+-      has_user_gesture_(false) {}
++      has_user_gesture_(false),
++      update_first_party_url_on_redirect_(true) {}
+
+ DownloadUrlParameters::~DownloadUrlParameters() = default;
+
+diff --git a/components/download/public/common/download_url_parameters.h b/components/download/public/common/download_url_parameters.h
+index ba0a03cb035a79651967ac2882f89f577bd0c012..61eb9af8a00c3e4adf700178994a9e6e864bcf0c 100644
+--- a/components/download/public/common/download_url_parameters.h
++++ b/components/download/public/common/download_url_parameters.h
+@@ -279,6 +279,11 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
+     has_user_gesture_ = has_user_gesture;
+   }
+
++  void set_update_first_party_url_on_redirect(
++      bool update_first_party_url_on_redirect) {
++    update_first_party_url_on_redirect_ = update_first_party_url_on_redirect;
++  }
++
+   OnStartedCallback& callback() { return callback_; }
+   bool content_initiated() const { return content_initiated_; }
+   const std::string& last_modified() const { return last_modified_; }
+@@ -335,6 +340,9 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
+     return isolation_info_;
+   }
+   bool has_user_gesture() const { return has_user_gesture_; }
++  bool update_first_party_url_on_redirect() const {
++    return update_first_party_url_on_redirect_;
++  }
+
+   // STATE CHANGING: All save_info_ sub-objects will be in an indeterminate
+   // state following this call.
+@@ -383,6 +391,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
+   bool require_safety_checks_;
+   absl::optional<net::IsolationInfo> isolation_info_;
+   bool has_user_gesture_;
++  bool update_first_party_url_on_redirect_;
+ };
+
+ }  // namespace download
+diff --git a/content/browser/download/download_browsertest.cc b/content/browser/download/download_browsertest.cc
+index 6dcc861171b7034d7426c5c683cafca689e3d7ea..500521ce2c383d40c13a9359ba4c49d3417670f7 100644
+--- a/content/browser/download/download_browsertest.cc
++++ b/content/browser/download/download_browsertest.cc
+@@ -3763,6 +3763,58 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, UpdateSiteForCookies) {
+                                 site_a.GetURL("a.test", "/")));
+ }
+
++// Tests that if `update_first_party_url_on_redirect` is set to false, download
++// will not behave like a top-level frame navigation and SameSite=Strict cookies
++// will not be set on a redirection.
++IN_PROC_BROWSER_TEST_F(
++    DownloadContentTest,
++    SiteForCookies_DownloadUrl_NotUpdateFirstPartyUrlOnRedirect) {
++  net::EmbeddedTestServer site_a;
++  net::EmbeddedTestServer site_b;
++
++  base::StringPairs cookie_headers;
++  cookie_headers.push_back(std::make_pair(
++      std::string("Set-Cookie"), std::string("A=strict; SameSite=Strict")));
++  cookie_headers.push_back(std::make_pair(std::string("Set-Cookie"),
++                                          std::string("B=lax; SameSite=Lax")));
++
++  // This will request a URL on b.test, which redirects to a url that sets the
++  // cookies on a.test.
++  site_a.RegisterRequestHandler(CreateBasicResponseHandler(
++      "/sets-samesite-cookies", net::HTTP_OK, cookie_headers,
++      "application/octet-stream", "abcd"));
++  ASSERT_TRUE(site_a.Start());
++  site_b.RegisterRequestHandler(
++      CreateRedirectHandler("/redirected-download",
++                            site_a.GetURL("a.test", "/sets-samesite-cookies")));
++  ASSERT_TRUE(site_b.Start());
++
++  // Download the file.
++  SetupEnsureNoPendingDownloads();
++  std::unique_ptr<download::DownloadUrlParameters> download_parameters(
++      DownloadRequestUtils::CreateDownloadForWebContentsMainFrame(
++          shell()->web_contents(),
++          site_b.GetURL("b.test", "/redirected-download"),
++          TRAFFIC_ANNOTATION_FOR_TESTS));
++  download_parameters->set_update_first_party_url_on_redirect(false);
++  std::unique_ptr<DownloadTestObserver> observer(CreateWaiter(shell(), 1));
++  DownloadManagerForShell(shell())->DownloadUrl(std::move(download_parameters));
++  observer->WaitForFinished();
++
++  // Get the important info from other threads and check it.
++  EXPECT_TRUE(EnsureNoPendingDownloads());
++
++  std::vector<download::DownloadItem*> downloads;
++  DownloadManagerForShell(shell())->GetAllDownloads(&downloads);
++  ASSERT_EQ(1u, downloads.size());
++  ASSERT_EQ(download::DownloadItem::COMPLETE, downloads[0]->GetState());
++
++  // Check that the cookies were not set on a.test.
++  EXPECT_EQ("",
++            content::GetCookies(shell()->web_contents()->GetBrowserContext(),
++                                site_a.GetURL("a.test", "/")));
++}
++
+ // Verifies that isolation info set in DownloadUrlParameters can be populated.
+ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
+                        SiteForCookies_DownloadUrl_IsolationInfoPopulated) {