Skip to content

Commit 05cac71

Browse files
committed
Picture-in-Picture: notifies the controller when window closed via system.
When the internal object is destroyed, notify the controller so it can reset its internal state. It allows the user to close the window via any system/window manager UI. Bug: 863842 Change-Id: Ica84f4ffee2578658a60e0ae708a4e327d293077 Reviewed-on: https://chromium-review.googlesource.com/1147252 Reviewed-by: Camille Lamy <clamy@chromium.org> Reviewed-by: apacible <apacible@chromium.org> Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#577594}(cherry picked from commit c04a9a2) Reviewed-on: https://chromium-review.googlesource.com/1150580 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#83} Cr-Branched-From: 271eaf5-refs/heads/master@{#576753}
1 parent 7f5bca9 commit 05cac71

File tree

6 files changed

+71
-9
lines changed

6 files changed

+71
-9
lines changed

chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -916,4 +916,27 @@ IN_PROC_BROWSER_TEST_F(PictureInPictureWindowControllerBrowserTest,
916916

917917
#endif // defined(OS_LINUX) && !defined(OS_CHROMEOS)
918918

919+
// Tests that the Picture-in-Picture state is properly updated when the window
920+
// is closed at a system level.
921+
IN_PROC_BROWSER_TEST_F(PictureInPictureWindowControllerBrowserTest,
922+
CloseWindowNotifiesController) {
923+
LoadTabAndEnterPictureInPicture(browser());
924+
925+
content::WebContents* active_web_contents =
926+
browser()->tab_strip_model()->GetActiveWebContents();
927+
928+
OverlayWindowViews* overlay_window = static_cast<OverlayWindowViews*>(
929+
window_controller()->GetWindowForTesting());
930+
ASSERT_TRUE(overlay_window);
931+
ASSERT_TRUE(overlay_window->IsVisible());
932+
933+
// Simulate closing from the system.
934+
overlay_window->OnNativeWidgetDestroyed();
935+
936+
bool in_picture_in_picture = false;
937+
ASSERT_TRUE(ExecuteScriptAndExtractBool(
938+
active_web_contents, "isInPictureInPicture();", &in_picture_in_picture));
939+
EXPECT_FALSE(in_picture_in_picture);
940+
}
941+
919942
#endif // !defined(OS_ANDROID)

chrome/browser/ui/views/overlay/overlay_window_views.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,10 @@ void OverlayWindowViews::OnNativeWidgetSizeChanged(const gfx::Size& new_size) {
588588
views::Widget::OnNativeWidgetSizeChanged(new_size);
589589
}
590590

591+
void OverlayWindowViews::OnNativeWidgetDestroyed() {
592+
controller_->OnWindowDestroyed();
593+
}
594+
591595
void OverlayWindowViews::TogglePlayPause() {
592596
// Retrieve expected active state based on what command was sent in
593597
// TogglePlayPause() since the IPC message may not have been propogated

chrome/browser/ui/views/overlay/overlay_window_views.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class OverlayWindowViews : public content::OverlayWindow, public views::Widget {
5050
void OnNativeBlur() override;
5151
void OnNativeWidgetMove() override;
5252
void OnNativeWidgetSizeChanged(const gfx::Size& new_size) override;
53+
void OnNativeWidgetDestroyed() override;
5354

5455
// Gets the bounds of the controls.
5556
gfx::Rect GetCloseControlsBounds();

content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ PictureInPictureWindowControllerImpl::PictureInPictureWindowControllerImpl(
5858

5959
media_web_contents_observer_ = initiator_->media_web_contents_observer();
6060

61-
window_ =
62-
GetContentClient()->browser()->CreateWindowForPictureInPicture(this);
61+
EnsureWindow();
6362
DCHECK(window_) << "Picture in Picture requires a valid window.";
6463
}
6564

@@ -84,23 +83,25 @@ void PictureInPictureWindowControllerImpl::ClickCustomControl(
8483
}
8584

8685
void PictureInPictureWindowControllerImpl::Close(bool should_pause_video) {
87-
DCHECK(window_);
88-
89-
if (!window_->IsVisible())
86+
if (!window_ || !window_->IsVisible())
9087
return;
9188

9289
window_->Hide();
93-
initiator_->SetHasPictureInPictureVideo(false);
94-
95-
surface_id_ = viz::SurfaceId();
90+
CloseInternal(should_pause_video);
91+
}
9692

97-
OnLeavingPictureInPicture(should_pause_video);
93+
void PictureInPictureWindowControllerImpl::OnWindowDestroyed() {
94+
window_ = nullptr;
95+
embedder_ = nullptr;
96+
CloseInternal(true /* should_pause_video */);
9897
}
9998

10099
void PictureInPictureWindowControllerImpl::EmbedSurface(
101100
const viz::SurfaceId& surface_id,
102101
const gfx::Size& natural_size) {
102+
EnsureWindow();
103103
DCHECK(window_);
104+
104105
DCHECK(surface_id.is_valid());
105106
surface_id_ = surface_id;
106107

@@ -195,4 +196,21 @@ void PictureInPictureWindowControllerImpl::OnLeavingPictureInPicture(
195196
}
196197
}
197198

199+
void PictureInPictureWindowControllerImpl::CloseInternal(
200+
bool should_pause_video) {
201+
initiator_->SetHasPictureInPictureVideo(false);
202+
203+
surface_id_ = viz::SurfaceId();
204+
205+
OnLeavingPictureInPicture(should_pause_video);
206+
}
207+
208+
void PictureInPictureWindowControllerImpl::EnsureWindow() {
209+
if (window_)
210+
return;
211+
212+
window_ =
213+
GetContentClient()->browser()->CreateWindowForPictureInPicture(this);
214+
}
215+
198216
} // namespace content

content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class PictureInPictureWindowControllerImpl
3636
// PictureInPictureWindowController:
3737
CONTENT_EXPORT gfx::Size Show() override;
3838
CONTENT_EXPORT void Close(bool should_pause_video) override;
39+
CONTENT_EXPORT void OnWindowDestroyed() override;
3940
CONTENT_EXPORT void ClickCustomControl(
4041
const std::string& control_id) override;
4142
CONTENT_EXPORT void EmbedSurface(const viz::SurfaceId& surface_id,
@@ -59,6 +60,14 @@ class PictureInPictureWindowControllerImpl
5960
// Signal to the media player that |this| is leaving Picture-in-Picture mode.
6061
void OnLeavingPictureInPicture(bool should_pause_video);
6162

63+
// Internal method to set the states after the window was closed, whether via
64+
// the system or Chromium.
65+
void CloseInternal(bool should_pause_video);
66+
67+
// Creates a new window if the previous one was destroyed. It can happen
68+
// because of the system control of the window.
69+
void EnsureWindow();
70+
6271
std::unique_ptr<OverlayWindow> window_;
6372
std::unique_ptr<OverlaySurfaceEmbedder> embedder_;
6473
WebContentsImpl* const initiator_;

content/public/browser/picture_in_picture_window_controller.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,14 @@ class PictureInPictureWindowController {
3838
// Returns the size of the window in pixels.
3939
virtual gfx::Size Show() = 0;
4040

41+
// Called to notify the controller that the window was requested to be closed
42+
// by the user or the content.
4143
virtual void Close(bool should_pause_video) = 0;
44+
45+
// Called by the window implementation to notify the controller that the
46+
// window was requested to be closed and destroyed by the system.
47+
virtual void OnWindowDestroyed() = 0;
48+
4249
virtual void ClickCustomControl(const std::string& control_id) = 0;
4350
virtual void EmbedSurface(const viz::SurfaceId& surface_id,
4451
const gfx::Size& natural_size) = 0;

0 commit comments

Comments
 (0)