Enable Picture-in-Picture for extensions.
This CL enables Picture-in-Picture in extensions by implementing
EnterPictureInPicture and ExitPictureInPicture in ExtensionHostDelegate.
Change-Id: I77522e345b7fed7d4515e81204a635a5ed8ecb6c
Bug: 726619
Reviewed-on: https://chromium-review.googlesource.com/1145261
Commit-Queue: François Beaufort <[email protected]>
Reviewed-by: Sadrul Chowdhury <[email protected]>
Reviewed-by: Devlin <[email protected]>
Reviewed-by: Sergey Volk <[email protected]>
Cr-Commit-Position: refs/heads/master@{#586632}diff --git a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
index 889510d0..cac23b1 100644
--- a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
+++ b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
@@ -36,6 +36,7 @@
#include "components/prefs/pref_service.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/notification_service.h"
+#include "content/public/browser/picture_in_picture_window_controller.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
@@ -1115,5 +1116,48 @@
#endif
}
+// Verify video can enter and exit Picture-in_Picture when browser action icon
+// is clicked.
+IN_PROC_BROWSER_TEST_F(BrowserActionApiTest,
+ TestPictureInPictureOnBrowserActionIconClick) {
+ ASSERT_TRUE(StartEmbeddedTestServer());
+
+ ASSERT_TRUE(
+ RunExtensionTest("trigger_actions/browser_action_picture_in_picture"))
+ << message_;
+ const Extension* extension = GetSingleLoadedExtension();
+ ASSERT_TRUE(extension) << message_;
+
+ // Test that there is a browser action in the toolbar.
+ ASSERT_EQ(1, GetBrowserActionsBar()->NumberOfBrowserActions());
+
+ ExtensionAction* browser_action = GetBrowserAction(*extension);
+ EXPECT_TRUE(browser_action);
+
+ // Find the background page.
+ ProcessManager* process_manager =
+ extensions::ProcessManager::Get(browser()->profile());
+ content::WebContents* web_contents =
+ process_manager->GetBackgroundHostForExtension(extension->id())
+ ->web_contents();
+ ASSERT_TRUE(web_contents);
+ content::PictureInPictureWindowController* window_controller =
+ content::PictureInPictureWindowController::GetOrCreateForWebContents(
+ web_contents);
+ ASSERT_TRUE(window_controller->GetWindowForTesting());
+ EXPECT_FALSE(window_controller->GetWindowForTesting()->IsVisible());
+
+ // Click on the browser action icon to enter Picture-in-Picture.
+ ResultCatcher catcher;
+ GetBrowserActionsBar()->Press(0);
+ EXPECT_TRUE(catcher.GetNextResult());
+ EXPECT_TRUE(window_controller->GetWindowForTesting()->IsVisible());
+
+ // Click on the browser action icon to exit Picture-in-Picture.
+ GetBrowserActionsBar()->Press(0);
+ EXPECT_TRUE(catcher.GetNextResult());
+ EXPECT_FALSE(window_controller->GetWindowForTesting()->IsVisible());
+}
+
} // namespace
} // namespace extensions
diff --git a/chrome/browser/extensions/chrome_extension_host_delegate.cc b/chrome/browser/extensions/chrome_extension_host_delegate.cc
index 6736efb..13d09c9 100644
--- a/chrome/browser/extensions/chrome_extension_host_delegate.cc
+++ b/chrome/browser/extensions/chrome_extension_host_delegate.cc
@@ -14,6 +14,7 @@
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
#include "chrome/browser/ui/prefs/prefs_tab_helper.h"
#include "components/app_modal/javascript_dialog_manager.h"
#include "extensions/browser/extension_host.h"
@@ -102,4 +103,16 @@
return g_queue.Get().queue.get();
}
+gfx::Size ChromeExtensionHostDelegate::EnterPictureInPicture(
+ content::WebContents* web_contents,
+ const viz::SurfaceId& surface_id,
+ const gfx::Size& natural_size) {
+ return PictureInPictureWindowManager::GetInstance()->EnterPictureInPicture(
+ web_contents, surface_id, natural_size);
+}
+
+void ChromeExtensionHostDelegate::ExitPictureInPicture() {
+ PictureInPictureWindowManager::GetInstance()->ExitPictureInPicture();
+}
+
} // namespace extensions
diff --git a/chrome/browser/extensions/chrome_extension_host_delegate.h b/chrome/browser/extensions/chrome_extension_host_delegate.h
index 03d3727..3dd24a7f 100644
--- a/chrome/browser/extensions/chrome_extension_host_delegate.h
+++ b/chrome/browser/extensions/chrome_extension_host_delegate.h
@@ -33,6 +33,10 @@
content::MediaStreamType type,
const Extension* extension) override;
ExtensionHostQueue* GetExtensionHostQueue() const override;
+ gfx::Size EnterPictureInPicture(content::WebContents* web_contents,
+ const viz::SurfaceId& surface_id,
+ const gfx::Size& natural_size) override;
+ void ExitPictureInPicture() override;
};
} // namespace extensions
diff --git a/chrome/test/data/extensions/api_test/trigger_actions/browser_action_picture_in_picture/background.js b/chrome/test/data/extensions/api_test/trigger_actions/browser_action_picture_in_picture/background.js
new file mode 100644
index 0000000..6ed0f2f
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/trigger_actions/browser_action_picture_in_picture/background.js
@@ -0,0 +1,40 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+const video = document.createElement('video');
+
+chrome.test.getConfig(function(config) {
+ video.src = 'http://localhost:' + config.testServer.port +
+ '/media/bigbuck.webm';
+ video.load();
+ video.addEventListener('loadedmetadata', function() {
+ chrome.test.notifyPass();
+ })
+});
+
+// Toggle Picture-in-Picture when the user clicks on the browser action.
+chrome.browserAction.onClicked.addListener(function(tab) {
+ if (video !== document.pictureInPictureElement) {
+ enterPictureInPicture();
+ } else {
+ exitPictureInPicture();
+ }
+});
+
+function enterPictureInPicture() {
+ video.requestPictureInPicture()
+ .then(pipWindow => {
+ if (pipWindow.width === 0 || pipWindow.height === 0) {
+ return Promise.reject('Picture-in-Picture window size is not set.');
+ }
+ chrome.test.notifyPass();
+ })
+ .catch(error => { chrome.test.notifyFail('Error: ' + error); })
+}
+
+function exitPictureInPicture() {
+ document.exitPictureInPicture()
+ .then(() => { chrome.test.notifyPass(); })
+ .catch(error => { chrome.test.notifyFail('Error: ' + error); })
+}
diff --git a/chrome/test/data/extensions/api_test/trigger_actions/browser_action_picture_in_picture/manifest.json b/chrome/test/data/extensions/api_test/trigger_actions/browser_action_picture_in_picture/manifest.json
new file mode 100644
index 0000000..a1f17d1
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/trigger_actions/browser_action_picture_in_picture/manifest.json
@@ -0,0 +1,11 @@
+{
+ "name": "A browser action that toggles Picture-in-Picture",
+ "version": "1.0",
+ "manifest_version": 2,
+ "background": {
+ "scripts": ["background.js"]
+ },
+ "browser_action": {
+ "default_title": "Toggle Picture-in-Picture"
+ }
+}
diff --git a/chromecast/browser/extensions/cast_extension_host_delegate.cc b/chromecast/browser/extensions/cast_extension_host_delegate.cc
index d58d416c..1c1f6ee 100644
--- a/chromecast/browser/extensions/cast_extension_host_delegate.cc
+++ b/chromecast/browser/extensions/cast_extension_host_delegate.cc
@@ -62,4 +62,16 @@
return queue.get();
}
+gfx::Size CastExtensionHostDelegate::EnterPictureInPicture(
+ content::WebContents* web_contents,
+ const viz::SurfaceId& surface_id,
+ const gfx::Size& natural_size) {
+ NOTREACHED();
+ return gfx::Size();
+}
+
+void CastExtensionHostDelegate::ExitPictureInPicture() {
+ NOTREACHED();
+}
+
} // namespace extensions
diff --git a/chromecast/browser/extensions/cast_extension_host_delegate.h b/chromecast/browser/extensions/cast_extension_host_delegate.h
index c471edf..d548eed 100644
--- a/chromecast/browser/extensions/cast_extension_host_delegate.h
+++ b/chromecast/browser/extensions/cast_extension_host_delegate.h
@@ -34,6 +34,10 @@
content::MediaStreamType type,
const Extension* extension) override;
ExtensionHostQueue* GetExtensionHostQueue() const override;
+ gfx::Size EnterPictureInPicture(content::WebContents* web_contents,
+ const viz::SurfaceId& surface_id,
+ const gfx::Size& natural_size) override;
+ void ExitPictureInPicture() override;
private:
DISALLOW_COPY_AND_ASSIGN(CastExtensionHostDelegate);
diff --git a/extensions/browser/extension_host.cc b/extensions/browser/extension_host.cc
index 999df3d..56bf6a8 100644
--- a/extensions/browser/extension_host.cc
+++ b/extensions/browser/extension_host.cc
@@ -455,6 +455,18 @@
return view_type == extensions::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE;
}
+gfx::Size ExtensionHost::EnterPictureInPicture(const viz::SurfaceId& surface_id,
+ const gfx::Size& natural_size) {
+ // TODO(crbug.com/870609): Increment the keepalive count of the background
+ // page to avoid it shutting down while playing video.
+ return delegate_->EnterPictureInPicture(web_contents(), surface_id,
+ natural_size);
+}
+
+void ExtensionHost::ExitPictureInPicture() {
+ delegate_->ExitPictureInPicture();
+}
+
void ExtensionHost::RecordStopLoadingUMA() {
CHECK(load_start_.get());
if (extension_host_type_ == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
diff --git a/extensions/browser/extension_host.h b/extensions/browser/extension_host.h
index bd05034..db4489f 100644
--- a/extensions/browser/extension_host.h
+++ b/extensions/browser/extension_host.h
@@ -127,6 +127,9 @@
const GURL& security_origin,
content::MediaStreamType type) override;
bool IsNeverVisible(content::WebContents* web_contents) override;
+ gfx::Size EnterPictureInPicture(const viz::SurfaceId& surface_id,
+ const gfx::Size& natural_size) override;
+ void ExitPictureInPicture() override;
// ExtensionRegistryObserver:
void OnExtensionReady(content::BrowserContext* browser_context,
diff --git a/extensions/browser/extension_host_delegate.h b/extensions/browser/extension_host_delegate.h
index 548ea387..e648dc2 100644
--- a/extensions/browser/extension_host_delegate.h
+++ b/extensions/browser/extension_host_delegate.h
@@ -18,6 +18,11 @@
namespace gfx {
class Rect;
+class Size;
+} // namespace gfx
+
+namespace viz {
+class SurfaceId;
}
namespace extensions {
@@ -71,6 +76,17 @@
// Returns the ExtensionHostQueue implementation to use for creating
// ExtensionHost renderers.
virtual ExtensionHostQueue* GetExtensionHostQueue() const = 0;
+
+ // Notifies the Picture-in-Picture controller that there is a new player
+ // entering Picture-in-Picture.
+ // Returns the size of the Picture-in-Picture window.
+ virtual gfx::Size EnterPictureInPicture(content::WebContents* web_contents,
+ const viz::SurfaceId& surface_id,
+ const gfx::Size& natural_size) = 0;
+
+ // Updates the Picture-in-Picture controller with a signal that
+ // Picture-in-Picture mode has ended.
+ virtual void ExitPictureInPicture() = 0;
};
} // namespace extensions
diff --git a/extensions/shell/browser/shell_extension_host_delegate.cc b/extensions/shell/browser/shell_extension_host_delegate.cc
index ad7e362..686c912 100644
--- a/extensions/shell/browser/shell_extension_host_delegate.cc
+++ b/extensions/shell/browser/shell_extension_host_delegate.cc
@@ -71,4 +71,16 @@
return g_queue.Pointer();
}
+gfx::Size ShellExtensionHostDelegate::EnterPictureInPicture(
+ content::WebContents* web_contents,
+ const viz::SurfaceId& surface_id,
+ const gfx::Size& natural_size) {
+ NOTREACHED();
+ return gfx::Size();
+}
+
+void ShellExtensionHostDelegate::ExitPictureInPicture() {
+ NOTREACHED();
+}
+
} // namespace extensions
diff --git a/extensions/shell/browser/shell_extension_host_delegate.h b/extensions/shell/browser/shell_extension_host_delegate.h
index 832c9dd..1fe1ef66 100644
--- a/extensions/shell/browser/shell_extension_host_delegate.h
+++ b/extensions/shell/browser/shell_extension_host_delegate.h
@@ -34,6 +34,10 @@
content::MediaStreamType type,
const Extension* extension) override;
ExtensionHostQueue* GetExtensionHostQueue() const override;
+ gfx::Size EnterPictureInPicture(content::WebContents* web_contents,
+ const viz::SurfaceId& surface_id,
+ const gfx::Size& natural_size) override;
+ void ExitPictureInPicture() override;
private:
DISALLOW_COPY_AND_ASSIGN(ShellExtensionHostDelegate);
diff --git a/testing/buildbot/filters/chromeos.mash.fyi.browser_tests.filter b/testing/buildbot/filters/chromeos.mash.fyi.browser_tests.filter
index 31f1d06..c182a61 100644
--- a/testing/buildbot/filters/chromeos.mash.fyi.browser_tests.filter
+++ b/testing/buildbot/filters/chromeos.mash.fyi.browser_tests.filter
@@ -278,6 +278,7 @@
# https://crbug.com/827327
-PictureInPictureWindowControllerBrowserTest.*
-ControlPictureInPictureWindowControllerBrowserTest.*
+-BrowserActionApiTest.TestPictureInPictureOnBrowserActionIconClick
# These started failing with the switch to ws2.
# https:://crbug.com/855767
diff --git a/ui/views/test/widget_test.cc b/ui/views/test/widget_test.cc
index d94091c..91f1691 100644
--- a/ui/views/test/widget_test.cc
+++ b/ui/views/test/widget_test.cc
@@ -99,6 +99,9 @@
TestDesktopWidgetDelegate::TestDesktopWidgetDelegate() : widget_(new Widget) {
}
+TestDesktopWidgetDelegate::TestDesktopWidgetDelegate(Widget* widget)
+ : widget_(widget) {}
+
TestDesktopWidgetDelegate::~TestDesktopWidgetDelegate() {
if (widget_)
widget_->CloseNow();
diff --git a/ui/views/test/widget_test.h b/ui/views/test/widget_test.h
index 80c690a2..2e871d9 100644
--- a/ui/views/test/widget_test.h
+++ b/ui/views/test/widget_test.h
@@ -118,6 +118,7 @@
class TestDesktopWidgetDelegate : public WidgetDelegate {
public:
TestDesktopWidgetDelegate();
+ TestDesktopWidgetDelegate(Widget* widget);
~TestDesktopWidgetDelegate() override;
// Initialize the Widget, adding some meaningful default InitParams.
diff --git a/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc b/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
index 09612ee..01a429a 100644
--- a/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
+++ b/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
@@ -321,8 +321,12 @@
desktop_window_tree_host_ = NULL;
content_window_ = NULL;
+ // |OnNativeWidgetDestroyed| may delete |this| if the object does not own
+ // itself.
+ bool should_delete_this =
+ (ownership_ == Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET);
native_widget_delegate_->OnNativeWidgetDestroyed();
- if (ownership_ == Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET)
+ if (should_delete_this)
delete this;
}
diff --git a/ui/views/widget/widget_unittest.cc b/ui/views/widget/widget_unittest.cc
index 1bd99e8..24c23336 100644
--- a/ui/views/widget/widget_unittest.cc
+++ b/ui/views/widget/widget_unittest.cc
@@ -1944,6 +1944,29 @@
EXPECT_TRUE(observer.widget_closed());
}
+// Widget used to destroy itself when OnNativeWidgetDestroyed is called.
+class TestNativeWidgetDestroyedWidget : public Widget {
+ public:
+ // Overridden from NativeWidgetDelegate:
+ void OnNativeWidgetDestroyed() override;
+};
+
+void TestNativeWidgetDestroyedWidget::OnNativeWidgetDestroyed() {
+ Widget::OnNativeWidgetDestroyed();
+ delete this;
+}
+
+// Verifies that widget destroyed itself in OnNativeWidgetDestroyed does not
+// crash in ASan.
+TEST_F(WidgetTest, WidgetDestroyedItselfDoesNotCrash) {
+ TestDesktopWidgetDelegate delegate(new TestNativeWidgetDestroyedWidget);
+ Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
+ params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+ delegate.InitWidget(params);
+ delegate.GetWidget()->Show();
+ delegate.GetWidget()->CloseNow();
+}
+
// Verifies WindowClosing() is invoked correctly on the delegate when a Widget
// is closed.
TEST_F(WidgetTest, SingleWindowClosing) {
