From eb5ca4eaa0f8f836b49b2f49f85e2d83245217f5 Mon Sep 17 00:00:00 2001 From: cdesouza-chromium Date: Fri, 24 Apr 2026 01:24:13 +0100 Subject: [PATCH] [plaster] `DownloadToolbarUIController` migration (#35750) This class has a few customisations that were awkward with macro replacements and therefore the derived function. With plaster though, we can just add a `ChromiumImpl` variant of a function, and then only replace the declartion in the translation unit, which makes a lot of the complexity present go away. This change drops the derived class for `DownloadToolbarUIController`, and just adds a call to our custom function at the end of the relevant function updating the icon appearance. This particular class was running into problems in M149 (https://crrev.com/c/7766617). Resolves https://github.com/brave/brave-browser/issues/54814 --- .../bubble/download_toolbar_ui_controller.cc | 93 +++++++++---------- .../bubble/download_toolbar_ui_controller.h | 40 -------- ...le-download_toolbar_ui_controller.cc.patch | 20 ++++ .../download_toolbar_ui_controller.cc.toml | 22 +++++ 4 files changed, 87 insertions(+), 88 deletions(-) delete mode 100644 chromium_src/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.h create mode 100644 patches/chrome-browser-ui-views-download-bubble-download_toolbar_ui_controller.cc.patch create mode 100644 rewrite/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc.toml diff --git a/chromium_src/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc b/chromium_src/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc index ed958412717..a08f782ff0f 100644 --- a/chromium_src/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc +++ b/chromium_src/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc @@ -7,15 +7,13 @@ #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_ui_model.h" #include "chrome/browser/ui/color/chrome_color_id.h" +#include "chrome/browser/ui/views/frame/browser_view.h" #include "components/vector_icons/vector_icons.h" -#include "ui/base/models/image_model.h" namespace { -SkColor GetIconColor(SkColor chromium_color, - DownloadDisplay::IconState state, +SkColor GetIconColor(DownloadDisplay::IconState state, DownloadDisplay::IconActive active, const ui::ColorProvider* color_provider) { // Apply active color only when download is completed and user doesn't @@ -29,51 +27,9 @@ SkColor GetIconColor(SkColor chromium_color, return color_provider->GetColor(kColorDownloadToolbarButtonInactive); } -} // namespace - -#define DownloadToolbarUIController DownloadToolbarUIController_ChromiumImpl - -// Update upstream color with our own. Pass COLOR in so that icon_color variable -// in the original file isn't flagged as unused by the compiler. -#define FromVectorIcon(ICON, COLOR) \ - FromVectorIcon(ICON, GetIconColor(COLOR, state_, active_, \ - browser_view_->GetColorProvider())) - -#include -#undef FromVectorIcon -#undef DownloadToolbarUIController - -void DownloadToolbarUIController::UpdateIcon() { - DownloadToolbarUIController_ChromiumImpl::UpdateIcon(); - - if (!action_item_.get()) { - return; - } - - auto* button = GetDownloadsButton(browser_view_); - if (!button) { - return; - } - - // Use an exclamation point icon while there's an insecure download in the - // download models. - if (HasInsecureDownloads()) { - auto icon_color = browser_view_->GetColorProvider()->GetColor( - ui::kColorAlertMediumSeverityIcon); - button->SetIconEnabledColorsOverride(icon_color); - button->SetVectorIcon(vector_icons::kNotSecureWarningIcon); - const gfx::VectorIcon* new_icon = &vector_icons::kNotSecureWarningIcon; - const int icon_size = action_item_->GetImage().Size().height(); - action_item_->SetImage( - ui::ImageModel::FromVectorIcon(*new_icon, icon_color, icon_size)); - } else { - button->SetIconEnabledColorsOverride(std::nullopt); - } -} - -bool DownloadToolbarUIController::HasInsecureDownloads() { +bool HasInsecureDownloads(BrowserView* browser_view) { auto* update_service = DownloadBubbleUpdateServiceFactory::GetForProfile( - browser_view_->GetProfile()); + browser_view->GetProfile()); if (!update_service || !update_service->IsInitialized()) { return false; } @@ -89,3 +45,44 @@ bool DownloadToolbarUIController::HasInsecureDownloads() { download::DownloadItem::InsecureDownloadStatus::WARN); }); } + +// Forward-declaring this customisation point as it depends on +// `GetDownloadsButton`, which is declared in the unamed namespace of the +// shadowed source. +void UpdateIcon_BraveImpl(BrowserView* browser_view, + actions::ActionItem* action_item); + +} // namespace + +#include + +namespace { + +void UpdateIcon_BraveImpl(BrowserView* browser_view, + actions::ActionItem* action_item) { + if (!action_item) { + return; + } + + auto* button = GetDownloadsButton(browser_view); + if (!button) { + return; + } + + // Use an exclamation point icon while there's an insecure download in the + // download models. + if (HasInsecureDownloads(browser_view)) { + auto icon_color = browser_view->GetColorProvider()->GetColor( + ui::kColorAlertMediumSeverityIcon); + button->SetIconEnabledColorsOverride(icon_color); + button->SetVectorIcon(vector_icons::kNotSecureWarningIcon); + const gfx::VectorIcon* new_icon = &vector_icons::kNotSecureWarningIcon; + const int icon_size = action_item->GetImage().Size().height(); + action_item->SetImage( + ui::ImageModel::FromVectorIcon(*new_icon, icon_color, icon_size)); + } else { + button->SetIconEnabledColorsOverride(std::nullopt); + } +} + +} // namespace diff --git a/chromium_src/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.h b/chromium_src/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.h deleted file mode 100644 index 7c541cb9d43..00000000000 --- a/chromium_src/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.h +++ /dev/null @@ -1,40 +0,0 @@ -/* Copyright (c) 2025 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/. */ - -#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_DOWNLOAD_BUBBLE_DOWNLOAD_TOOLBAR_UI_CONTROLLER_H_ -#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_DOWNLOAD_BUBBLE_DOWNLOAD_TOOLBAR_UI_CONTROLLER_H_ - -class DownloadToolbarUIController; -using DownloadToolbarUIController_BraveImpl = DownloadToolbarUIController; - -#define DownloadToolbarUIController DownloadToolbarUIController_ChromiumImpl - -#define CreateBubbleDialogDelegate \ - Unused(); \ - friend DownloadToolbarUIController_BraveImpl; \ - void CreateBubbleDialogDelegate - -// Override to show warning icon in the toolbar when there's an insecure -// download in progress. -#define UpdateIcon virtual UpdateIcon - -#include // IWYU pragma: export -#undef UpdateIcon -#undef DownloadToolbarUIController -#undef CreateBubbleDialogDelegate - -class DownloadToolbarUIController - : public DownloadToolbarUIController_ChromiumImpl { - public: - using DownloadToolbarUIController_ChromiumImpl:: - DownloadToolbarUIController_ChromiumImpl; - - void UpdateIcon() override; - - private: - bool HasInsecureDownloads(); -}; - -#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_DOWNLOAD_BUBBLE_DOWNLOAD_TOOLBAR_UI_CONTROLLER_H_ diff --git a/patches/chrome-browser-ui-views-download-bubble-download_toolbar_ui_controller.cc.patch b/patches/chrome-browser-ui-views-download-bubble-download_toolbar_ui_controller.cc.patch new file mode 100644 index 00000000000..a7d17b995ed --- /dev/null +++ b/patches/chrome-browser-ui-views-download-bubble-download_toolbar_ui_controller.cc.patch @@ -0,0 +1,20 @@ +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 c67a71bad966bebb540b74a031f6310999dfc529..3cb0ac987e2bae17edfce9970661add88cf8355a 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 +@@ -674,6 +674,7 @@ void DownloadToolbarUIController::UpdateIcon() { + } + action_item_->SetProperty(kActionItemUnderlineIndicatorKey, is_icon_active); + ++ icon_color = GetIconColor(state_, active_, browser_view_->GetColorProvider()); + action_item_->SetImage(ui::ImageModel::FromVectorIcon(*new_icon, icon_color)); + + // Update the toolbar button's tooltip. +@@ -723,6 +724,7 @@ void DownloadToolbarUIController::UpdateIcon() { + return; + } + progress_ring->SetDownloading(progress_info_.progress_percentage); ++ UpdateIcon_BraveImpl(browser_view_, action_item_); + } + + void DownloadToolbarUIController::OpenPrimaryDialog() { diff --git a/rewrite/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc.toml b/rewrite/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc.toml new file mode 100644 index 00000000000..fcbe1bd0066 --- /dev/null +++ b/rewrite/chrome/browser/ui/views/download/bubble/download_toolbar_ui_controller.cc.toml @@ -0,0 +1,22 @@ +# Copyright (c) 2025 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/. + +[[substitution]] +description = '''Override to show insecure icon on insecure download link.' + +This replacement adds a `UpdateIcon_BraveImpl` call at the end of the function. +''' +re_pattern = '(DownloadToolbarUIController::UpdateIcon\(.*?\n)^\}' +replace = '''\1 UpdateIcon_BraveImpl(browser_view_, action_item_); +}''' +re_flags = ['MULTILINE', 'DOTALL'] +count = 1 + +[[substitution]] +description = 'Override for the icon colour.' +re_pattern = '^(\s*)action_item_->SetImage' +replace = '\1icon_color = GetIconColor(state_, active_, browser_view_->GetColorProvider());\1action_item_->SetImage' +re_flags = ['MULTILINE'] +count = 1