Discharge7443/brave-core
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Fixed download toolbar icon regressions with insecure file" (#36929)

Browse Source 
Reverts #36874

Original PR had one more commit but it's missed.
This commit is contained in:
Simon Hong
2026-06-02 16:53:13 +02:00
committed by GitHub
parent 358a9a1107
commit cc69e24bfc
5 changed files with 13 additions and 186 deletions
Download Patch File Download Diff File Expand all files Collapse all files
browser/ui/views/download/bubble/BUILD.gn
-24
View File
@@ -3,30 +3,6 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at https://mozilla.org/MPL/2.0/.
source_set("browser_tests") {
testonly = true
defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
sources = [ "download_toolbar_ui_controller_browsertest.cc" ]
deps = [
"//base",
"//base/test:test_support",
"//chrome/browser",
"//chrome/browser/download",
"//chrome/browser/ui",
"//chrome/browser/ui/download",
"//chrome/browser/ui/views/download",
"//chrome/test:test_support",
"//components/download/public/common:test_support",
"//components/keyed_service/content",
"//components/vector_icons",
"//content/test:test_support",
"//testing/gmock",
"//testing/gtest",
]
}
source_set("unit_tests") {
testonly = true
browser/ui/views/download/bubble/download_toolbar_ui_controller_browsertest.cc
-147
View File
@@ -1,147 +0,0 @@
/* Copyright (c) 2026 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */
#include "chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.h"
#include "base/callback_list.h"
#include "base/functional/bind.h"
#include "base/memory/raw_ptr.h"
#include "chrome/browser/download/bubble/download_bubble_update_service.h"
#include "chrome/browser/download/bubble/download_bubble_update_service_factory.h"
#include "chrome/browser/download/download_item_model.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/download/download_display.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/toolbar_button_provider.h"
#include "chrome/browser/ui/views/toolbar/toolbar_button.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "components/download/public/common/download_item.h"
#include "components/download/public/common/mock_download_item.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/vector_icons/vector_icons.h"
#include "content/public/browser/browser_context.h"
#include "content/public/test/browser_test.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
using ::testing::NiceMock;
using ::testing::Return;
namespace {
// Minimal fake that lets tests control which download items are visible, so
// HasInsecureDownloads() returns a predictable value without needing real
// network activity.
class FakeDownloadBubbleUpdateService : public DownloadBubbleUpdateService {
public:
explicit FakeDownloadBubbleUpdateService(Profile* profile)
: DownloadBubbleUpdateService(profile) {}
bool IsInitialized() const override { return true; }
bool GetAllModelsToDisplay(
std::vector<DownloadUIModel::DownloadUIModelPtr>& models,
const webapps::AppId* /*web_app_id*/,
bool /*force_backfill*/) override {
models.clear();
for (download::DownloadItem* item : items_) {
models.push_back(DownloadItemModel::Wrap(item));
}
return true;
}
void AddItem(download::DownloadItem* item) { items_.push_back(item); }
private:
std::vector<raw_ptr<download::DownloadItem, VectorExperimental>> items_;
};
std::unique_ptr<KeyedService> BuildFakeService(
content::BrowserContext* context) {
return std::make_unique<FakeDownloadBubbleUpdateService>(
Profile::FromBrowserContext(context));
}
} // namespace
class DownloadToolbarInsecureIconTest : public InProcessBrowserTest {
public:
void SetUpInProcessBrowserTestFixture() override {
InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
// Register the fake service factory before the browser (and its profile)
// is created, so DownloadBubbleUIController picks it up from the start and
// no dangling raw_ptr is left to the replaced service.
subscription_ =
BrowserContextDependencyManager::GetInstance()
->RegisterCreateServicesCallbackForTesting(
base::BindRepeating([](content::BrowserContext* context) {
DownloadBubbleUpdateServiceFactory::GetInstance()
->SetTestingFactory(
context, base::BindRepeating(&BuildFakeService));
}));
}
FakeDownloadBubbleUpdateService* fake_service() {
return static_cast<FakeDownloadBubbleUpdateService*>(
DownloadBubbleUpdateServiceFactory::GetForProfile(
browser()->profile()));
}
ToolbarButton* download_button() {
return BrowserView::GetBrowserViewForBrowser(browser())
->toolbar_button_provider()
->GetDownloadButton();
}
DownloadToolbarUIController* controller() {
return DownloadToolbarUIController::From(browser());
}
private:
base::CallbackListSubscription subscription_;
};
// Verifies that UpdateIcon_BraveImpl sets / clears the toolbar button colour
// override according to the insecure-download status of the current models.
// Covers: no insecure download, WARN status, and BLOCK status.
IN_PROC_BROWSER_TEST_F(DownloadToolbarInsecureIconTest, InsecureDownloadIcon) {
auto* ctrl = controller();
ASSERT_NE(nullptr, ctrl) << "controller should be available";
ctrl->Show();
auto* btn = download_button();
ASSERT_NE(nullptr, btn) << "download button should be present after Show()";
NiceMock<download::MockDownloadItem> item;
// No insecure downloads — colour override must be absent.
ctrl->UpdateDownloadIcon(
{.new_active = DownloadDisplay::IconActive::kActive});
EXPECT_FALSE(btn->HasIconEnabledColorsOverride()) << "state: no items";
EXPECT_TRUE(!btn->HasVectorIcons() ||
&btn->GetVectorIcon() != &vector_icons::kNotSecureWarningIcon)
<< "state: no items; HasVectorIcons=" << btn->HasVectorIcons();
// WARN status: warning colour override and warning vector icon must be set.
ON_CALL(item, GetInsecureDownloadStatus())
.WillByDefault(
Return(download::DownloadItem::InsecureDownloadStatus::WARN));
fake_service()->AddItem(&item);
ctrl->UpdateDownloadIcon(
{.new_state = DownloadDisplay::IconState::kProgress});
EXPECT_TRUE(btn->HasIconEnabledColorsOverride()) << "state: WARN";
EXPECT_EQ(&btn->GetVectorIcon(), &vector_icons::kNotSecureWarningIcon)
<< "state: WARN";
// BLOCK status is treated the same way.
ON_CALL(item, GetInsecureDownloadStatus())
.WillByDefault(
Return(download::DownloadItem::InsecureDownloadStatus::BLOCK));
ctrl->UpdateDownloadIcon(
{.new_state = DownloadDisplay::IconState::kComplete});
EXPECT_TRUE(btn->HasIconEnabledColorsOverride()) << "state: BLOCK";
EXPECT_EQ(&btn->GetVectorIcon(), &vector_icons::kNotSecureWarningIcon)
<< "state: BLOCK";
}
patches/chrome-browser-ui-views-download-bubble-download_toolbar_ui_controller.cc.patch
+7 -7
View File
@@ -1,5 +1,5 @@
diff --git a/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc b/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc
index 5360031b8a2b29a58572f1caa5531f168c710335..717ed2c360e8350ee97722ef6589d6bd115e5a3c 100644
index 5360031b8a2b29a58572f1caa5531f168c710335..5d9e72d1fe4e398435fa46e620dfa25935da340b 100644
--- a/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc
+++ b/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc
@@ -686,6 +686,7 @@ void DownloadToolbarUIController::UpdateIcon() {
@@ -10,11 +10,11 @@ index 5360031b8a2b29a58572f1caa5531f168c710335..717ed2c360e8350ee97722ef6589d6bd
action_item_->SetImage(ui::ImageModel::FromVectorIcon(*new_icon, icon_color));
// Update the toolbar button's tooltip.
@@ -716,6 +717,7 @@ void DownloadToolbarUIController::UpdateIcon() {
GetImageBadge(browser_view_)
->UpdateImage(is_active, progress_download_count, progress_color,
badge_background_color);
@@ -735,6 +736,7 @@ void DownloadToolbarUIController::UpdateIcon() {
return;
}
progress_ring->SetDownloading(progress_info_.progress_percentage);
+ UpdateIcon_BraveImpl(browser_view_, action_item_);
}
// Do not show the progress ring when there is no in progress download.
if (state_ == IconState::kComplete || progress_info_.download_count == 0) {
void DownloadToolbarUIController::OpenPrimaryDialog() {
rewrite/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc.toml
+6 -7
View File
@@ -4,15 +4,14 @@
# You can obtain one at https://mozilla.org/MPL/2.0/.
[[substitution]]
description = '''Override to show insecure icon on insecure download link.
description = '''Override to show insecure icon on insecure download link.'
This replacement adds a `UpdateIcon_BraveImpl` call after the upstream image
update so it runs even when UpdateIcon() returns early afterwards.
This replacement adds a `UpdateIcon_BraveImpl` call at the end of the function.
'''
re_pattern = '(badge_background_color\);)'
replace = '''\1
UpdateIcon_BraveImpl(browser_view_, action_item_);'''
re_flags = ['MULTILINE']
re_pattern = '(DownloadToolbarUIController::UpdateIcon\(.*?\n)^\}'
replace = '''\1 UpdateIcon_BraveImpl(browser_view_, action_item_);
}'''
re_flags = ['MULTILINE', 'DOTALL']
[[substitution]]
description = 'Override for the icon colour.'
test/BUILD.gn
-1
View File
@@ -1398,7 +1398,6 @@ test("brave_browser_tests") {
"//brave/browser/ui/page_info",
"//brave/browser/ui/tabs:tab_strip",
"//brave/browser/ui/toolbar:brave_app_menu_browser_tests",
"//brave/browser/ui/views/download/bubble:browser_tests",
"//brave/browser/ui/webui/brave_new_tab_page_refresh:browser_tests",
"//brave/browser/ui/webui/side_panel/customize_chrome:browser_tests",
"//brave/browser/ui/whats_new:browser_test",