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

Issue 829677 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
no longer active
Closed: Aug 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocked on:
issue 853276

Blocking:
issue 726619



Sign in to add a comment

[PIP] Adhere window to the video's aspect ratio.

Project Member Reported by apaci...@chromium.org, Apr 6 2018

Issue description

Currently, the window size is determined by a percentage (default 20%) of the display size. This means we don't take the aspect ratio into consideration.
 
Blocking: 726619
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 12 2018

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

commit b1c9ca1c62b9990e35f1955d59bfc741411eeb3f
Author: Jennifer Apacible <apacible@chromium.org>
Date: Thu Apr 12 07:49:38 2018

[Picture in Picture] Use video aspect ratio to determine window size.

Currently, Picture-in-Picture window sizes are determined solely from
calculating 20% of the primary display size. This change scales that
initial size also adhere to the aspect ratio of the video size.

BUG:  726621   829677 
Change-Id: I61c26f0dc9d44ebf77620f875aed005b7594d88f
Reviewed-on: https://chromium-review.googlesource.com/1001514
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550087}
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/chrome/browser/ui/browser.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/chrome/browser/ui/browser.h
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/chrome/browser/ui/views/overlay/overlay_window_views.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/chrome/browser/ui/views/overlay/overlay_window_views.h
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/browser/frame_host/render_frame_host_delegate.h
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/common/frame.mojom
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/public/browser/overlay_window.h
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/public/browser/picture_in_picture_window_controller.h
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/public/browser/web_contents_delegate.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/public/browser/web_contents_delegate.h
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/renderer/render_frame_impl.h
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/content/test/test_render_frame.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/media/blink/webmediaplayer_impl_unittest.cc
[modify] https://crrev.com/b1c9ca1c62b9990e35f1955d59bfc741411eeb3f/media/blink/webmediaplayer_params.h

Components: Blink>Media>PictureInPicture
Project Member

Comment 4 by bugdroid1@chromium.org, May 9 2018

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

commit c74fd383bd7daeb9882299222ce02f5c6b894fd8
Author: Jennifer Apacible <apacible@chromium.org>
Date: Wed May 09 01:48:36 2018

[Picture in Picture] Adhere to aspect ratio when resizing.

Currently, the OverlayWindow does not adhere to the video aspect ratio
while resizing. It only does that the first time the window is created.

This changes makes the window snap to the original (and intended) aspect
ratio when the window is resized. The window will always keep to the
original orientation, whether landscape or portrait (or square).

This change includes a non-smooth behavior where the user is able to
make the window whatever aspect ratio while in the process of resizing.
After the user 'completes' the action, such as letting go of a mouse
drag, the window will snap to adhere to the aspect ratio.

The layer contents will only be refitted in the window when it's
adhering to the aspect ratio.

There will be future work on making this smooth.

BUG:  829677 
Change-Id: Ic7bea948b901f7a5174ea5bd50c921c29fb2f487
Reviewed-on: https://chromium-review.googlesource.com/1043546
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557057}
[modify] https://crrev.com/c74fd383bd7daeb9882299222ce02f5c6b894fd8/chrome/browser/ui/views/overlay/overlay_window_views.cc
[modify] https://crrev.com/c74fd383bd7daeb9882299222ce02f5c6b894fd8/chrome/browser/ui/views/overlay/overlay_window_views.h

Status: Fixed (was: Started)
Status: Started (was: Fixed)
Project Member

Comment 7 by bugdroid1@chromium.org, May 25 2018

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

commit 089c3c38046b1d2239dc086582ec32852cba67b3
Author: Jennifer Apacible <apacible@chromium.org>
Date: Fri May 25 21:07:37 2018

[Picture in Picture] Adhere to aspect ratio for resize.

Currently, resizing sometimes doesn't always adhere to the aspect
ratio when the window also moves (origin point updates) alongside it.

OnNativeWidgetMove() is called in these cases. With this change, if the
sizes of the previous and new bounds are the same, the window was only
moved rather than resized. This change will make
OnNativeWidgetSizeChanged() handle the updates for |current_bounds_|.

BUG:  829677 
Change-Id: Id7df25c0a888efa4b2a7cad39cf83105aa03a759
Reviewed-on: https://chromium-review.googlesource.com/1071125
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: apacible <apacible@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562009}
[modify] https://crrev.com/089c3c38046b1d2239dc086582ec32852cba67b3/chrome/browser/ui/views/overlay/overlay_window_views.cc

Labels: -Pri-3 Merge-Request-68 OS-Chrome OS-Linux OS-Mac OS-Windows Pri-1
Requesting to merge #7
Labels: M-68
Labels: -Merge-Request-68
Cancelling merge request from #8
Project Member

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

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

commit 17982700ad2b8ed7b1c139ab80afdad17d15dd40
Author: Jennifer Apacible <apacible@chromium.org>
Date: Fri Jun 01 23:10:16 2018

[Picture in Picture] Letterbox video during window resizing.

This change updates the way Picture-in-Picture window resizing is
handled.

Previously, there was an issue where OverlayWindowViews attempts to
resize itself in the middle of already changing its size, which puts
aura into a conflicted state.

This is an initial step towards allowing the window to adhere to the
video aspect ratio by just letterboxing the video. The window can be
of any dimension as long as it is constrained by the already given
min and max values of the window.

Bug:  845747   829677 
Change-Id: I6a120f3d346f7aa64909b06d5f48068497d8d7d7
Reviewed-on: https://chromium-review.googlesource.com/1080129
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563851}
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/chrome/browser/ui/views/overlay/overlay_window_views.cc
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/chrome/browser/ui/views/overlay/overlay_window_views.h
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/content/browser/picture_in_picture/overlay_surface_embedder.cc
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/content/public/browser/overlay_window.h
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/content/shell/browser/layout_test/layout_test_content_browser_client.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 5

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

commit 46630165101db53080bf718a258a3be606f7c633
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Thu Jul 05 01:45:09 2018

[Picture in Picture] Set aspect ratio for OverlayWindowViews.

This allows the Picture-in-Picture window to adhere to the given aspect
ratio.

Bug:  829677 
Change-Id: I7a78d4bfa369ab6126dc16951015aa207ed889c1
Reviewed-on: https://chromium-review.googlesource.com/1102894
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572689}
[modify] https://crrev.com/46630165101db53080bf718a258a3be606f7c633/chrome/browser/ui/views/overlay/overlay_window_views.cc
[modify] https://crrev.com/46630165101db53080bf718a258a3be606f7c633/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc

Blockedon: 853276
Labels: -M-68 ReleaseBlock-Stable Target-69
Project Member

Comment 15 by sheriffbot@chromium.org, Jul 27

Cc: mlamouri@chromium.org
This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: M-69
Adding M-69 as sheriffbot@ didn't get the memo that it was deprecated it seems :)
Friendly ping to get an update as it is marked as RB stable.
Thanks..!

apacible@,
Gentle ping to get an update on this issue as it is marked as RB stable.
Thanks..!
M69 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.
Status: Fixed (was: Started)
This is done per 853276.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-69; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-69 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD

Sign in to add a comment