Commit 3b8affa4 authored by Alan Cutter's avatar Alan Cutter Committed by Commit Bot

Reland "Fix GTK+ themed web app windows using incorrect window title color"

This is a reland of 61579f7a
The original CL was optimisic that all linux bots used the same
system theme colors.
This is not the case for Linux MSan Tests:
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8877850460224985872/+/steps/browser_tests/0/logs/Deterministic_failure:_WebAppOpaqueBrowserFrameViewTest.SystemThemeColor__status_FAILURE_/0
This revised CL loosens the expectations to allow for any light title
bar color.

Original change's description:
> Fix GTK+ themed web app windows using incorrect window title color
>
> This CL fixes a bug where web app windows on Linux with GTK+ themes set
> would use system colors for the title bar but web app colors for the text.
> This sometimes resulted in unreadable contrast levels.
>
> Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=450694&signed_aid=ojOLuOeFlvGsonFz7D8lkA==&inline=1
> After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=450693&signed_aid=CN1FqbV6S5JWEJfOW5L4oQ==&inline=1
>
> Bug: 1087693
> Change-Id: Ifc408c4c1fb1c6cae4014f806248a05f5e2f617f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2239769
> Commit-Queue: Alan Cutter <alancutter@chromium.org>
> Reviewed-by: Bret Sepulveda <bsep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#776994}

Bug: 1087693
Change-Id: I3f924f2976114f8bfe58e8a230a036eed9b0df0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2239912Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#777271}
parent 03d6b561
......@@ -202,7 +202,7 @@ bool BrowserFrame::GetAccelerator(int command_id,
const ui::ThemeProvider* BrowserFrame::GetThemeProvider() const {
Browser* browser = browser_view_->browser();
if (browser->app_controller())
if (browser->app_controller() && !IsUsingGtkTheme(browser->profile()))
return browser->app_controller()->GetThemeProvider();
return &ThemeService::GetThemeProviderForProfile(browser->profile());
}
......
......@@ -17,6 +17,7 @@
#include "chrome/common/web_application_info.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "content/public/test/browser_test.h"
#include "ui/gfx/color_utils.h"
#include "ui/views/test/test_views.h"
// Tests web-app windows that use the OpaqueBrowserFrameView implementation
......@@ -104,13 +105,35 @@ IN_PROC_BROWSER_TEST_F(WebAppOpaqueBrowserFrameViewTest, NoThemeColor) {
}
#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
// The app theme color should be ignored in system theme mode.
IN_PROC_BROWSER_TEST_F(WebAppOpaqueBrowserFrameViewTest, SystemThemeColor) {
SetThemeMode(ThemeMode::kSystem);
// The color here should be ignored in system mode.
ASSERT_TRUE(InstallAndLaunchWebApp(SK_ColorRED));
// Read unthemed native frame color.
SkColor native_frame_color =
BrowserView::GetBrowserViewForBrowser(browser())
->frame()
->GetFrameView()
->GetFrameColor(BrowserFrameActiveState::kActive);
SkColor expected_caption_color =
color_utils::GetColorWithMaxContrast(native_frame_color);
// Install web app with theme color contrasting against native frame color.
SkColor theme_color =
color_utils::GetColorWithMaxContrast(native_frame_color);
EXPECT_NE(color_utils::IsDark(theme_color),
color_utils::IsDark(native_frame_color));
ASSERT_TRUE(InstallAndLaunchWebApp(theme_color));
// App theme color should be ignored in favor of native system theme.
EXPECT_EQ(opaque_browser_frame_view_->GetFrameColor(
BrowserFrameActiveState::kActive),
native_frame_color);
EXPECT_EQ(opaque_browser_frame_view_->GetCaptionColor(
BrowserFrameActiveState::kActive),
expected_caption_color);
EXPECT_EQ(web_app_frame_toolbar_->active_color_for_testing(),
gfx::kGoogleGrey900);
expected_caption_color);
}
#endif // defined(OS_LINUX) && !defined(OS_CHROMEOS)
......
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