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

Issue 821874 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
no longer active
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 726619



Sign in to add a comment

[PIP] Make some functionality available for use in more generic content/

Project Member Reported by apaci...@chromium.org, Mar 14 2018

Issue description

Currently, everything is implemented under chrome/. Move more generic (non chrome specific) files to content/.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 5 2018

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

commit d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c
Author: Jennifer Apacible <apacible@chromium.org>
Date: Thu Apr 05 15:29:12 2018

[Picture in Picture] Move generic code to content/.

Currently, all Picture in Picture code is housed in chrome/. Since
only some of the Picture in Picture work is Chrome specific, we move
the more generic work into content/.

This change completely moves the PictureInPictureWindowController
and OverlaySurfaceEmbedder into content/.

This change will also pave for some future work related to user
initiated commands from the window, such as play/pause. IPC messaging
for MediaPlayerDelegateMsg must be done through content/ rather than
in chrome/.

OverlayWindowViews continues to be the desktop Chrome implementation
of OverlayWindow, which is now moved into content/. With the
convergence of historically platform-dependent implementations (views
vs cocoa) into just views, there will only be one window implementation
for Chrome.

BUG:  726619   821874 
Change-Id: I6ef73552c6729aec235f4a6d176fd4109f6674b3
Reviewed-on: https://chromium-review.googlesource.com/972841
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548428}
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/BUILD.gn
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/chrome_content_browser_client.h
[delete] https://crrev.com/bdc2b6900dc5a33bc44b60fc68dbe87bd04305de/chrome/browser/picture_in_picture/picture_in_picture_window_controller.cc
[delete] https://crrev.com/bdc2b6900dc5a33bc44b60fc68dbe87bd04305de/chrome/browser/picture_in_picture/picture_in_picture_window_controller.h
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/ui/android/overlay/overlay_window_android.cc
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/ui/browser.cc
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/ui/browser.h
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/ui/views/overlay/OWNERS
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/ui/views/overlay/overlay_window_views.cc
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/chrome/browser/ui/views/overlay/overlay_window_views.h
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/browser/BUILD.gn
[rename] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/browser/picture_in_picture/OWNERS
[rename] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/browser/picture_in_picture/overlay_surface_embedder.cc
[rename] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/browser/picture_in_picture/overlay_surface_embedder.h
[add] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc
[add] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/public/browser/BUILD.gn
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/public/browser/content_browser_client.h
[rename] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/public/browser/overlay_window.h
[add] https://crrev.com/d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c/content/public/browser/picture_in_picture_window_controller.h

Project Member

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

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

commit 746bf6d72fce5b360345ead1e7f27a6af38bd2ba
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Thu Apr 05 16:25:30 2018

Revert "[Picture in Picture] Move generic code to content/."

This reverts commit d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c.

Reason for revert: Breaks compile on WinMSVC64 (dbg), due to unreachable code (the return after the #endif):

https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.win%2FWinMSVC64__dbg_%2F4540%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

c:\b\c\b\win\src\chrome\browser\chrome_content_browser_client.cc(4172) : warning C4702: unreachable code

Original change's description:
> [Picture in Picture] Move generic code to content/.
> 
> Currently, all Picture in Picture code is housed in chrome/. Since
> only some of the Picture in Picture work is Chrome specific, we move
> the more generic work into content/.
> 
> This change completely moves the PictureInPictureWindowController
> and OverlaySurfaceEmbedder into content/.
> 
> This change will also pave for some future work related to user
> initiated commands from the window, such as play/pause. IPC messaging
> for MediaPlayerDelegateMsg must be done through content/ rather than
> in chrome/.
> 
> OverlayWindowViews continues to be the desktop Chrome implementation
> of OverlayWindow, which is now moved into content/. With the
> convergence of historically platform-dependent implementations (views
> vs cocoa) into just views, there will only be one window implementation
> for Chrome.
> 
> BUG:  726619   821874 
> Change-Id: I6ef73552c6729aec235f4a6d176fd4109f6674b3
> Reviewed-on: https://chromium-review.googlesource.com/972841
> Commit-Queue: apacible <apacible@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#548428}

TBR=ellyjones@chromium.org,kinuko@chromium.org,lazyboy@chromium.org,mlamouri@chromium.org,apacible@chromium.org

Change-Id: I3a18da4e27bbff14d2acfa11707727d46e033f73
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/998173
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548446}
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/BUILD.gn
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/chrome_content_browser_client.h
[rename] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/overlay/OWNERS
[rename] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/overlay/overlay_surface_embedder.cc
[rename] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/overlay/overlay_surface_embedder.h
[rename] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/overlay/overlay_window.h
[add] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/picture_in_picture/picture_in_picture_window_controller.cc
[add] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/picture_in_picture/picture_in_picture_window_controller.h
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/ui/android/overlay/overlay_window_android.cc
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/ui/browser.cc
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/ui/browser.h
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/ui/views/overlay/OWNERS
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/ui/views/overlay/overlay_window_views.cc
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/chrome/browser/ui/views/overlay/overlay_window_views.h
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/content/browser/BUILD.gn
[delete] https://crrev.com/24a94c7493d3c2497082b05b1553a3932b898e43/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc
[delete] https://crrev.com/24a94c7493d3c2497082b05b1553a3932b898e43/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/content/public/browser/BUILD.gn
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/746bf6d72fce5b360345ead1e7f27a6af38bd2ba/content/public/browser/content_browser_client.h
[delete] https://crrev.com/24a94c7493d3c2497082b05b1553a3932b898e43/content/public/browser/picture_in_picture_window_controller.h

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

Comment 5 by bugdroid1@chromium.org, Apr 6 2018

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

commit 4f854a804a031dade96ce4b9f4660ea3423302c6
Author: Jennifer Apacible <apacible@chromium.org>
Date: Fri Apr 06 22:22:11 2018

Reland "[Picture in Picture] Move generic code to content/."

This is a reland of d1f4cdd4af23f9aca1ffb7c939c48e19e6308c1c

This change places the offending unreachable code in an #else
#endif block.

Original change's description:
> [Picture in Picture] Move generic code to content/.
>
> Currently, all Picture in Picture code is housed in chrome/. Since
> only some of the Picture in Picture work is Chrome specific, we move
> the more generic work into content/.
>
> This change completely moves the PictureInPictureWindowController
> and OverlaySurfaceEmbedder into content/.
>
> This change will also pave for some future work related to user
> initiated commands from the window, such as play/pause. IPC messaging
> for MediaPlayerDelegateMsg must be done through content/ rather than
> in chrome/.
>
> OverlayWindowViews continues to be the desktop Chrome implementation
> of OverlayWindow, which is now moved into content/. With the
> convergence of historically platform-dependent implementations (views
> vs cocoa) into just views, there will only be one window implementation
> for Chrome.
>
> BUG:  726619   821874 
> Change-Id: I6ef73552c6729aec235f4a6d176fd4109f6674b3
> Reviewed-on: https://chromium-review.googlesource.com/972841
> Commit-Queue: apacible <apacible@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#548428}

TBR=kinuko@chromium.org,mlamouri@chromium.org,lazyboy@chromium.org,
ellyjones@chromium.org

BUG:  726619   821874 
Change-Id: I10fbd1d682108d351119158c4da00ee4c1915106
Reviewed-on: https://chromium-review.googlesource.com/998186
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: apacible <apacible@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548949}
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/BUILD.gn
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/chrome_content_browser_client.h
[delete] https://crrev.com/e534b672f07210fd804e234f54685678b2cc8d50/chrome/browser/picture_in_picture/picture_in_picture_window_controller.cc
[delete] https://crrev.com/e534b672f07210fd804e234f54685678b2cc8d50/chrome/browser/picture_in_picture/picture_in_picture_window_controller.h
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/ui/android/overlay/overlay_window_android.cc
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/ui/browser.cc
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/ui/browser.h
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/ui/views/overlay/OWNERS
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/ui/views/overlay/overlay_window_views.cc
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/chrome/browser/ui/views/overlay/overlay_window_views.h
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/browser/BUILD.gn
[rename] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/browser/picture_in_picture/OWNERS
[rename] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/browser/picture_in_picture/overlay_surface_embedder.cc
[rename] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/browser/picture_in_picture/overlay_surface_embedder.h
[add] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc
[add] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/public/browser/BUILD.gn
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/public/browser/content_browser_client.h
[rename] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/public/browser/overlay_window.h
[add] https://crrev.com/4f854a804a031dade96ce4b9f4660ea3423302c6/content/public/browser/picture_in_picture_window_controller.h

Components: Blink>Media>PictureInPicture

Sign in to add a comment