Skip to content

Commit

Permalink
Pip 2.0: Merge VideoOverlayWindowViews and OverlayWindowViews
Browse files Browse the repository at this point in the history
When exploring Document Picture-in-Picture first[1], a new
DocumentOverlayWindowViews subtype was introduced which caused the
creation of VideoOverlayWindowViews from OverlayWindowViews. Since it is
not necessary anymore, this CL merges  VideoOverlayWindowViews and
OverlayWindowViews and cleans up many left overs.

This CL has no functional changes.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3252789

Bug: 1379386
Change-Id: I64ec577f321da8b80ea3a2d0fdd689e9b1cb3671
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4040140
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Commit-Queue: Fr <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/main@{#1075095}
  • Loading branch information
beaufortfrancois authored and Chromium LUCI CQ committed Nov 23, 2022
1 parent a6f4f71 commit eac774f
Show file tree
Hide file tree
Showing 19 changed files with 678 additions and 936 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/picture_in_picture/DEPS
@@ -1,7 +1,7 @@
specific_include_rules = {
"video_picture_in_picture_window_controller_browsertest\.cc": [
"+chrome/browser/ui/views/overlay/hang_up_button.h",
"+chrome/browser/ui/views/overlay/overlay_window_views.h",
"+chrome/browser/ui/views/overlay/video_overlay_window_views.h",
"+chrome/browser/ui/views/overlay/playback_image_button.h",
"+chrome/browser/ui/views/overlay/skip_ad_label_button.h",
"+chrome/browser/ui/views/overlay/toggle_camera_button.h",
Expand Down
Expand Up @@ -21,12 +21,12 @@
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/views/overlay/hang_up_button.h"
#include "chrome/browser/ui/views/overlay/overlay_window_views.h"
#include "chrome/browser/ui/views/overlay/playback_image_button.h"
#include "chrome/browser/ui/views/overlay/skip_ad_label_button.h"
#include "chrome/browser/ui/views/overlay/toggle_camera_button.h"
#include "chrome/browser/ui/views/overlay/toggle_microphone_button.h"
#include "chrome/browser/ui/views/overlay/track_image_button.h"
#include "chrome/browser/ui/views/overlay/video_overlay_window_views.h"
#include "chrome/browser/ui/web_applications/app_browser_controller.h"
#include "chrome/browser/ui/web_applications/test/web_app_browsertest_util.h"
#include "chrome/browser/ui/web_applications/web_app_controller_browsertest.h"
Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/ui/BUILD.gn
Expand Up @@ -4712,8 +4712,6 @@ static_library("ui") {
"views/overlay/hang_up_button.h",
"views/overlay/overlay_window_image_button.cc",
"views/overlay/overlay_window_image_button.h",
"views/overlay/overlay_window_views.cc",
"views/overlay/overlay_window_views.h",
"views/overlay/playback_image_button.cc",
"views/overlay/playback_image_button.h",
"views/overlay/resize_handle_button.cc",
Expand Down
19 changes: 2 additions & 17 deletions chrome/browser/ui/android/overlay/overlay_window_android.cc
Expand Up @@ -23,13 +23,6 @@ content::VideoOverlayWindow::Create(
return std::make_unique<OverlayWindowAndroid>(controller);
}

// static
std::unique_ptr<content::DocumentOverlayWindow>
content::DocumentOverlayWindow::Create(
DocumentPictureInPictureWindowController* controller) {
return nullptr;
}

OverlayWindowAndroid::OverlayWindowAndroid(
content::VideoPictureInPictureWindowController* controller)
: window_android_(nullptr),
Expand Down Expand Up @@ -213,15 +206,11 @@ void OverlayWindowAndroid::CloseInternal() {
Java_PictureInPictureActivity_close(env, java_ref_.get(env));
}

bool OverlayWindowAndroid::IsActive() {
return true;
}

bool OverlayWindowAndroid::IsVisible() {
bool OverlayWindowAndroid::IsActive() const {
return true;
}

bool OverlayWindowAndroid::IsAlwaysOnTop() {
bool OverlayWindowAndroid::IsVisible() const {
return true;
}

Expand Down Expand Up @@ -330,10 +319,6 @@ void OverlayWindowAndroid::SetSurfaceId(const viz::SurfaceId& surface_id) {
cc::DeadlinePolicy::UseDefaultDeadline());
}

cc::Layer* OverlayWindowAndroid::GetLayerForTesting() {
return nullptr;
}

void OverlayWindowAndroid::MaybeNotifyVisibleActionsChanged() {
if (java_ref_.is_uninitialized())
return;
Expand Down
10 changes: 3 additions & 7 deletions chrome/browser/ui/android/overlay/overlay_window_android.h
Expand Up @@ -57,17 +57,14 @@ class OverlayWindowAndroid : public content::VideoOverlayWindow,
void OnActivityStopped() override;
void OnActivityStarted() override {}

// OverlayWindow implementation.
bool IsActive() override;
// VideoOverlayWindow implementation.
bool IsActive() const override;
void Close() override;
void ShowInactive() override {}
void Hide() override;
bool IsVisible() override;
bool IsAlwaysOnTop() override;
bool IsVisible() const override;
gfx::Rect GetBounds() override;
void UpdateNaturalSize(const gfx::Size& natural_size) override;

// VideoOverlayWindow implementation
void SetPlaybackState(PlaybackState playback_state) override;
void SetPlayPauseButtonVisibility(bool is_visible) override;
void SetSkipAdButtonVisibility(bool is_visible) override {}
Expand All @@ -79,7 +76,6 @@ class OverlayWindowAndroid : public content::VideoOverlayWindow,
void SetToggleCameraButtonVisibility(bool is_visible) override;
void SetHangUpButtonVisibility(bool is_visible) override;
void SetSurfaceId(const viz::SurfaceId& surface_id) override;
cc::Layer* GetLayerForTesting() override;

private:
// Notify PictureInPictureActivity that visible actions have changed.
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/views/overlay/close_image_button.cc
Expand Up @@ -38,9 +38,9 @@ CloseImageButton::CloseImageButton(PressedCallback callback)

void CloseImageButton::SetPosition(
const gfx::Size& size,
OverlayWindowViews::WindowQuadrant quadrant) {
VideoOverlayWindowViews::WindowQuadrant quadrant) {
#if BUILDFLAG(IS_CHROMEOS_ASH)
if (quadrant == OverlayWindowViews::WindowQuadrant::kBottomLeft) {
if (quadrant == VideoOverlayWindowViews::WindowQuadrant::kBottomLeft) {
views::ImageButton::SetPosition(
gfx::Point(kCloseButtonMargin, kCloseButtonMargin));
return;
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/views/overlay/close_image_button.h
Expand Up @@ -6,7 +6,7 @@
#define CHROME_BROWSER_UI_VIEWS_OVERLAY_CLOSE_IMAGE_BUTTON_H_

#include "chrome/browser/ui/views/overlay/overlay_window_image_button.h"
#include "chrome/browser/ui/views/overlay/overlay_window_views.h"
#include "chrome/browser/ui/views/overlay/video_overlay_window_views.h"
#include "ui/base/metadata/metadata_header_macros.h"

// An image button representing a close button.
Expand All @@ -21,7 +21,7 @@ class CloseImageButton : public OverlayWindowImageButton {

// Sets the position of itself with an offset from the given window size.
void SetPosition(const gfx::Size& size,
OverlayWindowViews::WindowQuadrant quadrant);
VideoOverlayWindowViews::WindowQuadrant quadrant);
};

#endif // CHROME_BROWSER_UI_VIEWS_OVERLAY_CLOSE_IMAGE_BUTTON_H_

0 comments on commit eac774f

Please sign in to comment.