Skip to content

Commit

Permalink
Use favicon for the Terminal app home tab.
Browse files Browse the repository at this point in the history
crrev.com/c/4505893 added logic to use the app monochrome icon or
colored icon on the home tab instead of the favicon, but Terminal is
dynamically setting the color of their favicon.

(cherry picked from commit 6099084)

Bug: 1381377
Change-Id: I382e36d31945d8f5edc758ec9e83c805c82b4b32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4559845
Commit-Queue: Louise Brett <loubrett@google.com>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1149399}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4569972
Commit-Queue: Scott Violet <sky@chromium.org>
Auto-Submit: Louise Brett <loubrett@google.com>
Cr-Commit-Position: refs/branch-heads/5790@{#134}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
loubrett authored and Chromium LUCI CQ committed May 30, 2023
1 parent b8bf3ae commit ead2543
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 2 deletions.
6 changes: 4 additions & 2 deletions chrome/browser/ui/tabs/tab_renderer_data.cc
Expand Up @@ -49,10 +49,12 @@ TabRendererData TabRendererData::FromTabInModel(TabStripModel* model,

// Tabbed web apps should use the app icon on the home tab.
Browser* app_browser = chrome::FindBrowserWithWebContents(contents);
if (app_browser && app_browser->app_controller()) {

if (app_browser && app_browser->app_controller() &&
app_browser->app_controller()->ShouldShowAppIconOnTab(index)) {
web_app::WebAppBrowserController* app_controller =
app_browser->app_controller()->AsWebAppBrowserController();
if (app_controller && web_app::IsPinnedHomeTab(model, index)) {
if (app_controller) {
gfx::ImageSkia home_tab_icon = app_controller->GetHomeTabIcon();
if (!home_tab_icon.isNull()) {
data.is_monochrome_favicon = true;
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ui/web_applications/app_browser_controller.cc
Expand Up @@ -445,6 +445,10 @@ bool AppBrowserController::IsUrlInHomeTabScope(const GURL& url) const {
return false;
}

bool AppBrowserController::ShouldShowAppIconOnTab(int index) const {
return false;
}

#if BUILDFLAG(IS_MAC)
bool AppBrowserController::AlwaysShowToolbarInFullscreen() const {
return true;
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ui/web_applications/app_browser_controller.h
Expand Up @@ -152,6 +152,10 @@ class AppBrowserController
// Returns whether the url is within the scope of the tab strip home tab.
virtual bool IsUrlInHomeTabScope(const GURL& url) const;

// Returns whether the app icon should be displayed on the tab instead of the
// favicon.
virtual bool ShouldShowAppIconOnTab(int index) const;

// Determines whether the specified url is 'inside' the app |this| controls.
virtual bool IsUrlInAppScope(const GURL& url) const = 0;

Expand Down
Expand Up @@ -489,6 +489,15 @@ bool WebAppBrowserController::IsUrlInHomeTabScope(const GURL& url) const {
return false;
}

bool WebAppBrowserController::ShouldShowAppIconOnTab(int index) const {
#if BUILDFLAG(IS_CHROMEOS_ASH)
return !system_app() &&
web_app::IsPinnedHomeTab(browser()->tab_strip_model(), index);
#else
return web_app::IsPinnedHomeTab(browser()->tab_strip_model(), index);
#endif
}

bool WebAppBrowserController::IsUrlInAppScope(const GURL& url) const {
#if BUILDFLAG(IS_CHROMEOS_ASH)
if (system_app() && system_app()->IsUrlInSystemAppScope(url))
Expand Down
Expand Up @@ -91,6 +91,7 @@ class WebAppBrowserController : public AppBrowserController,
GURL GetAppStartUrl() const override;
GURL GetAppNewTabUrl() const override;
bool IsUrlInHomeTabScope(const GURL& url) const override;
bool ShouldShowAppIconOnTab(int index) const override;
bool IsUrlInAppScope(const GURL& url) const override;
WebAppBrowserController* AsWebAppBrowserController() override;
bool CanUserUninstall() const override;
Expand Down

0 comments on commit ead2543

Please sign in to comment.