New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 866671 link

Starred by 5 users
Status: Fixed
Owner:
pkasting@chromium.org
Closed: Aug 6
Cc:
█ abdulsyed@chromium.org
pkasting@chromium.org
est...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Theme background images misaligned

Project Member Reported by pkasting@chromium.org, Jul 23 
Various themes have vertical alignment problems of the background tabs against the frame.

Fixing this is in a couple pieces:
(1) Eliminate conditional positioning of the imagery, just so the code is more sane.
(2) Adjust the positioning to actually match refresh layout.
(3) If possible, for backwards compat, paint the frame as if it always has 16 DIP above the tabs, no matter how much space it actually has.

Comment 1 by pkasting@chromium.org, Jul 23

Components: UI>Browser>Themes
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 25 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5

commit 51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5
Author: Peter Kasting <pkasting@chromium.org>
Date: Wed Jul 25 02:56:04 2018

Partly correct vertical positioning of tab images against frame.

This computes the background tab image Y offset based on the refresh height of
the frame top, and simplifies the code by doing this unconditionally instead of
only when there's a tab background image.

This will not fix cases where a theme uses a custom toolbar image based on
assuming that there are 16 DIP above the top of tabs.  Added a TODO about that.

Bug:  866671 
Change-Id: I9c9b5dacaba31f825d8392f5538ebd4cc8140367
Reviewed-on: https://chromium-review.googlesource.com/1147618
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577780}
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/frame/browser_frame.cc
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/frame/browser_frame.h
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/frame/browser_view_layout.cc
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/frame/browser_view_layout_delegate.h
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/frame/browser_view_unittest.cc
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/tabs/new_tab_button.cc
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/tabs/new_tab_button.h
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5/chrome/browser/ui/views/tabs/tab_strip.h

Comment 3 by pkasting@chromium.org, Jul 30

Labels: Merge-Request-69

Comment 4 by pkasting@chromium.org, Jul 30 

I'm going to merge in advance of getting approval since I want these changes in to make today's cutoff and this blocks about 9 subsequent patches.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 30

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1

commit 5a9b5570c79825696dbd6e4db347fcec1c3ef4d1
Author: Peter Kasting <pkasting@chromium.org>
Date: Mon Jul 30 20:35:35 2018

Partly correct vertical positioning of tab images against frame.

This computes the background tab image Y offset based on the refresh height of
the frame top, and simplifies the code by doing this unconditionally instead of
only when there's a tab background image.

This will not fix cases where a theme uses a custom toolbar image based on
assuming that there are 16 DIP above the top of tabs.  Added a TODO about that.

Bug:  866671 
Change-Id: I9c9b5dacaba31f825d8392f5538ebd4cc8140367
Reviewed-on: https://chromium-review.googlesource.com/1147618
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577780}(cherry picked from commit 51ca9ce3bc2eaa002e44f5feb65da2ff7501dbe5)
Reviewed-on: https://chromium-review.googlesource.com/1155491
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#237}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/frame/browser_frame.cc
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/frame/browser_frame.h
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/frame/browser_view_layout.cc
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/frame/browser_view_layout_delegate.h
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/frame/browser_view_unittest.cc
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/tabs/new_tab_button.cc
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/tabs/new_tab_button.h
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/5a9b5570c79825696dbd6e4db347fcec1c3ef4d1/chrome/browser/ui/views/tabs/tab_strip.h

Comment 6 by gov...@chromium.org, Jul 30 

Sure, I'm ok with M69 merge at #5. Thank you.

Comment 7 by pkasting@chromium.org, Jul 30

Labels: -Merge-Request-69
K, still ongoing work here to implement comment 0 item (3).

Comment 8 by robliao@chromium.org, Jul 31

Labels: Group-Themes

Comment 9 by pkasting@chromium.org, Aug 1

Labels: -merge-merged-3497
Summary: Theme background images misaligned (was: Tab background images misaligned against frame)
Gonna repurpose this to cover a few other types of misalignment too:

* Opaque frame frame background image doesn't end at the bottom of the tabstrip due to TABSTRIP_TOOLBAR_OVERLAP
* Opaque frame toolbar background is not aligned with tabs
* Turns out the frame image needs to mirror-tile too
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 1 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21a88094c09dec71b8236f83b7a28c4b13ff8220

commit 21a88094c09dec71b8236f83b7a28c4b13ff8220
Author: Peter Kasting <pkasting@chromium.org>
Date: Wed Aug 01 23:04:23 2018

Account for TABSTRIP_TOOLBAR_OVERLAP in opaque frame top height.

This seems to affect only the painting of the frame background and Linux caption
buttons.  In testing the first and from code inspection of the second, both seem
to want "the actual height above the toolbar", which doesn't include this
overlap.

This eliminates an ugly 1 DIP misalignment of the theme background image bottom
with the toolbar top in some themes.

Bug:  866671 
Change-Id: I55d1aea4c3af656ae427373dff9bacc76bb932ab
Reviewed-on: https://chromium-review.googlesource.com/1157875
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579980}
[modify] https://crrev.com/21a88094c09dec71b8236f83b7a28c4b13ff8220/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 1 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a71aae9b6772d0900454b30f0dc09a579ae70a8

commit 3a71aae9b6772d0900454b30f0dc09a579ae70a8
Author: Peter Kasting <pkasting@chromium.org>
Date: Wed Aug 01 23:06:10 2018

Correctly align the toolbar background image with the tabstrip.

Bug:  866671 
Change-Id: Ibee4b8efc16008d4d181b3008860858c9d75efa5
Reviewed-on: https://chromium-review.googlesource.com/1157872
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579983}
[modify] https://crrev.com/3a71aae9b6772d0900454b30f0dc09a579ae70a8/chrome/browser/ui/views/toolbar/toolbar_view.cc
[modify] https://crrev.com/3a71aae9b6772d0900454b30f0dc09a579ae70a8/chrome/browser/ui/views/toolbar/toolbar_view.h
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 2 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4052d411adc06d21e60b8920edd486573bb347fe

commit 4052d411adc06d21e60b8920edd486573bb347fe
Author: Peter Kasting <pkasting@chromium.org>
Date: Thu Aug 02 21:16:21 2018

Vertically mirror custom theme frame images.

With the increased frame/tab heights in refresh, not all theme frame images are
tall enough.  Opque frame didn't tile at all, and glass frame repeat-tiled; both
look bad with certain themes (e.g. Into The Mist).  Vertically mirroring looks
better.

Bug:  866671 
Change-Id: I594e189196abff0ea55248be4ab9ca8f404fe3c2
Reviewed-on: https://chromium-review.googlesource.com/1157877
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580323}
[modify] https://crrev.com/4052d411adc06d21e60b8920edd486573bb347fe/ash/frame/custom_frame_header.cc
[modify] https://crrev.com/4052d411adc06d21e60b8920edd486573bb347fe/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/4052d411adc06d21e60b8920edd486573bb347fe/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/4052d411adc06d21e60b8920edd486573bb347fe/ui/views/window/frame_background.cc
[modify] https://crrev.com/4052d411adc06d21e60b8920edd486573bb347fe/ui/views/window/frame_background.h
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/040776588dc67db6d308e9715715c2342410ab85

commit 040776588dc67db6d308e9715715c2342410ab85
Author: Peter Kasting <pkasting@chromium.org>
Date: Fri Aug 03 19:00:35 2018

Draw theme frames starting at 16 DIP above tabs regardless of frame height.

This provides a consistent alignment point for theme authors, makes some themes
(e.g. Dots) look correct (other themes want other values, e.g. 0, so there's no
one "right way" here), and prevents mirroring above the top of the frame image
in Refresh, since no mode has a frame height above the tabs > 16 DIP.

Bug:  866671 
Change-Id: I1d79627e2735d54a332013ee70a2c71f107a898f
Reviewed-on: https://chromium-review.googlesource.com/1157878
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580615}
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/ash/frame/custom_frame_header.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/ash/frame/custom_frame_header.h
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/ash/frame/header_view.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/ash/public/cpp/mus_property_mirror_ash.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/ash/public/cpp/window_properties.h
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/ash/public/interfaces/window_properties.mojom
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/chrome/browser/themes/theme_properties.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/chrome/browser/themes/theme_properties.h
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/ui/views/window/frame_background.cc
[modify] https://crrev.com/040776588dc67db6d308e9715715c2342410ab85/ui/views/window/frame_background.h

Comment 14 by pkasting@chromium.org, Aug 3

Labels: Merge-Request-69
Requesting merge for CLs in comments 10-13.  This addresses a variety of positioning issues with themes, some caused by GM2 (comments 10, 12), some of which will be necessary with other yet-to-be-landed GM2 changes (comment 13), and one of which may have been pre-existing, I didn't check thoroughly (comment 11).
Project Member

Comment 15 by sheriffbot@chromium.org, Aug 4

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 16 Deleted

Comment 17 by gov...@chromium.org, Aug 5 

Are  CLs in comments 10-13 looking good in canary and safe to merge?  Also do we have enough automation tests coverage for these changes?

Comment 18 by pkasting@chromium.org, Aug 6 

I haven't seen any problems with these changes.

Test coverage for the theme stuff in general is rather spotty.  In particular for the changes here that affect alignment, there's no obvious right answer or way to unittest things.

Comment 19 by gov...@chromium.org, Aug 6

Labels: -Merge-Review-69 Merge-Approved-69
Approving merge for CLs in comments 10-13 based on comment #14 and #18. Please merge ASAP so we can take them in for this week Beta Release. Thank you.
Project Member

Comment 20 by bugdroid1@chromium.org, Aug 6

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab7714eae1cd601536d87f10973c686194bcd5ef

commit ab7714eae1cd601536d87f10973c686194bcd5ef
Author: Peter Kasting <pkasting@chromium.org>
Date: Mon Aug 06 21:09:36 2018

Account for TABSTRIP_TOOLBAR_OVERLAP in opaque frame top height.

This seems to affect only the painting of the frame background and Linux caption
buttons.  In testing the first and from code inspection of the second, both seem
to want "the actual height above the toolbar", which doesn't include this
overlap.

This eliminates an ugly 1 DIP misalignment of the theme background image bottom
with the toolbar top in some themes.

Bug:  866671 
Change-Id: I55d1aea4c3af656ae427373dff9bacc76bb932ab
Reviewed-on: https://chromium-review.googlesource.com/1157875
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579980}(cherry picked from commit 21a88094c09dec71b8236f83b7a28c4b13ff8220)
Reviewed-on: https://chromium-review.googlesource.com/1164183
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#437}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/ab7714eae1cd601536d87f10973c686194bcd5ef/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
Project Member

Comment 21 by bugdroid1@chromium.org, Aug 6 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/079586422068229e8bb7a7eaf6e994ea03f97f4a

commit 079586422068229e8bb7a7eaf6e994ea03f97f4a
Author: Peter Kasting <pkasting@chromium.org>
Date: Mon Aug 06 21:10:23 2018

Correctly align the toolbar background image with the tabstrip.

Bug:  866671 
Change-Id: Ibee4b8efc16008d4d181b3008860858c9d75efa5
Reviewed-on: https://chromium-review.googlesource.com/1157872
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579983}(cherry picked from commit 3a71aae9b6772d0900454b30f0dc09a579ae70a8)
Reviewed-on: https://chromium-review.googlesource.com/1164184
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#438}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/079586422068229e8bb7a7eaf6e994ea03f97f4a/chrome/browser/ui/views/toolbar/toolbar_view.cc
[modify] https://crrev.com/079586422068229e8bb7a7eaf6e994ea03f97f4a/chrome/browser/ui/views/toolbar/toolbar_view.h
Project Member

Comment 22 by bugdroid1@chromium.org, Aug 6 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c067360da0f092503bec05935c940790f6784a14

commit c067360da0f092503bec05935c940790f6784a14
Author: Peter Kasting <pkasting@chromium.org>
Date: Mon Aug 06 21:10:56 2018

Vertically mirror custom theme frame images.

With the increased frame/tab heights in refresh, not all theme frame images are
tall enough.  Opque frame didn't tile at all, and glass frame repeat-tiled; both
look bad with certain themes (e.g. Into The Mist).  Vertically mirroring looks
better.

Bug:  866671 
Change-Id: I594e189196abff0ea55248be4ab9ca8f404fe3c2
Reviewed-on: https://chromium-review.googlesource.com/1157877
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#580323}(cherry picked from commit 4052d411adc06d21e60b8920edd486573bb347fe)
Reviewed-on: https://chromium-review.googlesource.com/1164185
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#439}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/c067360da0f092503bec05935c940790f6784a14/ash/frame/custom_frame_header.cc
[modify] https://crrev.com/c067360da0f092503bec05935c940790f6784a14/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/c067360da0f092503bec05935c940790f6784a14/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/c067360da0f092503bec05935c940790f6784a14/ui/views/window/frame_background.cc
[modify] https://crrev.com/c067360da0f092503bec05935c940790f6784a14/ui/views/window/frame_background.h

Comment 23 by pkasting@chromium.org, Aug 6

Status: Fixed (was: Started)
Project Member

Comment 24 by bugdroid1@chromium.org, Aug 6 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fa52832da93b3d14658feb140b224d4632296153

commit fa52832da93b3d14658feb140b224d4632296153
Author: Peter Kasting <pkasting@chromium.org>
Date: Mon Aug 06 21:11:36 2018

Draw theme frames starting at 16 DIP above tabs regardless of frame height.

This provides a consistent alignment point for theme authors, makes some themes
(e.g. Dots) look correct (other themes want other values, e.g. 0, so there's no
one "right way" here), and prevents mirroring above the top of the frame image
in Refresh, since no mode has a frame height above the tabs > 16 DIP.

Bug:  866671 
Change-Id: I1d79627e2735d54a332013ee70a2c71f107a898f
Reviewed-on: https://chromium-review.googlesource.com/1157878
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#580615}(cherry picked from commit 040776588dc67db6d308e9715715c2342410ab85)
Reviewed-on: https://chromium-review.googlesource.com/1164204
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#440}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/ash/frame/custom_frame_header.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/ash/frame/custom_frame_header.h
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/ash/frame/header_view.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/ash/public/cpp/mus_property_mirror_ash.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/ash/public/cpp/window_properties.h
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/ash/public/interfaces/window_properties.mojom
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/chrome/browser/themes/theme_properties.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/chrome/browser/themes/theme_properties.h
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/ui/views/window/frame_background.cc
[modify] https://crrev.com/fa52832da93b3d14658feb140b224d4632296153/ui/views/window/frame_background.h

Comment 25 by gov...@chromium.org, Aug 14

Cc: abdulsyed@chromium.org
+abdulsyed@ fyi, M69 merges taken for Proj-MdRefresh .
►

Sign in to add a comment