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

Issue 853276 link

Starred by 4 users
Status: Fixed
Owner:
█ apaci...@chromium.org
no longer active
Closed: Aug 13
Cc:
mlamouri@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 829677



Sign in to add a comment

[Widget] Add ability to define aspect ratio.

Project Member Reported by apaci...@chromium.org, Jun 15 2018 
Currently, there is no way for windows in Chrome do not adhere to any aspect ratio. When showing content on windows that need to adhere to this, it is customary to use letterboxing or pillarboxing (i.e. black bars around video to preserve the clips original aspect ratio in film).

This change is meant to allow windows to be able to adhere to a certain aspect ratio to current and future views::Widgets.
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 22 2018 

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

commit dff8b5eb611609e9b3046a2e57d1fff39961a186
Author: Jennifer Apacible <apacible@chromium.org>
Date: Fri Jun 22 07:21:02 2018

[Widget] Add option to adhere to a given aspect ratio.

This change adds the views::Widget API for setting the aspect ratio
and implements it for Linux.

Bug:  853276 
Change-Id: I9fe4dbc2059c2c7ee3a6d84b55aa29b11a31112c
Reviewed-on: https://chromium-review.googlesource.com/1102881
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569547}
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/mus/desktop_window_tree_host_mus.h
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/desktop_aura/desktop_native_widget_aura.h
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/desktop_aura/desktop_window_tree_host.h
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/desktop_aura/desktop_window_tree_host_platform.h
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/desktop_aura/desktop_window_tree_host_win.h
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/native_widget_aura.h
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/native_widget_mac.h
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/native_widget_private.h
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/widget.cc
[modify] https://crrev.com/dff8b5eb611609e9b3046a2e57d1fff39961a186/ui/views/widget/widget.h
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 7 

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

commit ee0b7c5b4cfe8550498787744bcc769981209f5f
Author: Jennifer Apacible <apacible@chromium.org>
Date: Sat Jul 07 05:46:16 2018

[Widget] Implement SetAspectRatio() for Mac.

This change also updates the SetAspectRatio() API to take in gfx::SizeF()
rather than gfx::Size(). MacOS only allows integer multiples of
|aspect_ratio| when setting |aspectRatio| and |contentAspectRatio|.

Bug:  853276 
Change-Id: Ifa5065b634a48b08c9dd4eecd9f796b7208e17d0
Reviewed-on: https://chromium-review.googlesource.com/1112206
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573151}
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/chrome/browser/ui/views/overlay/overlay_window_views.cc
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/mus/desktop_window_tree_host_mus.h
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/desktop_aura/desktop_native_widget_aura.h
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/desktop_aura/desktop_window_tree_host.h
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/desktop_aura/desktop_window_tree_host_platform.h
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/desktop_aura/desktop_window_tree_host_win.h
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/native_widget_aura.h
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/native_widget_mac.h
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/native_widget_mac.mm
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/native_widget_private.h
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/widget.cc
[modify] https://crrev.com/ee0b7c5b4cfe8550498787744bcc769981209f5f/ui/views/widget/widget.h

Comment 3 by apaci...@chromium.org, Jul 10

Blocking: 829677

Comment 4 by mlamouri@chromium.org, Jul 27

Cc: mlamouri@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 3 

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

commit 456d6e533cfb4531995e0ef52c279d4b5aa8a352
Author: Jennifer Apacible <apacible@chromium.org>
Date: Fri Aug 03 19:34:14 2018

[Widget] Implement SetAspectRatio for Windows.

The WM_SIZING message is now handled by HWNDMessageHandler. When the
aspect ratio is set for a window on Windows OS, the handler will adjust
the final RECT bounds to adhere to the given aspect ratio.

Bug:  853276 
Change-Id: I54099581012d3f59e3d0535ed1285a25bb512cd9
Reviewed-on: https://chromium-review.googlesource.com/1136226
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580626}
[modify] https://crrev.com/456d6e533cfb4531995e0ef52c279d4b5aa8a352/ui/views/BUILD.gn
[modify] https://crrev.com/456d6e533cfb4531995e0ef52c279d4b5aa8a352/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc
[modify] https://crrev.com/456d6e533cfb4531995e0ef52c279d4b5aa8a352/ui/views/win/DEPS
[modify] https://crrev.com/456d6e533cfb4531995e0ef52c279d4b5aa8a352/ui/views/win/hwnd_message_handler.cc
[modify] https://crrev.com/456d6e533cfb4531995e0ef52c279d4b5aa8a352/ui/views/win/hwnd_message_handler.h
[add] https://crrev.com/456d6e533cfb4531995e0ef52c279d4b5aa8a352/ui/views/window/window_resize_utils.cc
[add] https://crrev.com/456d6e533cfb4531995e0ef52c279d4b5aa8a352/ui/views/window/window_resize_utils.h
[add] https://crrev.com/456d6e533cfb4531995e0ef52c279d4b5aa8a352/ui/views/window/window_resize_utils_unittest.cc

Comment 6 by apaci...@chromium.org, Aug 6

Labels: Merge-Request-69
Requesting merge #5 to support Picture-in-Picture.
Project Member

Comment 7 by sheriffbot@chromium.org, Aug 6

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
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 8 by gov...@chromium.org, Aug 6 

Is the change listed at #5 well baked/verified in canary, having enough automation tests coverage and safe to merge?

Comment 9 by apaci...@chromium.org, Aug 6 

Yes. We've also verified it on Canary by two different people on various Windows machines.

Comment 10 by mlamouri@chromium.org, Aug 6 

Just spent a few minutes playing with the change on Windows on Canary and it works just fine :) It should be safe as it only impacts this one feature that is behind a finch flag. It's coming with unit tests.

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

Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #9 and #10. Please merge now. Thank you.

Comment 12 Deleted

Comment 13 by apaci...@chromium.org, Aug 6 

(deleted -- posted to the wrong bug)
Project Member

Comment 14 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/+/b87833cdf49ccec583f5194afa1ef03309cf5e48

commit b87833cdf49ccec583f5194afa1ef03309cf5e48
Author: Jennifer Apacible <apacible@chromium.org>
Date: Mon Aug 06 20:01:58 2018

[Widget] Implement SetAspectRatio for Windows.

The WM_SIZING message is now handled by HWNDMessageHandler. When the
aspect ratio is set for a window on Windows OS, the handler will adjust
the final RECT bounds to adhere to the given aspect ratio.

Bug:  853276 
Change-Id: I54099581012d3f59e3d0535ed1285a25bb512cd9
Reviewed-on: https://chromium-review.googlesource.com/1136226
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#580626}(cherry picked from commit 456d6e533cfb4531995e0ef52c279d4b5aa8a352)
Reviewed-on: https://chromium-review.googlesource.com/1163944
Reviewed-by: apacible <apacible@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#432}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/b87833cdf49ccec583f5194afa1ef03309cf5e48/ui/views/BUILD.gn
[modify] https://crrev.com/b87833cdf49ccec583f5194afa1ef03309cf5e48/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc
[modify] https://crrev.com/b87833cdf49ccec583f5194afa1ef03309cf5e48/ui/views/win/DEPS
[modify] https://crrev.com/b87833cdf49ccec583f5194afa1ef03309cf5e48/ui/views/win/hwnd_message_handler.cc
[modify] https://crrev.com/b87833cdf49ccec583f5194afa1ef03309cf5e48/ui/views/win/hwnd_message_handler.h
[add] https://crrev.com/b87833cdf49ccec583f5194afa1ef03309cf5e48/ui/views/window/window_resize_utils.cc
[add] https://crrev.com/b87833cdf49ccec583f5194afa1ef03309cf5e48/ui/views/window/window_resize_utils.h
[add] https://crrev.com/b87833cdf49ccec583f5194afa1ef03309cf5e48/ui/views/window/window_resize_utils_unittest.cc
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 10 

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

commit 7044fb57e1ce3506a1eea2a67a6ef663a8820379
Author: Jennifer Apacible <apacible@chromium.org>
Date: Fri Aug 10 01:54:37 2018

[Widget] Implement SetAspectRatio for CrOS.

For ChromeOS, ash (window manager), will handle adjusting the window
size to adhere to the specified aspect ratio.

This change adds a new aura constant to keep track of the window's
aspect ratio, if any. This is used in WindowResizer to determine the new
window bounds.

Bug:  853276 
Change-Id: I4c7eaa6252e26ffccf3ac10a4dafd635a3b556d1
Reviewed-on: https://chromium-review.googlesource.com/1164483
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582010}
[modify] https://crrev.com/7044fb57e1ce3506a1eea2a67a6ef663a8820379/ash/BUILD.gn
[add] https://crrev.com/7044fb57e1ce3506a1eea2a67a6ef663a8820379/ash/wm/default_window_resizer_unittest.cc
[modify] https://crrev.com/7044fb57e1ce3506a1eea2a67a6ef663a8820379/ash/wm/window_resizer.cc
[modify] https://crrev.com/7044fb57e1ce3506a1eea2a67a6ef663a8820379/ash/wm/window_resizer.h
[modify] https://crrev.com/7044fb57e1ce3506a1eea2a67a6ef663a8820379/ui/aura/client/aura_constants.cc
[modify] https://crrev.com/7044fb57e1ce3506a1eea2a67a6ef663a8820379/ui/aura/client/aura_constants.h
[modify] https://crrev.com/7044fb57e1ce3506a1eea2a67a6ef663a8820379/ui/views/widget/native_widget_aura.cc
[modify] https://crrev.com/7044fb57e1ce3506a1eea2a67a6ef663a8820379/ui/views/widget/native_widget_aura.h

Comment 16 by mlamouri@chromium.org, Aug 13

Labels: Merge-Request-69
Requesting Merge to 69 as it's an important fix for the Picture-in-Picture feature. It should only impact the feature which is guarded by a Finch flag.
Project Member

Comment 17 by sheriffbot@chromium.org, Aug 13

Labels: -Merge-Request-69 Merge-Review-69
This bug requires manual review: DEPS changes referenced in bugdroid comments.
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 18 by mlamouri@chromium.org, Aug 13 

Worth pointing that this is blocking a RBS bug:  bug 829677

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

Is this merge request ONLY for cl listed at #15?

Comment 20 by mlamouri@chromium.org, Aug 13 

Yes, that's correct, only #15.

Comment 21 by gov...@chromium.org, Aug 13

Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 for CL listed at #15 based on comment #16 and #18. Please merge ASAP. Thank you.
Project Member

Comment 22 by bugdroid1@chromium.org, Aug 13

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

commit 9bf66039995e79be89082ae1d80a742f6da74fbe
Author: Jennifer Apacible <apacible@chromium.org>
Date: Mon Aug 13 20:33:26 2018

[Widget] Implement SetAspectRatio for CrOS.

For ChromeOS, ash (window manager), will handle adjusting the window
size to adhere to the specified aspect ratio.

This change adds a new aura constant to keep track of the window's
aspect ratio, if any. This is used in WindowResizer to determine the new
window bounds.

Bug:  853276 
Change-Id: I4c7eaa6252e26ffccf3ac10a4dafd635a3b556d1
Reviewed-on: https://chromium-review.googlesource.com/1164483
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#582010}(cherry picked from commit 7044fb57e1ce3506a1eea2a67a6ef663a8820379)
Reviewed-on: https://chromium-review.googlesource.com/1173078
Cr-Commit-Position: refs/branch-heads/3497@{#586}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/9bf66039995e79be89082ae1d80a742f6da74fbe/ash/BUILD.gn
[add] https://crrev.com/9bf66039995e79be89082ae1d80a742f6da74fbe/ash/wm/default_window_resizer_unittest.cc
[modify] https://crrev.com/9bf66039995e79be89082ae1d80a742f6da74fbe/ash/wm/window_resizer.cc
[modify] https://crrev.com/9bf66039995e79be89082ae1d80a742f6da74fbe/ash/wm/window_resizer.h
[modify] https://crrev.com/9bf66039995e79be89082ae1d80a742f6da74fbe/ui/aura/client/aura_constants.cc
[modify] https://crrev.com/9bf66039995e79be89082ae1d80a742f6da74fbe/ui/aura/client/aura_constants.h
[modify] https://crrev.com/9bf66039995e79be89082ae1d80a742f6da74fbe/ui/views/widget/native_widget_aura.cc
[modify] https://crrev.com/9bf66039995e79be89082ae1d80a742f6da74fbe/ui/views/widget/native_widget_aura.h

Comment 23 by mlamouri@chromium.org, Aug 13

Status: Fixed (was: Started)
►

Sign in to add a comment