Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(174)

Issue 2248103002: Reland: Replace CONTROL_BACKGROUND and DETACHED_BOOKMARK_BAR_BACKGROUND by COLOR_NTP_BACKGROUND (Closed)

Created:
4 years, 4 months ago by Julien Isorce Samsung
Modified:
3 years, 2 months ago
CC:
chromium-reviews, tfarina, noyau+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Reland: Replace CONTROL_BACKGROUND and DETACHED_BOOKMARK_BAR_BACKGROUND by COLOR_NTP_BACKGROUND This fixes issues where the ntp background image was interrupted underneath the detached bookmark bar. Also the ntp_background color is now forced to be opaque even if a none opaque alpha value is provided from the theme. This prevent issues with subpixel AA which does not work on none opaque background. (and to avoid falling back to grayscale AA) Also see crbug.com/618278 BUG=618335 R=estade@chromium.org, pkasting@chromium.org TEST=./out/Release/unit_tests --gtest_filter=*BrowserThemePack*

Patch Set 1 #

Patch Set 2 : Properly handle incognito browsing #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -51 lines) Patch
M chrome/browser/themes/browser_theme_pack.cc View 2 chunks +6 lines, -3 lines 0 comments Download
M chrome/browser/themes/browser_theme_pack_unittest.cc View 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/themes/theme_properties.h View 2 chunks +1 line, -4 lines 0 comments Download
M chrome/browser/themes/theme_properties.cc View 1 6 chunks +7 lines, -13 lines 3 comments Download
M chrome/browser/themes/theme_service.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/chrome_style.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_bar_toolbar_view.mm View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/ui/libgtk2ui/gtk2_ui.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc View 1 2 chunks +6 lines, -4 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view.cc View 1 2 chunks +6 lines, -9 lines 2 comments Download
M chrome/browser/ui/webui/ntp/ntp_resource_cache.cc View 1 1 chunk +3 lines, -4 lines 1 comment Download

Messages

Total messages: 17 (4 generated)
Julien Isorce Samsung
This is a reland of https://codereview.chromium.org/2062353002 , please have a look, thx.
4 years, 4 months ago (2016-08-16 15:59:04 UTC) #3
Julien Isorce Samsung
https://codereview.chromium.org/2248103002/diff/20001/chrome/browser/ui/views/frame/browser_view.cc File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/2248103002/diff/20001/chrome/browser/ui/views/frame/browser_view.cc#newcode230 chrome/browser/ui/views/frame/browser_view.cc:230: : ThemeProperties::COLOR_NTP_BACKGROUND)); It relies on HasCustomImage to avoid regression ...
4 years, 4 months ago (2016-08-16 16:01:03 UTC) #4
Peter Kasting
Patch set 1 is the rebased version of what landed before, right? Can you briefly ...
4 years, 4 months ago (2016-08-16 20:14:33 UTC) #7
Julien Isorce Samsung
On 2016/08/16 20:14:33, Peter Kasting wrote: > Patch set 1 is the rebased version of ...
4 years, 4 months ago (2016-08-16 20:51:59 UTC) #8
Julien Isorce Samsung
Since I am at it here are the history of this CL: 2 months ago ...
4 years, 4 months ago (2016-08-16 21:13:57 UTC) #9
Peter Kasting
https://codereview.chromium.org/2248103002/diff/20001/chrome/browser/themes/theme_properties.cc File chrome/browser/themes/theme_properties.cc (right): https://codereview.chromium.org/2248103002/diff/20001/chrome/browser/themes/theme_properties.cc#newcode108 chrome/browser/themes/theme_properties.cc:108: kDefaultColorNTPText, SkColorSetRGB(0x32, 0x32, 0x32)}; I think you meant kDefaultColorNTPBackground ...
4 years, 4 months ago (2016-08-18 09:12:36 UTC) #10
Julien Isorce Samsung
Thx for the comments I will have a look this week.
4 years, 4 months ago (2016-08-22 10:46:29 UTC) #11
Peter Kasting
On 2016/08/22 10:46:29, Julien Isorce Samsung wrote: > Thx for the comments I will have ...
3 years, 10 months ago (2017-02-11 02:01:01 UTC) #12
Julien Isorce
On 2017/02/11 02:01:01, Peter Kasting wrote: > On 2016/08/22 10:46:29, Julien Isorce Samsung wrote: > ...
3 years, 10 months ago (2017-02-17 10:22:51 UTC) #13
Peter Kasting
On 2017/02/17 10:22:51, Julien Isorce wrote: > On 2017/02/11 02:01:01, Peter Kasting wrote: > > ...
3 years, 10 months ago (2017-02-18 00:13:22 UTC) #14
Peter Kasting
Rietveld has been deprecated, so if you want to do further work on this, you'll ...
3 years, 3 months ago (2017-08-29 04:13:36 UTC) #15
Peter Kasting
FYI, Rietveld becomes fully read-only in one week.
3 years, 3 months ago (2017-09-22 06:36:04 UTC) #16
Peter Kasting
3 years, 2 months ago (2017-09-29 00:02:24 UTC) #17
Rietveld goes fully read-only tomorrow.  Closing this.  If it's still valuable,
please reopen in Gerrit.

Powered by Google App Engine
This is Rietveld 408576698