Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2891983003:
Fix white flash when opening incognito NTP (Closed)
|
Created:
3 years, 7 months ago by Marc Treib Modified:
3 years, 7 months ago CC:
chromium-reviews, dbeam+watch-ntp_chromium.org, arv+watch_chromium.org, pedrosimonetti+watch_chromium.org, msramek Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix white flash when opening incognito NTP
This specifies a different default background color when in incognito
mode in ThemeProvider::GetDefaultColor, matching the MD incognito NTP.
The previous (non-MD) incognito NTP uses an ever-so-slightly different
shade of gray, so that still has a barely-noticeable flash.
I've checked that this doesn't affect the regular or the guest mode
NTP, and that the incognito one works both with and without a theme.
BUG=21798
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2891983003
Cr-Commit-Position: refs/heads/master@{#473224}
Committed: https://chromium.googlesource.com/chromium/src/+/6886191d1df3fc561df9010fc6c850d282a461de
Patch Set 1 #Patch Set 2 : fix themed incognito #Patch Set 3 : . #
Total comments: 2
Messages
Total messages: 21 (15 generated)
Description was changed from ========== Fix white flash when opening incognito NTP This specifies a different default background color when in incognito mode in ThemeProvider::GetDefaultColor, matching the MD incognito NTP. The previous (non-MD) incognito NTP uses an ever-so-slightly different shade of gray, so that still has a barely-noticeable flash. BUG=21798 ========== to ========== Fix white flash when opening incognito NTP This specifies a different default background color when in incognito mode in ThemeProvider::GetDefaultColor, matching the MD incognito NTP. The previous (non-MD) incognito NTP uses an ever-so-slightly different shade of gray, so that still has a barely-noticeable flash. BUG=21798 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Fix white flash when opening incognito NTP This specifies a different default background color when in incognito mode in ThemeProvider::GetDefaultColor, matching the MD incognito NTP. The previous (non-MD) incognito NTP uses an ever-so-slightly different shade of gray, so that still has a barely-noticeable flash. BUG=21798 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Fix white flash when opening incognito NTP This specifies a different default background color when in incognito mode in ThemeProvider::GetDefaultColor, matching the MD incognito NTP. The previous (non-MD) incognito NTP uses an ever-so-slightly different shade of gray, so that still has a barely-noticeable flash. I've checked that this doesn't affect the regular or the guest mode NTP, and that the incognito one works both with and without a theme. BUG=21798 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by treib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by treib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
treib@chromium.org changed reviewers: + estade@chromium.org
estade: Please review! msramek: FYI, touching incognito NTP I'll be out for two weeks now. If this looks good to you, feel free to hit "Commit" so it'll still go into M60. Otherwise, also feel free to take it over and land yourself if you feel like it :) Thanks!
lgtm https://codereview.chromium.org/2891983003/diff/40001/chrome/browser/themes/t... File chrome/browser/themes/theme_properties.cc (right): https://codereview.chromium.org/2891983003/diff/40001/chrome/browser/themes/t... chrome/browser/themes/theme_properties.cc:70: const SkColor kDefaultColorNTPBackgroundOtr = SkColorSetRGB(0x30, 0x30, 0x30); nit: the term used for the colors in this file is incognito rather than otr. I think whatever references to otr do exist are supposed to be changed.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
msramek@chromium.org changed reviewers: + msramek@chromium.org
Non-owner LGTM, appreciate the fix :)
Thanks! https://codereview.chromium.org/2891983003/diff/40001/chrome/browser/themes/t... File chrome/browser/themes/theme_properties.cc (right): https://codereview.chromium.org/2891983003/diff/40001/chrome/browser/themes/t... chrome/browser/themes/theme_properties.cc:70: const SkColor kDefaultColorNTPBackgroundOtr = SkColorSetRGB(0x30, 0x30, 0x30); On 2017/05/19 15:34:54, Evan Stade wrote: > nit: the term used for the colors in this file is incognito rather than otr. I > think whatever references to otr do exist are supposed to be changed. I was being consistent with the "bool otr" below ;) Since I'm OOO now and can't change it right now, I'll land this as-is to get the fix into M60. I solemnly promise to send a followup to fix the naming once I'm back in ~two weeks. :)
The CQ bit was checked by treib@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495214030494390,
"parent_rev": "7bd80624d68ec2ff9cca61c0bdf0754c6830f6ef", "commit_rev":
"6886191d1df3fc561df9010fc6c850d282a461de"}
Message was sent while issue was closed.
Description was changed from ========== Fix white flash when opening incognito NTP This specifies a different default background color when in incognito mode in ThemeProvider::GetDefaultColor, matching the MD incognito NTP. The previous (non-MD) incognito NTP uses an ever-so-slightly different shade of gray, so that still has a barely-noticeable flash. I've checked that this doesn't affect the regular or the guest mode NTP, and that the incognito one works both with and without a theme. BUG=21798 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Fix white flash when opening incognito NTP This specifies a different default background color when in incognito mode in ThemeProvider::GetDefaultColor, matching the MD incognito NTP. The previous (non-MD) incognito NTP uses an ever-so-slightly different shade of gray, so that still has a barely-noticeable flash. I've checked that this doesn't affect the regular or the guest mode NTP, and that the incognito one works both with and without a theme. BUG=21798 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2891983003 Cr-Commit-Position: refs/heads/master@{#473224} Committed: https://chromium.googlesource.com/chromium/src/+/6886191d1df3fc561df9010fc6c8... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/6886191d1df3fc561df9010fc6c8... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
