Commit 220ecb7e authored by Min Qin's avatar Min Qin Committed by Commit Bot

Fix an issue that opaque origin triggered download is not throttled

If a download is triggered by opaque origin, currently we create an origin
from main WebContents' URL to determine if the download should be blocked.
However, if main WebContents' URL is also an opaque origin, the newly
created origin will be different from the previous origin. And making
the download always allowed.
This CL fixes the issue by using the originating opaque origin instead
if the WebContents' origin is opaque. An alternative solution is to
assign a dedicated opaque origin to the main WebContents.

BUG=1044277

Change-Id: Ia38280f4237ba5cd35c7afcf350734833fb9d002
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2048843
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740375}
parent 9aaf7905
......@@ -605,10 +605,16 @@ void DownloadRequestLimiter::CanDownloadImpl(
// settings first to see if the download needs to be blocked.
GURL initiator = request_initiator ? request_initiator->GetURL()
: originating_contents->GetVisibleURL();
// Use the origin of |originating_contents| as a back up, if it is non-opaque.
url::Origin origin =
request_initiator && !request_initiator->opaque()
? request_initiator.value()
: url::Origin::Create(originating_contents->GetVisibleURL());
url::Origin::Create(originating_contents->GetVisibleURL());
// If |request_initiator| has a non-opaque origin or if the origin from
// |originating_contents| is opaque, use the origin from |request_initiator|
// to make decisions so that it won't impact the download state of
// |originating_contents|.
if (request_initiator && (!request_initiator->opaque() || origin.opaque()))
origin = request_initiator.value();
DownloadStatus status = state->GetDownloadStatus(origin);
bool is_opaque_initiator = request_initiator && request_initiator->opaque();
......
......@@ -999,3 +999,39 @@ TEST_F(DownloadRequestLimiterTest,
EXPECT_EQ(DownloadRequestLimiter::DOWNLOAD_UI_BLOCKED,
download_request_limiter_->GetDownloadUiStatus(web_contents()));
}
// Test that renderer initiated download from opaque origins are correctly
// limited.
TEST_F(DownloadRequestLimiterTest, OpaqueOrigins) {
// about:blank is an opaque origin.
NavigateAndCommit(GURL("about:blank"));
LoadCompleted();
// Create another opaque origin that will trigger all the download.
url::Origin origin;
// The first download should go through.
CanDownloadFor(web_contents(), origin);
ExpectAndResetCounts(1, 0, 0, __LINE__);
EXPECT_EQ(DownloadRequestLimiter::PROMPT_BEFORE_DOWNLOAD,
download_request_limiter_->GetDownloadStatus(web_contents()));
EXPECT_EQ(DownloadRequestLimiter::DOWNLOAD_UI_DEFAULT,
download_request_limiter_->GetDownloadUiStatus(web_contents()));
// The 2nd download will be canceled, there is no prompt since the origin
// is opaque.
CanDownloadFor(web_contents(), origin);
ExpectAndResetCounts(0, 1, 0, __LINE__);
EXPECT_EQ(DownloadRequestLimiter::DOWNLOADS_NOT_ALLOWED,
download_request_limiter_->GetDownloadStatus(web_contents()));
EXPECT_EQ(DownloadRequestLimiter::DOWNLOAD_UI_BLOCKED,
download_request_limiter_->GetDownloadUiStatus(web_contents()));
// Trigger another download from about:blank, that should not be affected
// as it is a special URL.
CanDownloadFor(web_contents());
ExpectAndResetCounts(1, 0, 0, __LINE__);
EXPECT_EQ(DownloadRequestLimiter::PROMPT_BEFORE_DOWNLOAD,
download_request_limiter_->GetDownloadStatus(web_contents()));
EXPECT_EQ(DownloadRequestLimiter::DOWNLOAD_UI_DEFAULT,
download_request_limiter_->GetDownloadUiStatus(web_contents()));
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment