From 9ccd6aa0dd9ae9de1cb013a1ee328690f1cc14d9 Mon Sep 17 00:00:00 2001 From: Heilig Benedek Date: Thu, 22 Aug 2019 12:17:50 +0200 Subject: [PATCH] feat: enable picture-in-picture mode for video tags (#17686) * feat: enable picture in picture mode for video tags * test: add test to verify picture in picture support * lint: fix indent * fix: clean up after rebase * test: update test with 16:9 test video * fix: .paches after rebase --- buildflags/BUILD.gn | 1 + buildflags/buildflags.gni | 2 + chromium_src/BUILD.gn | 28 +++++ electron_strings.grdp | 51 ++++++++ patches/chromium/.patches | 1 + patches/chromium/picture-in-picture.patch | 110 ++++++++++++++++++ shell/browser/atom_browser_client.cc | 11 ++ shell/browser/atom_browser_client.h | 5 + shell/browser/common_web_contents_delegate.cc | 23 ++++ shell/browser/common_web_contents_delegate.h | 5 + shell/browser/feature_list.cc | 5 + shell/common/api/features.cc | 5 + spec/api-web-contents-spec.js | 19 +++ spec/fixtures/api/picture-in-picture.html | 51 ++++++++ typings/internal-ambient.d.ts | 1 + 15 files changed, 318 insertions(+) create mode 100644 patches/chromium/picture-in-picture.patch create mode 100644 spec/fixtures/api/picture-in-picture.html diff --git a/buildflags/BUILD.gn b/buildflags/BUILD.gn index 359a060941..03f1a508a0 100644 --- a/buildflags/BUILD.gn +++ b/buildflags/BUILD.gn @@ -18,6 +18,7 @@ buildflag_header("buildflags") { "ENABLE_TTS=$enable_tts", "ENABLE_COLOR_CHOOSER=$enable_color_chooser", "ENABLE_ELECTRON_EXTENSIONS=$enable_electron_extensions", + "ENABLE_PICTURE_IN_PICTURE=$enable_picture_in_picture", "OVERRIDE_LOCATION_PROVIDER=$enable_fake_location_provider", ] } diff --git a/buildflags/buildflags.gni b/buildflags/buildflags.gni index 86c25bdca4..3f7f3f2a62 100644 --- a/buildflags/buildflags.gni +++ b/buildflags/buildflags.gni @@ -18,6 +18,8 @@ declare_args() { enable_color_chooser = true + enable_picture_in_picture = true + # Provide a fake location provider for mocking # the geolocation responses. Disable it if you # need to test with chromium's location provider. diff --git a/chromium_src/BUILD.gn b/chromium_src/BUILD.gn index 7e371d085e..67c1609cb7 100644 --- a/chromium_src/BUILD.gn +++ b/chromium_src/BUILD.gn @@ -184,4 +184,32 @@ static_library("chrome") { ] } } + + if (enable_picture_in_picture) { + sources += [ + "//chrome/browser/picture_in_picture/picture_in_picture_window_manager.cc", + "//chrome/browser/picture_in_picture/picture_in_picture_window_manager.h", + "//chrome/browser/ui/views/overlay/back_to_tab_image_button.cc", + "//chrome/browser/ui/views/overlay/back_to_tab_image_button.h", + "//chrome/browser/ui/views/overlay/close_image_button.cc", + "//chrome/browser/ui/views/overlay/close_image_button.h", + "//chrome/browser/ui/views/overlay/mute_image_button.cc", + "//chrome/browser/ui/views/overlay/mute_image_button.h", + "//chrome/browser/ui/views/overlay/overlay_window_views.cc", + "//chrome/browser/ui/views/overlay/overlay_window_views.h", + "//chrome/browser/ui/views/overlay/playback_image_button.cc", + "//chrome/browser/ui/views/overlay/playback_image_button.h", + "//chrome/browser/ui/views/overlay/resize_handle_button.cc", + "//chrome/browser/ui/views/overlay/resize_handle_button.h", + "//chrome/browser/ui/views/overlay/skip_ad_label_button.cc", + "//chrome/browser/ui/views/overlay/skip_ad_label_button.h", + "//chrome/browser/ui/views/overlay/track_image_button.cc", + "//chrome/browser/ui/views/overlay/track_image_button.h", + ] + + deps += [ + "//chrome/app/vector_icons", + "//components/vector_icons:vector_icons", + ] + } } diff --git a/electron_strings.grdp b/electron_strings.grdp index 78324e76f5..4ea78421dd 100644 --- a/electron_strings.grdp +++ b/electron_strings.grdp @@ -18,4 +18,55 @@ {SCREEN_INDEX, plural, =1{Screen #} other{Screen #}} + + + + + Picture in Picture + + + + + Picture in picture + + + + Pause + + + Play + + + Play from the beginning + + + Back to video player + + + Mute + + + Unmute + + + Skip Ad + + + Close + + + Resize + + + Toggle video to play or pause + + + Toggle mute + + + Next track + + + Previous track + diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 06dbe4836b..d8d37f30a8 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -74,3 +74,4 @@ build_win_disable_zc_twophase.patch disable_color_correct_rendering.patch add_contentgpuclient_precreatemessageloop_callback.patch fix_vc_incompatible_inline_calls.patch +picture-in-picture.patch diff --git a/patches/chromium/picture-in-picture.patch b/patches/chromium/picture-in-picture.patch new file mode 100644 index 0000000000..41950136de --- /dev/null +++ b/patches/chromium/picture-in-picture.patch @@ -0,0 +1,110 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Heilig Benedek +Date: Sat, 10 Aug 2019 00:41:50 +0200 +Subject: feat: enable picture in picture mode for video players + + +diff --git a/chrome/browser/ui/views/overlay/back_to_tab_image_button.cc b/chrome/browser/ui/views/overlay/back_to_tab_image_button.cc +index 8e4deafa1746eeb48802a0503fefb37bedb33d04..127c62efd2327e1f3f09e9b93a0b8344e2714f80 100644 +--- a/chrome/browser/ui/views/overlay/back_to_tab_image_button.cc ++++ b/chrome/browser/ui/views/overlay/back_to_tab_image_button.cc +@@ -4,7 +4,7 @@ + + #include "chrome/browser/ui/views/overlay/back_to_tab_image_button.h" + +-#include "chrome/grit/generated_resources.h" ++#include "electron/grit/electron_resources.h" + #include "third_party/skia/include/core/SkColor.h" + #include "ui/base/l10n/l10n_util.h" + #include "ui/gfx/color_palette.h" +diff --git a/chrome/browser/ui/views/overlay/close_image_button.cc b/chrome/browser/ui/views/overlay/close_image_button.cc +index 0aca25164dcad26cc000e289d6eb9010e336e6fc..70114b5aa865b96d3ace898d1faf515b9098abd9 100644 +--- a/chrome/browser/ui/views/overlay/close_image_button.cc ++++ b/chrome/browser/ui/views/overlay/close_image_button.cc +@@ -4,7 +4,7 @@ + + #include "chrome/browser/ui/views/overlay/close_image_button.h" + +-#include "chrome/grit/generated_resources.h" ++#include "electron/grit/electron_resources.h" + #include "third_party/skia/include/core/SkColor.h" + #include "ui/base/l10n/l10n_util.h" + #include "ui/gfx/color_palette.h" +diff --git a/chrome/browser/ui/views/overlay/mute_image_button.cc b/chrome/browser/ui/views/overlay/mute_image_button.cc +index 8c88ef08dd5165c0429dd90e8a76b711ac15a4df..ebdb06a6391b8108fa51796a4ad5f3a8ca489b60 100644 +--- a/chrome/browser/ui/views/overlay/mute_image_button.cc ++++ b/chrome/browser/ui/views/overlay/mute_image_button.cc +@@ -5,7 +5,7 @@ + #include "chrome/browser/ui/views/overlay/mute_image_button.h" + + #include "chrome/app/vector_icons/vector_icons.h" +-#include "chrome/grit/generated_resources.h" ++#include "electron/grit/electron_resources.h" + #include "components/vector_icons/vector_icons.h" + #include "third_party/skia/include/core/SkColor.h" + #include "ui/base/l10n/l10n_util.h" +diff --git a/chrome/browser/ui/views/overlay/overlay_window_views.cc b/chrome/browser/ui/views/overlay/overlay_window_views.cc +index 780863193ca12ec1295752969dfc47ac06a9ae64..e2947b893cfcdb1beaa27beac80a1885ed011ce4 100644 +--- a/chrome/browser/ui/views/overlay/overlay_window_views.cc ++++ b/chrome/browser/ui/views/overlay/overlay_window_views.cc +@@ -20,7 +20,7 @@ + #include "chrome/browser/ui/views/overlay/resize_handle_button.h" + #include "chrome/browser/ui/views/overlay/skip_ad_label_button.h" + #include "chrome/browser/ui/views/overlay/track_image_button.h" +-#include "chrome/grit/generated_resources.h" ++#include "electron/grit/electron_resources.h" + #include "components/vector_icons/vector_icons.h" + #include "content/public/browser/picture_in_picture_window_controller.h" + #include "content/public/browser/web_contents.h" +diff --git a/chrome/browser/ui/views/overlay/playback_image_button.cc b/chrome/browser/ui/views/overlay/playback_image_button.cc +index d9e5174ed622fb030bc37d32fbb40b132d7c4c23..1bf19c344721e74bb29c11a4c5c762a75e5cd821 100644 +--- a/chrome/browser/ui/views/overlay/playback_image_button.cc ++++ b/chrome/browser/ui/views/overlay/playback_image_button.cc +@@ -5,7 +5,7 @@ + #include "chrome/browser/ui/views/overlay/playback_image_button.h" + + #include "chrome/app/vector_icons/vector_icons.h" +-#include "chrome/grit/generated_resources.h" ++#include "electron/grit/electron_resources.h" + #include "components/vector_icons/vector_icons.h" + #include "third_party/skia/include/core/SkColor.h" + #include "ui/base/l10n/l10n_util.h" +diff --git a/chrome/browser/ui/views/overlay/resize_handle_button.cc b/chrome/browser/ui/views/overlay/resize_handle_button.cc +index ee6b3612d7bdda591e05e5af338a80167ce6cd53..af093f14f1ef49c6de7228b296c32532203ca568 100644 +--- a/chrome/browser/ui/views/overlay/resize_handle_button.cc ++++ b/chrome/browser/ui/views/overlay/resize_handle_button.cc +@@ -5,7 +5,7 @@ + #include "chrome/browser/ui/views/overlay/resize_handle_button.h" + + #include "chrome/app/vector_icons/vector_icons.h" +-#include "chrome/grit/generated_resources.h" ++#include "electron/grit/electron_resources.h" + #include "third_party/skia/include/core/SkColor.h" + #include "ui/base/hit_test.h" + #include "ui/base/l10n/l10n_util.h" +diff --git a/chrome/browser/ui/views/overlay/skip_ad_label_button.cc b/chrome/browser/ui/views/overlay/skip_ad_label_button.cc +index da780c96bb757d7382df5f419e2c0fd644ac72b0..ae520bcf73cf6c39ca428c03975746e20b23c3ee 100644 +--- a/chrome/browser/ui/views/overlay/skip_ad_label_button.cc ++++ b/chrome/browser/ui/views/overlay/skip_ad_label_button.cc +@@ -4,7 +4,7 @@ + + #include "chrome/browser/ui/views/overlay/skip_ad_label_button.h" + +-#include "chrome/grit/generated_resources.h" ++#include "electron/grit/electron_resources.h" + #include "ui/base/l10n/l10n_util.h" + #include "ui/gfx/color_palette.h" + #include "ui/views/background.h" +diff --git a/chrome/browser/ui/views/overlay/track_image_button.cc b/chrome/browser/ui/views/overlay/track_image_button.cc +index 8f42277409a216f81d21723eb03045ac54525b0e..f7a15bfde9a43c15b18e8afbd60a0b19960f2c93 100644 +--- a/chrome/browser/ui/views/overlay/track_image_button.cc ++++ b/chrome/browser/ui/views/overlay/track_image_button.cc +@@ -5,7 +5,7 @@ + #include "chrome/browser/ui/views/overlay/track_image_button.h" + + #include "chrome/app/vector_icons/vector_icons.h" +-#include "chrome/grit/generated_resources.h" ++#include "electron/grit/electron_resources.h" + #include "components/vector_icons/vector_icons.h" + #include "third_party/skia/include/core/SkColor.h" + #include "ui/base/l10n/l10n_util.h" diff --git a/shell/browser/atom_browser_client.cc b/shell/browser/atom_browser_client.cc index 75ff9bb302..7b0c19d7b0 100644 --- a/shell/browser/atom_browser_client.cc +++ b/shell/browser/atom_browser_client.cc @@ -402,6 +402,9 @@ void AtomBrowserClient::OverrideWebkitPrefs(content::RenderViewHost* host, prefs->default_minimum_page_scale_factor = 1.f; prefs->default_maximum_page_scale_factor = 1.f; prefs->navigate_on_drag_drop = false; +#if !BUILDFLAG(ENABLE_PICTURE_IN_PICTURE) + prefs->picture_in_picture_enabled = false; +#endif ui::NativeTheme* native_theme = ui::NativeTheme::GetInstanceForNativeUi(); prefs->preferred_color_scheme = native_theme->ShouldUseDarkColors() @@ -684,6 +687,14 @@ bool AtomBrowserClient::CanCreateWindow( return false; } +#if BUILDFLAG(ENABLE_PICTURE_IN_PICTURE) +std::unique_ptr +AtomBrowserClient::CreateWindowForPictureInPicture( + content::PictureInPictureWindowController* controller) { + return content::OverlayWindow::Create(controller); +} +#endif + void AtomBrowserClient::GetAdditionalAllowedSchemesForFileSystem( std::vector* additional_schemes) { auto schemes_list = api::GetStandardSchemes(); diff --git a/shell/browser/atom_browser_client.h b/shell/browser/atom_browser_client.h index de150acf2c..d0aeac22bf 100644 --- a/shell/browser/atom_browser_client.h +++ b/shell/browser/atom_browser_client.h @@ -14,6 +14,7 @@ #include "base/synchronization/lock.h" #include "content/public/browser/content_browser_client.h" #include "content/public/browser/render_process_host_observer.h" +#include "electron/buildflags/buildflags.h" #include "net/ssl/client_cert_identity.h" namespace content { @@ -127,6 +128,10 @@ class AtomBrowserClient : public content::ContentBrowserClient, bool user_gesture, bool opener_suppressed, bool* no_javascript_access) override; +#if BUILDFLAG(ENABLE_PICTURE_IN_PICTURE) + std::unique_ptr CreateWindowForPictureInPicture( + content::PictureInPictureWindowController* controller) override; +#endif void GetAdditionalAllowedSchemesForFileSystem( std::vector* additional_schemes) override; void GetAdditionalWebUISchemes( diff --git a/shell/browser/common_web_contents_delegate.cc b/shell/browser/common_web_contents_delegate.cc index 21e9e5cdd2..1b104f4a92 100644 --- a/shell/browser/common_web_contents_delegate.cc +++ b/shell/browser/common_web_contents_delegate.cc @@ -55,6 +55,10 @@ #include "shell/browser/printing/print_preview_message_handler.h" #endif +#if BUILDFLAG(ENABLE_PICTURE_IN_PICTURE) +#include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h" +#endif + using content::BrowserThread; namespace electron { @@ -636,4 +640,23 @@ void CommonWebContentsDelegate::SetHtmlApiFullscreen(bool enter_fullscreen) { native_fullscreen_ = false; } +content::PictureInPictureResult +CommonWebContentsDelegate::EnterPictureInPicture( + content::WebContents* web_contents, + const viz::SurfaceId& surface_id, + const gfx::Size& natural_size) { +#if BUILDFLAG(ENABLE_PICTURE_IN_PICTURE) + return PictureInPictureWindowManager::GetInstance()->EnterPictureInPicture( + web_contents, surface_id, natural_size); +#else + return content::PictureInPictureResult::kNotSupported; +#endif +} + +void CommonWebContentsDelegate::ExitPictureInPicture() { +#if BUILDFLAG(ENABLE_PICTURE_IN_PICTURE) + PictureInPictureWindowManager::GetInstance()->ExitPictureInPicture(); +#endif +} + } // namespace electron diff --git a/shell/browser/common_web_contents_delegate.h b/shell/browser/common_web_contents_delegate.h index a583f8f84c..756fbfc2b8 100644 --- a/shell/browser/common_web_contents_delegate.h +++ b/shell/browser/common_web_contents_delegate.h @@ -102,6 +102,11 @@ class CommonWebContentsDelegate : public content::WebContentsDelegate, bool HandleKeyboardEvent( content::WebContents* source, const content::NativeWebKeyboardEvent& event) override; + content::PictureInPictureResult EnterPictureInPicture( + content::WebContents* web_contents, + const viz::SurfaceId&, + const gfx::Size& natural_size) override; + void ExitPictureInPicture() override; // InspectableWebContentsDelegate: void DevToolsSaveToFile(const std::string& url, diff --git a/shell/browser/feature_list.cc b/shell/browser/feature_list.cc index fa8f3f581f..b19596b7b4 100644 --- a/shell/browser/feature_list.cc +++ b/shell/browser/feature_list.cc @@ -10,6 +10,8 @@ #include "base/command_line.h" #include "base/feature_list.h" #include "content/public/common/content_features.h" +#include "electron/buildflags/buildflags.h" +#include "media/base/media_switches.h" namespace electron { @@ -25,6 +27,9 @@ void InitializeFeatureList() { // when node integration is enabled. disable_features += std::string(",") + features::kSpareRendererForSitePerProcess.name; +#if !BUILDFLAG(ENABLE_PICTURE_IN_PICTURE) + disable_features += std::string(",") + media::kPictureInPicture.name; +#endif base::FeatureList::InitializeInstance(enable_features, disable_features); } diff --git a/shell/common/api/features.cc b/shell/common/api/features.cc index f905183a2f..bf3a82109d 100644 --- a/shell/common/api/features.cc +++ b/shell/common/api/features.cc @@ -45,6 +45,10 @@ bool IsExtensionsEnabled() { return BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS); } +bool IsPictureInPictureEnabled() { + return BUILDFLAG(ENABLE_PICTURE_IN_PICTURE); +} + bool IsComponentBuild() { #if defined(COMPONENT_BUILD) return true; @@ -67,6 +71,7 @@ void Initialize(v8::Local exports, dict.SetMethod("isViewApiEnabled", &IsViewApiEnabled); dict.SetMethod("isTtsEnabled", &IsTtsEnabled); dict.SetMethod("isPrintingEnabled", &IsPrintingEnabled); + dict.SetMethod("isPictureInPictureEnabled", &IsPictureInPictureEnabled); dict.SetMethod("isComponentBuild", &IsComponentBuild); dict.SetMethod("isExtensionsEnabled", &IsExtensionsEnabled); } diff --git a/spec/api-web-contents-spec.js b/spec/api-web-contents-spec.js index d709463f1c..7c556d644c 100644 --- a/spec/api-web-contents-spec.js +++ b/spec/api-web-contents-spec.js @@ -1273,4 +1273,23 @@ describe('webContents module', () => { expect(data).to.be.an.instanceof(Buffer).that.is.not.empty() }) }) + + describe('PictureInPicture video', () => { + it('works as expected', (done) => { + w.destroy() + w = new BrowserWindow({ + show: false, + webPreferences: { + sandbox: true + } + }) + w.webContents.once('did-finish-load', async () => { + const result = await w.webContents.executeJavaScript( + `runTest(${features.isPictureInPictureEnabled()})`, true) + expect(result).to.be.true() + done() + }) + w.loadFile(path.join(fixtures, 'api', 'picture-in-picture.html')) + }) + }) }) diff --git a/spec/fixtures/api/picture-in-picture.html b/spec/fixtures/api/picture-in-picture.html new file mode 100644 index 0000000000..36a67070c8 --- /dev/null +++ b/spec/fixtures/api/picture-in-picture.html @@ -0,0 +1,51 @@ + + + + + + + + + + diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts index 6cd62a4a4f..f687c8b332 100644 --- a/typings/internal-ambient.d.ts +++ b/typings/internal-ambient.d.ts @@ -10,6 +10,7 @@ declare namespace NodeJS { isViewApiEnabled(): boolean; isTtsEnabled(): boolean; isPrintingEnabled(): boolean; + isPictureInPictureEnabled(): boolean; isExtensionsEnabled(): boolean; isComponentBuild(): boolean; }