From 313c1ce85165833b914300836bee74222342b4d3 Mon Sep 17 00:00:00 2001
From: Yaw Frempong <yawfrempong@google.com>
Date: Wed, 05 Aug 2026 15:16:57 -0700
Subject: [PATCH] [Downloads] Scope queued download callbacks to requesting origin

DownloadRequestLimiter::TabDownloadState queues pending download
decisions in a single per-tab vector while the multiple-download
permission prompt is showing. Downloads initiated by an origin other
than the one shown in the prompt would join that queue and be released
with allow=true when the user accepted the prompt.

Tag each queued callback with its requesting origin and have
NotifyCallbacks() apply the user's decision only to callbacks that match
the prompt's origin; queued callbacks for any other origin are
cancelled.

Reviewed in https://crrev.com/i/9588737

Bug: 514055890
Change-Id: I858c3cb6fc07faacee96cd4c2cd073a003535f7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8165050
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Yaw Frempong <yawfrempong@google.com>
Cr-Commit-Position: refs/heads/main@{#1674546}
---
diff --git a/chrome/browser/download/download_request_limiter.cc b/chrome/browser/download/download_request_limiter.cc
index eb1ace9..011a277 100644
--- a/chrome/browser/download/download_request_limiter.cc
+++ b/chrome/browser/download/download_request_limiter.cc
@@ -139,7 +139,7 @@
   if (!shouldClearDownloadState(navigation_handle))
     return;
 
-  NotifyCallbacks(false);
+  NotifyCallbacks(origin_, false);
   host_->Remove(this, web_contents());
 }
 
@@ -173,7 +173,7 @@
     // just initially navigated to this page. See http://crbug.com/40299431.
     // However, explicitly leave the limiter in place if the navigation was
     // renderer-initiated and we are in a prompt state.
-    NotifyCallbacks(false);
+    NotifyCallbacks(origin_, false);
     host_->Remove(this, web_contents());
     return;
     // WARNING: We've been deleted.
@@ -198,7 +198,7 @@
   // Tab closed, no need to handle closing the dialog as it's owned by the
   // WebContents.
 
-  NotifyCallbacks(false);
+  NotifyCallbacks(origin_, false);
   host_->Remove(this, web_contents());
   // WARNING: We've been deleted.
 }
@@ -206,7 +206,7 @@
 void DownloadRequestLimiter::TabDownloadState::PromptUserForDownload(
     DownloadRequestLimiter::Callback callback,
     const url::Origin& request_origin) {
-  callbacks_.push_back(std::move(callback));
+  callbacks_.emplace_back(request_origin, std::move(callback));
   DCHECK(web_contents_);
   if (is_showing_prompt())
     return;
@@ -263,14 +263,14 @@
 void DownloadRequestLimiter::TabDownloadState::Cancel(
     const url::Origin& request_origin) {
   SetContentSetting(CONTENT_SETTING_BLOCK, request_origin);
-  bool throttled = NotifyCallbacks(false);
+  bool throttled = NotifyCallbacks(request_origin, false);
   SetDownloadStatusAndNotify(request_origin, throttled ? PROMPT_BEFORE_DOWNLOAD
                                                        : DOWNLOADS_NOT_ALLOWED);
 }
 
 void DownloadRequestLimiter::TabDownloadState::CancelOnce(
     const url::Origin& request_origin) {
-  bool throttled = NotifyCallbacks(false);
+  bool throttled = NotifyCallbacks(request_origin, false);
   SetDownloadStatusAndNotify(request_origin, throttled ? PROMPT_BEFORE_DOWNLOAD
                                                        : DOWNLOADS_NOT_ALLOWED);
 }
@@ -278,7 +278,7 @@
 void DownloadRequestLimiter::TabDownloadState::Accept(
     const url::Origin& request_origin) {
   SetContentSetting(CONTENT_SETTING_ALLOW, request_origin);
-  bool throttled = NotifyCallbacks(true);
+  bool throttled = NotifyCallbacks(request_origin, true);
   SetDownloadStatusAndNotify(
       request_origin, throttled ? PROMPT_BEFORE_DOWNLOAD : ALLOW_ALL_DOWNLOADS);
 }
@@ -379,34 +379,52 @@
                                  setting);
 }
 
-bool DownloadRequestLimiter::TabDownloadState::NotifyCallbacks(bool allow) {
-  std::vector<DownloadRequestLimiter::Callback> callbacks;
+bool DownloadRequestLimiter::TabDownloadState::NotifyCallbacks(
+    const url::Origin& request_origin,
+    bool allow) {
+  std::vector<DownloadRequestLimiter::Callback> requesting_origin_callbacks;
+  std::vector<DownloadRequestLimiter::Callback> other_origin_callbacks;
+  for (auto& [origin, callback] : callbacks_) {
+    if (origin == request_origin) {
+      requesting_origin_callbacks.push_back(std::move(callback));
+    } else {
+      other_origin_callbacks.push_back(std::move(callback));
+    }
+  }
+  callbacks_.clear();
+
   bool throttled = false;
 
   // Selectively send first few notifications only if number of downloads exceed
   // kMaxDownloadsAtOnce. In that case, we also retain the infobar instance and
   // don't close it. If allow is false, we send all the notifications to cancel
   // all remaining downloads and close the infobar.
-  if (!allow || (callbacks_.size() < kMaxDownloadsAtOnce)) {
+  if (!allow || (requesting_origin_callbacks.size() < kMaxDownloadsAtOnce)) {
     // Null the generated weak pointer so we don't get notified again.
     factory_.InvalidateWeakPtrs();
-    callbacks.swap(callbacks_);
   } else {
-    std::vector<DownloadRequestLimiter::Callback>::iterator start, end;
-    start = callbacks_.begin();
-    end = callbacks_.begin() + kMaxDownloadsAtOnce;
-    callbacks.assign(std::make_move_iterator(start),
-                     std::make_move_iterator(end));
-    callbacks_.erase(start, end);
+    auto start = requesting_origin_callbacks.begin() + kMaxDownloadsAtOnce;
+    auto end = requesting_origin_callbacks.end();
+    for (auto it = start; it != end; ++it) {
+      callbacks_.emplace_back(request_origin, std::move(*it));
+    }
+    requesting_origin_callbacks.erase(start, end);
     throttled = true;
   }
 
-  for (auto& callback : callbacks) {
+  for (auto& callback : requesting_origin_callbacks) {
     // When callback runs, it can cause the WebContents to be destroyed.
     content::GetUIThreadTaskRunner({})->PostTask(
         FROM_HERE, base::BindOnce(std::move(callback), allow));
   }
 
+  // Downloads queued for other origins are not covered by the user's decision
+  // for `request_origin`, so cancel them.
+  for (auto& callback : other_origin_callbacks) {
+    content::GetUIThreadTaskRunner({})->PostTask(
+        FROM_HERE, base::BindOnce(std::move(callback), false));
+  }
+
   return throttled;
 }
 
diff --git a/chrome/browser/download/download_request_limiter.h b/chrome/browser/download/download_request_limiter.h
index e5b2770..d9bf211 100644
--- a/chrome/browser/download/download_request_limiter.h
+++ b/chrome/browser/download/download_request_limiter.h
@@ -10,6 +10,7 @@
 #include <map>
 #include <set>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/functional/callback.h"
@@ -176,8 +177,11 @@
                            const url::Origin& request_origin);
 
     // Notifies the callbacks as to whether the download is allowed or not.
-    // Returns false if it didn't notify all callbacks.
-    bool NotifyCallbacks(bool allow);
+    // Only callbacks queued for `request_origin` are eligible to be allowed;
+    // callbacks queued for any other origin are always notified with false.
+    // Returns true if downloads were throttled and remaining callbacks were
+    // kept queued.
+    bool NotifyCallbacks(const url::Origin& request_origin, bool allow);
 
     // Set the download limiter state and notify if it has changed. Callers must
     // guarantee that |status| and |setting| correspond to each other.
@@ -208,10 +212,11 @@
     // True if a download has been seen on the current page load.
     bool download_seen_;
 
-    // Callbacks we need to notify. This is only non-empty if we're showing a
-    // dialog.
+    // Callbacks we need to notify, paired with the origin that initiated each
+    // download. This is only non-empty if we're showing a dialog.
     // See description above CanDownload for details on lifetime of callbacks.
-    std::vector<DownloadRequestLimiter::Callback> callbacks_;
+    std::vector<std::pair<url::Origin, DownloadRequestLimiter::Callback>>
+        callbacks_;
 
     // Origins that have non-default download state.
     using DownloadStatusMap = std::map<url::Origin, DownloadStatus>;
diff --git a/chrome/browser/download/download_request_limiter_unittest.cc b/chrome/browser/download/download_request_limiter_unittest.cc
index d92148d7..259ceddc 100644
--- a/chrome/browser/download/download_request_limiter_unittest.cc
+++ b/chrome/browser/download/download_request_limiter_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/command_line.h"
 #include "base/functional/bind.h"
 #include "base/run_loop.h"
+#include "base/test/run_until.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
@@ -138,6 +139,11 @@
     SetHostContentSetting(web_contents, setting);
   }
 
+  DownloadRequestLimiter::TabDownloadState* GetTabDownloadState(
+      WebContents* web_contents) {
+    return download_request_limiter_->GetDownloadState(web_contents);
+  }
+
  protected:
   const GURL kTestURL = GURL("http://foo.com/bar");
 
@@ -1025,6 +1031,58 @@
             download_request_limiter_->GetDownloadUiStatus(web_contents()));
 }
 
+// Test that accepting the multiple-download prompt only allows the queued
+// downloads that were initiated by the origin the prompt was shown for.
+TEST_F(DownloadRequestLimiterTest, AcceptOnlyAllowsPromptOriginDownloads) {
+  NavigateAndCommit(kTestURL);
+  LoadCompleted();
+
+  url::Origin main_origin = url::Origin::Create(kTestURL);
+  url::Origin other_origin = url::Origin::Create(GURL("http://foobar.com"));
+
+  // First download from the main origin is allowed and moves the tab to
+  // PROMPT_BEFORE_DOWNLOAD.
+  CanDownloadFor(kTestURL, web_contents(), main_origin);
+  ExpectAndResetCounts(1, 0, 0, __LINE__);
+  EXPECT_EQ(DownloadRequestLimiter::PROMPT_BEFORE_DOWNLOAD,
+            download_request_limiter_->GetDownloadStatus(web_contents()));
+
+  // A download from another origin in the same tab triggers the prompt for that
+  // origin. Leave the prompt visible so subsequent downloads queue behind it.
+  UpdateExpectations(WAIT);
+  CanDownloadFor(kTestURL, web_contents(), other_origin);
+  EXPECT_TRUE(mock_permission_prompt_factory_->RequestOriginSeen(
+      other_origin.GetURL()));
+  EXPECT_FALSE(
+      mock_permission_prompt_factory_->RequestOriginSeen(main_origin.GetURL()));
+  ExpectAndResetCounts(0, 0, 1, __LINE__);
+
+  // While the prompt is showing, queue several more downloads from the main
+  // origin. These must not be released by accepting the other origin's prompt.
+  for (int i = 0; i < 5; ++i) {
+    CanDownloadFor(kTestURL, web_contents(), main_origin);
+  }
+  ExpectAndResetCounts(0, 0, 0, __LINE__);
+
+  // Accept the prompt as the permission request would for `other_origin`.
+  DownloadRequestLimiter::TabDownloadState* state =
+      GetTabDownloadState(web_contents());
+  ASSERT_TRUE(state);
+  state->Accept(other_origin);
+  EXPECT_TRUE(base::test::RunUntil([&]() {
+    return state->GetDownloadStatus(other_origin) ==
+           DownloadRequestLimiter::ALLOW_ALL_DOWNLOADS;
+  }));
+
+  // Only the download queued for `other_origin` should proceed; downloads
+  // queued for the main origin should be cancelled.
+  ExpectAndResetCounts(1, 5, 0, __LINE__);
+  EXPECT_EQ(DownloadRequestLimiter::ALLOW_ALL_DOWNLOADS,
+            state->GetDownloadStatus(other_origin));
+  EXPECT_EQ(DownloadRequestLimiter::PROMPT_BEFORE_DOWNLOAD,
+            state->GetDownloadStatus(main_origin));
+}
+
 // Test that user interaction on the current page won't reset download status
 // for another origin.
 TEST_F(DownloadRequestLimiterTest,

diff --git a/chrome/browser/download/download_request_limiter_unittest.cc b/chrome/browser/download/download_request_limiter_unittest.cc
index d92148d7..259ceddc 100644
--- a/chrome/browser/download/download_request_limiter_unittest.cc
+++ b/chrome/browser/download/download_request_limiter_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/command_line.h"
 #include "base/functional/bind.h"
 #include "base/run_loop.h"
+#include "base/test/run_until.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
@@ -138,6 +139,11 @@
     SetHostContentSetting(web_contents, setting);
   }
 
+  DownloadRequestLimiter::TabDownloadState* GetTabDownloadState(
+      WebContents* web_contents) {
+    return download_request_limiter_->GetDownloadState(web_contents);
+  }
+
  protected:
   const GURL kTestURL = GURL("http://foo.com/bar");
 
@@ -1025,6 +1031,58 @@
             download_request_limiter_->GetDownloadUiStatus(web_contents()));
 }
 
+// Test that accepting the multiple-download prompt only allows the queued
+// downloads that were initiated by the origin the prompt was shown for.
+TEST_F(DownloadRequestLimiterTest, AcceptOnlyAllowsPromptOriginDownloads) {
+  NavigateAndCommit(kTestURL);
+  LoadCompleted();
+
+  url::Origin main_origin = url::Origin::Create(kTestURL);
+  url::Origin other_origin = url::Origin::Create(GURL("http://foobar.com"));
+
+  // First download from the main origin is allowed and moves the tab to
+  // PROMPT_BEFORE_DOWNLOAD.
+  CanDownloadFor(kTestURL, web_contents(), main_origin);
+  ExpectAndResetCounts(1, 0, 0, __LINE__);
+  EXPECT_EQ(DownloadRequestLimiter::PROMPT_BEFORE_DOWNLOAD,
+            download_request_limiter_->GetDownloadStatus(web_contents()));
+
+  // A download from another origin in the same tab triggers the prompt for that
+  // origin. Leave the prompt visible so subsequent downloads queue behind it.
+  UpdateExpectations(WAIT);
+  CanDownloadFor(kTestURL, web_contents(), other_origin);
+  EXPECT_TRUE(mock_permission_prompt_factory_->RequestOriginSeen(
+      other_origin.GetURL()));
+  EXPECT_FALSE(
+      mock_permission_prompt_factory_->RequestOriginSeen(main_origin.GetURL()));
+  ExpectAndResetCounts(0, 0, 1, __LINE__);
+
+  // While the prompt is showing, queue several more downloads from the main
+  // origin. These must not be released by accepting the other origin's prompt.
+  for (int i = 0; i < 5; ++i) {
+    CanDownloadFor(kTestURL, web_contents(), main_origin);
+  }
+  ExpectAndResetCounts(0, 0, 0, __LINE__);
+
+  // Accept the prompt as the permission request would for `other_origin`.
+  DownloadRequestLimiter::TabDownloadState* state =
+      GetTabDownloadState(web_contents());
+  ASSERT_TRUE(state);
+  state->Accept(other_origin);
+  EXPECT_TRUE(base::test::RunUntil([&]() {
+    return state->GetDownloadStatus(other_origin) ==
+           DownloadRequestLimiter::ALLOW_ALL_DOWNLOADS;
+  }));
+
+  // Only the download queued for `other_origin` should proceed; downloads
+  // queued for the main origin should be cancelled.
+  ExpectAndResetCounts(1, 5, 0, __LINE__);
+  EXPECT_EQ(DownloadRequestLimiter::ALLOW_ALL_DOWNLOADS,
+            state->GetDownloadStatus(other_origin));
+  EXPECT_EQ(DownloadRequestLimiter::PROMPT_BEFORE_DOWNLOAD,
+            state->GetDownloadStatus(main_origin));
+}
+
 // Test that user interaction on the current page won't reset download status
 // for another origin.
 TEST_F(DownloadRequestLimiterTest,