WebKit Bugzilla
Attachment 349216 Details for
Bug 189433
: Touch Bar displays an active PIP button for audio elements (and it doesn't do anything)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-189433-20180907164040.patch (text/plain), 33.69 KB, created by
Matt Rajca
on 2018-09-07 16:40:41 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Matt Rajca
Created:
2018-09-07 16:40:41 PDT
Size:
33.69 KB
patch
obsolete
>Subversion Revision: 235808 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index ccee452ca98f4b068c0edee91062d298a1e618c4..00563cc0bdf4b5554fb0f879e3143da5d20293f6 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,37 @@ >+2018-09-07 Matt Rajca <mrajca@apple.com> >+ >+ Touch Bar displays an active PIP button for audio elements (and it doesn't do anything) >+ https://bugs.webkit.org/show_bug.cgi?id=189433 >+ <rdar://problem/44186498> Touch Bar displays an active PIP button for audio elements (and it doesn't do anything) >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ When playing an audio element, the media Touch Bar displays an active PIP button even though only >+ videos are PIP-able. Pressing it does not do anything. The issue is canTogglePictureInPicture is set >+ to YES unconditionally on the WebPlaybackControlsManager. It is then only updated based on whether or >+ not external playback is enabled. >+ >+ This patch extends that logic such that the picture-in-picture Touch Bar button will be disabled for >+ audio elements. Since PlaybackSessionModelMediaElement today does not know whether we're dealing >+ with an audio or video element, a new isPictureInPictureSupported flag has been added (as well as >+ the plumbing necessary to get the state over from the web process). >+ >+ An API test has been added that checks the value of the canTogglePictureInPicture and ensures it >+ is NO when audio elements are playing. To expose it to tests, a _canTogglePictureInPictureForTesting >+ property has been added to the WKTesting category. >+ >+ * platform/cocoa/PlaybackSessionModel.h: >+ (WebCore::PlaybackSessionModelClient::isPictureInPictureSupportedChanged): >+ * platform/cocoa/PlaybackSessionModelMediaElement.h: >+ * platform/cocoa/PlaybackSessionModelMediaElement.mm: >+ (WebCore::PlaybackSessionModelMediaElement::setMediaElement): >+ (WebCore::PlaybackSessionModelMediaElement::isPictureInPictureSupported const): >+ * platform/mac/PlaybackSessionInterfaceMac.h: >+ * platform/mac/PlaybackSessionInterfaceMac.mm: >+ (WebCore::PlaybackSessionInterfaceMac::isPictureInPictureSupportedChanged): >+ (WebCore::PlaybackSessionInterfaceMac::externalPlaybackChanged): >+ (WebCore::PlaybackSessionInterfaceMac::updatePlaybackControlsManagerCanTogglePictureInPicture): >+ > 2018-09-07 Rob Buis <rbuis@igalia.com> > > XMLHttpRequest: open() does not throw a SYNTAX_ERR exception if method is empty or url cannot be resolved >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 77cce9454dfedeb8ffede17a17161e0f1934a6b9..d4287cd40df4d7b58c87e6c932724570d90fc642 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,44 @@ >+2018-09-07 Matt Rajca <mrajca@apple.com> >+ >+ Touch Bar displays an active PIP button for audio elements (and it doesn't do anything) >+ https://bugs.webkit.org/show_bug.cgi?id=189433 >+ <rdar://problem/44186498> Touch Bar displays an active PIP button for audio elements (and it doesn't do anything) >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ When playing an audio element, the media Touch Bar displays an active PIP button even though only >+ videos are PIP-able. Pressing it does not do anything. The issue is canTogglePictureInPicture is set >+ to YES unconditionally on the WebPlaybackControlsManager. It is then only updated based on whether or >+ not external playback is enabled. >+ >+ This patch extends that logic such that the picture-in-picture Touch Bar button will be disabled for >+ audio elements. Since PlaybackSessionModelMediaElement today does not know whether we're dealing >+ with an audio or video element, a new isPictureInPictureSupported flag has been added (as well as >+ the plumbing necessary to get the state over from the web process). >+ >+ An API test has been added that checks the value of the canTogglePictureInPicture and ensures it >+ is NO when audio elements are playing. To expose it to tests, a _canTogglePictureInPictureForTesting >+ property has been added to the WKTesting category. >+ >+ Source/WebKit: >+ * UIProcess/API/Cocoa/WKWebView.mm: >+ (-[WKWebView _canTogglePictureInPictureForTesting]): >+ * UIProcess/API/Cocoa/WKWebViewPrivate.h: >+ * UIProcess/Cocoa/PlaybackSessionManagerProxy.h: >+ * UIProcess/Cocoa/PlaybackSessionManagerProxy.messages.in: >+ * UIProcess/Cocoa/PlaybackSessionManagerProxy.mm: >+ (WebKit::PlaybackSessionModelContext::pictureInPictureSupportedChanged): >+ (WebKit::PlaybackSessionManagerProxy::pictureInPictureSupportedChanged): >+ * UIProcess/Cocoa/WebViewImpl.h: >+ * UIProcess/Cocoa/WebViewImpl.mm: >+ (WebKit::WebViewImpl::updateMediaTouchBar): >+ (WebKit::WebViewImpl::canTogglePictureInPictureForTesting): >+ * UIProcess/ios/fullscreen/WKFullScreenViewController.mm: >+ * WebProcess/cocoa/PlaybackSessionManager.h: >+ * WebProcess/cocoa/PlaybackSessionManager.mm: >+ (WebKit::PlaybackSessionInterfaceContext::isPictureInPictureSupportedChanged): >+ (WebKit::PlaybackSessionManager::isPictureInPictureSupportedChanged): >+ > 2018-09-07 Brent Fulgham <bfulgham@apple.com> > > Allow WebContent access to AVCSupported IOKit property in sandbox >diff --git a/Source/WebCore/platform/cocoa/PlaybackSessionModel.h b/Source/WebCore/platform/cocoa/PlaybackSessionModel.h >index eff018e03340cd660dd2ab1de56cb1a7f1b911e5..c43f732fd643dd9fdbad3ca57fa917a8fa25a437 100644 >--- a/Source/WebCore/platform/cocoa/PlaybackSessionModel.h >+++ b/Source/WebCore/platform/cocoa/PlaybackSessionModel.h >@@ -84,6 +84,7 @@ public: > virtual bool wirelessVideoPlaybackDisabled() const = 0; > virtual bool isMuted() const = 0; > virtual double volume() const = 0; >+ virtual bool isPictureInPictureSupported() const = 0; > virtual bool isPictureInPictureActive() const = 0; > }; > >@@ -105,6 +106,7 @@ public: > virtual void wirelessVideoPlaybackDisabledChanged(bool) { } > virtual void mutedChanged(bool) { } > virtual void volumeChanged(double) { } >+ virtual void isPictureInPictureSupportedChanged(bool) { } > virtual void pictureInPictureActiveChanged(bool) { } > virtual void ensureControlsManager() { } > }; >diff --git a/Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.h b/Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.h >index f7420b34bb9f9ea6540650acb3cb22372e68482f..9eefd10f2a3dceead1194ec94853139484723791 100644 >--- a/Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.h >+++ b/Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.h >@@ -95,6 +95,7 @@ public: > bool wirelessVideoPlaybackDisabled() const final; > bool isMuted() const final; > double volume() const final; >+ bool isPictureInPictureSupported() const final; > bool isPictureInPictureActive() const final; > > protected: >diff --git a/Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm b/Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm >index a190d68d7eefa9ab4e027be8e4a8cfa65076d778..c01792175dd33d29fa384a5e972550524739d51a 100644 >--- a/Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm >+++ b/Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm >@@ -101,6 +101,9 @@ void PlaybackSessionModelMediaElement::setMediaElement(HTMLMediaElement* mediaEl > } > > updateForEventName(eventNameAll()); >+ >+ for (auto client : m_clients) >+ client->isPictureInPictureSupportedChanged(isPictureInPictureSupported()); > } > > void PlaybackSessionModelMediaElement::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event& event) >@@ -573,6 +576,11 @@ double PlaybackSessionModelMediaElement::volume() const > return m_mediaElement ? m_mediaElement->volume() : 0; > } > >+bool PlaybackSessionModelMediaElement::isPictureInPictureSupported() const >+{ >+ return m_mediaElement ? m_mediaElement->isVideo() : false; >+} >+ > bool PlaybackSessionModelMediaElement::isPictureInPictureActive() const > { > if (!m_mediaElement) >diff --git a/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm b/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm >index 32a1e722efce068755a40b41936101cc2902b3f2..d43b87c0d95220a17f4c10bf247ddcba30d098fd 100644 >--- a/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm >+++ b/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm >@@ -202,6 +202,7 @@ private: > FloatSize videoDimensions() const override; > bool isMuted() const override; > double volume() const override; >+ bool isPictureInPictureSupported() const override; > bool isPictureInPictureActive() const override; > void willEnterPictureInPicture() final; > void didEnterPictureInPicture() final; >@@ -634,6 +635,12 @@ bool VideoFullscreenControllerContext::isPictureInPictureActive() const > return m_playbackModel ? m_playbackModel->isPictureInPictureActive() : false; > } > >+bool VideoFullscreenControllerContext::isPictureInPictureSupported() const >+{ >+ ASSERT(isUIThread()); >+ return m_playbackModel ? m_playbackModel->isPictureInPictureSupported() : false; >+} >+ > void VideoFullscreenControllerContext::willEnterPictureInPicture() > { > ASSERT(isUIThread()); >diff --git a/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.h b/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.h >index 9aa955fc44fa4c84b9e4a0f66a943aa5b84b29e8..e7db397cce40fd5bdcc4f5af26e7bf327419f60e 100644 >--- a/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.h >+++ b/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.h >@@ -62,12 +62,15 @@ public: > void audioMediaSelectionIndexChanged(uint64_t) final; > void legibleMediaSelectionIndexChanged(uint64_t) final; > void externalPlaybackChanged(bool /* enabled */, PlaybackSessionModel::ExternalPlaybackTargetType, const String& /* localizedDeviceName */) final; >+ void isPictureInPictureSupportedChanged(bool) final; > > void invalidate(); > void ensureControlsManager(); > #if ENABLE(WEB_PLAYBACK_CONTROLS_MANAGER) > void setPlayBackControlsManager(WebPlaybackControlsManager *); > WebPlaybackControlsManager *playBackControlsManager(); >+ >+ void updatePlaybackControlsManagerCanTogglePictureInPicture(); > #endif > void beginScrubbing(); > void endScrubbing(); >diff --git a/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.mm b/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.mm >index 602eddba51fce11393dbce445fbb86086648c967..b86266d4e33d007bfd78a6fe533c7f5386d3939e 100644 >--- a/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.mm >+++ b/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.mm >@@ -184,12 +184,17 @@ void PlaybackSessionInterfaceMac::legibleMediaSelectionIndexChanged(uint64_t sel > #endif > } > >-void PlaybackSessionInterfaceMac::externalPlaybackChanged(bool enabled, PlaybackSessionModel::ExternalPlaybackTargetType, const String&) >+void PlaybackSessionInterfaceMac::isPictureInPictureSupportedChanged(bool) > { > #if ENABLE(WEB_PLAYBACK_CONTROLS_MANAGER) >- [playBackControlsManager() setCanTogglePictureInPicture:!enabled]; >-#else >- UNUSED_PARAM(enabled); >+ updatePlaybackControlsManagerCanTogglePictureInPicture(); >+#endif >+} >+ >+void PlaybackSessionInterfaceMac::externalPlaybackChanged(bool, PlaybackSessionModel::ExternalPlaybackTargetType, const String&) >+{ >+#if ENABLE(WEB_PLAYBACK_CONTROLS_MANAGER) >+ updatePlaybackControlsManagerCanTogglePictureInPicture(); > #endif > } > >@@ -237,6 +242,17 @@ void PlaybackSessionInterfaceMac::setPlayBackControlsManager(WebPlaybackControls > [manager setLegibleMediaSelectionOptions:m_playbackSessionModel->legibleMediaSelectionOptions() withSelectedIndex:static_cast<NSUInteger>(m_playbackSessionModel->legibleMediaSelectedIndex())]; > } > >+void PlaybackSessionInterfaceMac::updatePlaybackControlsManagerCanTogglePictureInPicture() >+{ >+ PlaybackSessionModel* model = playbackSessionModel(); >+ if (!model) { >+ [playBackControlsManager() setCanTogglePictureInPicture:NO]; >+ return; >+ } >+ >+ [playBackControlsManager() setCanTogglePictureInPicture:model->isPictureInPictureSupported() && !model->externalPlaybackEnabled()]; >+} >+ > void PlaybackSessionInterfaceMac::updatePlaybackControlsManagerTiming(double currentTime, double anchorTime, double playbackRate, bool isPlaying) > { > WebPlaybackControlsManager *manager = playBackControlsManager(); >diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >index 66c28ebffcd523b7045c15adb2754f42a0cd8243..e0c29c5e4b7fd91ed3ca064b9c923e48abac8fb8 100644 >--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >@@ -6495,6 +6495,15 @@ - (void)_setDefersLoadingForTesting:(BOOL)defersLoading > _page->setDefersLoadingForTesting(defersLoading); > } > >+- (BOOL)_canTogglePictureInPictureForTesting >+{ >+#if HAVE(TOUCH_BAR) >+ return _impl->canTogglePictureInPictureForTesting(); >+#else >+ return NO; >+#endif >+} >+ > - (_WKInspector *)_inspector > { > if (auto* inspector = _page->inspector()) >diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h >index 6d1cb1df0d43ee23bcc0812ce84c1e6fc27a7d73..f5e99972298e7ce58d6b31a1af2ca720c65ffe56 100644 >--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h >+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h >@@ -472,6 +472,8 @@ typedef NS_OPTIONS(NSUInteger, _WKRectEdge) { > - (BOOL)_completeBackSwipeForTesting; > - (void)_setDefersLoadingForTesting:(BOOL)defersLoading; > >+- (BOOL)_canTogglePictureInPictureForTesting; >+ > @property (nonatomic, readonly) _WKInspector *_inspector WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); > @property (nonatomic, readonly) _WKFrameHandle *_mainFrame WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); > >diff --git a/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.h b/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.h >index 68a88b5451579e8824cad1ede889f5f5b206ad34..a2773e6f9f23ba2664875b5e819a272ce7543596 100644 >--- a/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.h >+++ b/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.h >@@ -85,6 +85,7 @@ public: > void wirelessVideoPlaybackDisabledChanged(bool); > void mutedChanged(bool); > void volumeChanged(double); >+ void pictureInPictureSupportedChanged(bool); > void pictureInPictureActiveChanged(bool); > > private: >@@ -136,6 +137,7 @@ private: > bool wirelessVideoPlaybackDisabled() const final { return m_wirelessVideoPlaybackDisabled; } > bool isMuted() const final { return m_muted; } > double volume() const final { return m_volume; } >+ bool isPictureInPictureSupported() const final { return m_pictureInPictureSupported; } > bool isPictureInPictureActive() const final { return m_pictureInPictureActive; } > > PlaybackSessionManagerProxy* m_manager; >@@ -163,6 +165,7 @@ private: > bool m_wirelessVideoPlaybackDisabled { false }; > bool m_muted { false }; > double m_volume { 0 }; >+ bool m_pictureInPictureSupported { false }; > bool m_pictureInPictureActive { false }; > }; > >@@ -213,6 +216,7 @@ private: > void handleControlledElementIDResponse(uint64_t, String) const; > void mutedChanged(uint64_t contextId, bool muted); > void volumeChanged(uint64_t contextId, double volume); >+ void pictureInPictureSupportedChanged(uint64_t contextId, bool pictureInPictureSupported); > void pictureInPictureActiveChanged(uint64_t contextId, bool pictureInPictureActive); > > // Messages to PlaybackSessionManager >diff --git a/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.messages.in b/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.messages.in >index b151c9ca3e61169c9cc46f77651865f28fdf6674..b31b5821263859db374563a9974850b6e8881723 100644 >--- a/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.messages.in >+++ b/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.messages.in >@@ -38,6 +38,7 @@ messages -> PlaybackSessionManagerProxy { > RateChanged(uint64_t contextId, bool isPlaying, double rate) > MutedChanged(uint64_t contextId, bool muted); > VolumeChanged(uint64_t contextId, double volume); >+ PictureInPictureSupportedChanged(uint64_t contextID, bool pictureInPictureSupported) > PictureInPictureActiveChanged(uint64_t contextId, bool pictureInPictureActive) > SetUpPlaybackControlsManagerWithID(uint64_t contextId) > ClearPlaybackControlsManager() >diff --git a/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.mm b/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.mm >index 6304be7e8ce6ed889b94f7e52cf504a8deb1e543..dc36171b7b41f62eae4be3dfd3ee3cdf9e3c06d0 100644 >--- a/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.mm >+++ b/Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.mm >@@ -276,6 +276,13 @@ void PlaybackSessionModelContext::volumeChanged(double volume) > client->volumeChanged(volume); > } > >+void PlaybackSessionModelContext::pictureInPictureSupportedChanged(bool supported) >+{ >+ m_pictureInPictureSupported = supported; >+ for (auto* client : m_clients) >+ client->isPictureInPictureSupportedChanged(supported); >+} >+ > void PlaybackSessionModelContext::pictureInPictureActiveChanged(bool active) > { > m_pictureInPictureActive = active; >@@ -478,6 +485,11 @@ void PlaybackSessionManagerProxy::rateChanged(uint64_t contextId, bool isPlaying > ensureModel(contextId).rateChanged(isPlaying, rate); > } > >+void PlaybackSessionManagerProxy::pictureInPictureSupportedChanged(uint64_t contextId, bool supported) >+{ >+ ensureModel(contextId).pictureInPictureSupportedChanged(supported); >+} >+ > void PlaybackSessionManagerProxy::pictureInPictureActiveChanged(uint64_t contextId, bool active) > { > ensureModel(contextId).pictureInPictureActiveChanged(active); >diff --git a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h >index 796c928d091c0a66b98b4ce2721955c4b03f32b3..d964d509d90bd3038ac9e58c39fe272bca17a8f3 100644 >--- a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h >+++ b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h >@@ -559,6 +559,8 @@ public: > > void updateTouchBarAndRefreshTextBarIdentifiers(); > void setIsCustomizingTouchBar(bool isCustomizingTouchBar) { m_isCustomizingTouchBar = isCustomizingTouchBar; }; >+ >+ bool canTogglePictureInPictureForTesting(); > #endif // HAVE(TOUCH_BAR) > > bool beginBackSwipeForTesting(); >diff --git a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm >index 552d6505d2538b86c5fceee77d950ba8c25a4006..1af4addf1d7c47a7128629aee3618dd981770bb4 100644 >--- a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm >+++ b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm >@@ -1235,11 +1235,13 @@ void WebViewImpl::updateMediaTouchBar() > if (!m_playbackControlsManager) { > m_playbackControlsManager = adoptNS([[WebPlaybackControlsManager alloc] init]); > [m_playbackControlsManager setAllowsPictureInPicturePlayback:m_page->preferences().allowsPictureInPictureMediaPlayback()]; >- [m_playbackControlsManager setCanTogglePictureInPicture:YES]; >+ [m_playbackControlsManager setCanTogglePictureInPicture:NO]; > } > >- if (PlatformPlaybackSessionInterface* interface = m_page->playbackSessionManager()->controlsManagerInterface()) >+ if (PlatformPlaybackSessionInterface* interface = m_page->playbackSessionManager()->controlsManagerInterface()) { > [m_playbackControlsManager setPlaybackSessionInterfaceMac:interface]; >+ interface->updatePlaybackControlsManagerCanTogglePictureInPicture(); >+ } > > [m_mediaTouchBarProvider setPlaybackControlsController:m_playbackControlsManager.get()]; > [m_mediaPlaybackControlsView setPlaybackControlsController:m_playbackControlsManager.get()]; >@@ -1275,6 +1277,19 @@ void WebViewImpl::updateMediaTouchBar() > #endif > } > >+#if HAVE(TOUCH_BAR) >+ >+bool WebViewImpl::canTogglePictureInPictureForTesting() >+{ >+#if ENABLE(WEB_PLAYBACK_CONTROLS_MANAGER) >+ return [m_playbackControlsManager canTogglePictureInPicture]; >+#else >+ return NO; >+#endif >+} >+ >+#endif // HAVE(TOUCH_BAR) >+ > void WebViewImpl::forceRequestCandidatesForTesting() > { > m_canCreateTouchBars = true; >diff --git a/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenViewController.mm b/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenViewController.mm >index d88d63d86e7e06eddcaaf66664eaff2af3553662..0fc388550cafe4f3ca721969f09464695903e671 100644 >--- a/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenViewController.mm >+++ b/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenViewController.mm >@@ -65,6 +65,10 @@ public: > m_parent.playing = isPlaying; > } > >+ void pictureInPictureSupportedChanged(bool) override >+ { >+ } >+ > void pictureInPictureActiveChanged(bool active) override > { > m_parent.pictureInPictureActive = active; >diff --git a/Source/WebKit/WebProcess/cocoa/PlaybackSessionManager.h b/Source/WebKit/WebProcess/cocoa/PlaybackSessionManager.h >index 1a857626991139b69de6c8310105511497e292aa..f0de62b8db05a37eae7d972ba66fbc587e3240f9 100644 >--- a/Source/WebKit/WebProcess/cocoa/PlaybackSessionManager.h >+++ b/Source/WebKit/WebProcess/cocoa/PlaybackSessionManager.h >@@ -89,6 +89,7 @@ private: > void wirelessVideoPlaybackDisabledChanged(bool) final; > void mutedChanged(bool) final; > void volumeChanged(double) final; >+ void isPictureInPictureSupportedChanged(bool) final; > > PlaybackSessionInterfaceContext(PlaybackSessionManager&, uint64_t contextId); > >@@ -143,6 +144,7 @@ protected: > void wirelessVideoPlaybackDisabledChanged(uint64_t contextId, bool); > void mutedChanged(uint64_t contextId, bool); > void volumeChanged(uint64_t contextId, double); >+ void isPictureInPictureSupportedChanged(uint64_t contextId, bool); > > // Messages from PlaybackSessionManagerProxy > void play(uint64_t contextId); >diff --git a/Source/WebKit/WebProcess/cocoa/PlaybackSessionManager.mm b/Source/WebKit/WebProcess/cocoa/PlaybackSessionManager.mm >index 088494bb67ab49deb4bc2708ddb3b40d60ba2156..d0aec25e92e97c2852b96dd35fc41671662d8d89 100644 >--- a/Source/WebKit/WebProcess/cocoa/PlaybackSessionManager.mm >+++ b/Source/WebKit/WebProcess/cocoa/PlaybackSessionManager.mm >@@ -154,6 +154,12 @@ void PlaybackSessionInterfaceContext::mutedChanged(bool muted) > m_manager->mutedChanged(m_contextId, muted); > } > >+void PlaybackSessionInterfaceContext::isPictureInPictureSupportedChanged(bool supported) >+{ >+ if (m_manager) >+ m_manager->isPictureInPictureSupportedChanged(m_contextId, supported); >+} >+ > void PlaybackSessionInterfaceContext::volumeChanged(double volume) > { > if (m_manager) >@@ -402,6 +408,11 @@ void PlaybackSessionManager::volumeChanged(uint64_t contextId, double volume) > m_page->send(Messages::PlaybackSessionManagerProxy::VolumeChanged(contextId, volume)); > } > >+void PlaybackSessionManager::isPictureInPictureSupportedChanged(uint64_t contextId, bool supported) >+{ >+ m_page->send(Messages::PlaybackSessionManagerProxy::PictureInPictureSupportedChanged(contextId, supported)); >+} >+ > #pragma mark Messages from PlaybackSessionManagerProxy: > > void PlaybackSessionManager::play(uint64_t contextId) >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index 3ad2b213c97fe7bfc32acd3b562508667bdd4039..28c28db81202333dd26e0810d5a97885127c354d 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,19 @@ >+2018-09-07 Matt Rajca <mrajca@apple.com> >+ >+ https://bugs.webkit.org/show_bug.cgi?id=189433 >+ <rdar://problem/44186498> Touch Bar displays an active PIP button for audio elements (and it doesn't do anything) >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ An API test has been added that checks the value of the canTogglePictureInPicture and ensures it >+ is NO when audio elements are playing. >+ >+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: >+ * TestWebKitAPI/Tests/WebKitCocoa/PictureInPictureDelegate.html: >+ * TestWebKitAPI/Tests/WebKitCocoa/PictureInPictureDelegate.mm: >+ (TestWebKitAPI::TEST): >+ * TestWebKitAPI/Tests/WebKitCocoa/audio-with-controls.html: Added. >+ > 2018-09-07 Frederic Wang <fwang@igalia.com> > > [CSSOM View] Handle the scrollingElement in Element::scroll(Left/Top/Width/Height/To) >diff --git a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj >index 4cb5ecca4c4a7190cd1e34eac6f1441c9b7d15c4..f744d582722c30900a36d13488c1c8a491f33846 100644 >--- a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj >+++ b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj >@@ -700,6 +700,7 @@ > C540F784152E5A9A00A40C8C /* verboseMarkup.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = C540F783152E5A7800A40C8C /* verboseMarkup.html */; }; > C54237F116B8957D00E638FC /* PasteboardNotifications_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C54237ED16B8955800E638FC /* PasteboardNotifications_Bundle.cpp */; }; > C5E1AFFE16B221F1006CC1F2 /* execCopy.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = C5E1AFFD16B22179006CC1F2 /* execCopy.html */; }; >+ C944160021430E8900B1EDDA /* audio-with-controls.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = C94415FF21430B6700B1EDDA /* audio-with-controls.html */; }; > C95984F41E36BC6B002C0D45 /* autoplay-check.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = C95984F21E36BC55002C0D45 /* autoplay-check.html */; }; > C95984F51E36BC6B002C0D45 /* autoplay-no-audio-check.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = C95984F31E36BC55002C0D45 /* autoplay-no-audio-check.html */; }; > C95984F71E36BCEF002C0D45 /* test-without-audio-track.mp4 in Copy Resources */ = {isa = PBXBuildFile; fileRef = C95984F61E36BCD7002C0D45 /* test-without-audio-track.mp4 */; }; >@@ -947,6 +948,7 @@ > 7C9ED98B17A19F4B00E4DC33 /* attributedStringStrikethrough.html in Copy Resources */, > 37137E4B21124D01002BEEA4 /* AttrStyle.html in Copy Resources */, > CD9E292E1C90C33F000BB800 /* audio-only.html in Copy Resources */, >+ C944160021430E8900B1EDDA /* audio-with-controls.html in Copy Resources */, > CD57779C211CE91F001B371E /* audio-with-web-audio.html in Copy Resources */, > 76E182DF154767E600F1FADD /* auto-submitting-form.html in Copy Resources */, > F41AB99F1EF4696B0083FA08 /* autofocus-contenteditable.html in Copy Resources */, >@@ -1917,6 +1919,7 @@ > C54237ED16B8955800E638FC /* PasteboardNotifications_Bundle.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PasteboardNotifications_Bundle.cpp; sourceTree = "<group>"; }; > C54237EE16B8955800E638FC /* PasteboardNotifications.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = PasteboardNotifications.mm; sourceTree = "<group>"; }; > C5E1AFFD16B22179006CC1F2 /* execCopy.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = execCopy.html; sourceTree = "<group>"; }; >+ C94415FF21430B6700B1EDDA /* audio-with-controls.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "audio-with-controls.html"; sourceTree = "<group>"; }; > C95501BE19AD2FAF0049BE3E /* Preferences.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = Preferences.mm; sourceTree = "<group>"; }; > C95984F21E36BC55002C0D45 /* autoplay-check.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "autoplay-check.html"; sourceTree = "<group>"; }; > C95984F31E36BC55002C0D45 /* autoplay-no-audio-check.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "autoplay-no-audio-check.html"; sourceTree = "<group>"; }; >@@ -2621,6 +2624,7 @@ > 5C9E59401D3EB1DE00E3C62E /* ApplicationCache.db-wal */, > F4856CA21E6498A8009D7EE7 /* attachment-element.html */, > 3760C4F221124BD000233ACC /* AttrStyle.html */, >+ C94415FF21430B6700B1EDDA /* audio-with-controls.html */, > CD57779A211CE6B7001B371E /* audio-with-web-audio.html */, > F41AB9981EF4692C0083FA08 /* autofocus-contenteditable.html */, > 93CFA8661CEB9DE1000565A8 /* autofocused-text-input.html */, >diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/PictureInPictureDelegate.html b/Tools/TestWebKitAPI/Tests/WebKitCocoa/PictureInPictureDelegate.html >index f95b7f199b00df7af50de073de9dc13531825d37..82cdc81a42daef62c713aa2a77cbbbab7c6dd743 100644 >--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/PictureInPictureDelegate.html >+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/PictureInPictureDelegate.html >@@ -8,6 +8,9 @@ > }); > > function load() { >+ var target = document.getElementById('video'); >+ target.play(); >+ > if (window.webkit !== undefined && window.webkit.messageHandlers.pictureInPictureChangeHandler !== undefined) { > window.webkit.messageHandlers.pictureInPictureChangeHandler.postMessage('load'); > } >diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/PictureInPictureDelegate.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/PictureInPictureDelegate.mm >index c224490869d8b8e7c2055c624b96d63a6b512ce5..acac484eb64d79ef921de7af0c3112d08533b264 100644 >--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/PictureInPictureDelegate.mm >+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/PictureInPictureDelegate.mm >@@ -30,6 +30,7 @@ > #import "PlatformUtilities.h" > #import "PlatformWebView.h" > #import "Test.h" >+#import "TestWKWebView.h" > #import <JavaScriptCore/JSRetainPtr.h> > #import <JavaScriptCore/JavaScriptCore.h> > #import <WebKit/WKPagePrivateMac.h> >@@ -119,13 +120,17 @@ TEST(PictureInPicture, WKUIDelegate) > RetainPtr<PictureInPictureUIDelegate> handler = adoptNS([[PictureInPictureUIDelegate alloc] init]); > [[configuration userContentController] addScriptMessageHandler:handler.get() name:@"pictureInPictureChangeHandler"]; > [webView setUIDelegate:handler.get()]; >- >+ >+#if HAVE(TOUCH_BAR) >+ [webView _forceRequestCandidates]; >+#endif >+ > RetainPtr<NSWindow> window = adoptNS([[NSWindow alloc] initWithContentRect:[webView frame] styleMask:NSWindowStyleMaskBorderless backing:NSBackingStoreBuffered defer:NO]); > [[window contentView] addSubview:webView.get()]; > [window makeKeyAndOrderFront:nil]; > > NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"PictureInPictureDelegate" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]]; >- >+ > receivedLoadedMessage = false; > > [webView loadRequest:request]; >@@ -138,16 +143,46 @@ TEST(PictureInPicture, WKUIDelegate) > [webView mouseDown:event]; > TestWebKitAPI::Util::run(&hasVideoInPictureInPictureCalled); > ASSERT_TRUE(hasVideoInPictureInPictureValue); >- >- sleep(1_s); // Wait for PIPAgent to launch, or it won't call -pipDidClose: callback. >- >+ >+ // Wait for PIPAgent to launch, or it won't call -pipDidClose: callback. >+ [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:1]]; >+ >+#if HAVE(TOUCH_BAR) >+ ASSERT_TRUE([webView _canTogglePictureInPictureForTesting]); >+#endif >+ > hasVideoInPictureInPictureCalled = false; > [webView mouseDown:event]; > TestWebKitAPI::Util::run(&hasVideoInPictureInPictureCalled); > ASSERT_FALSE(hasVideoInPictureInPictureValue); > } >- >- >+ >+#if HAVE(TOUCH_BAR) >+ >+TEST(PictureInPicture, AudioCannotTogglePictureInPicture) >+{ >+ RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]); >+ RetainPtr<TestWKWebView> webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 54) configuration:configuration.get()]); >+ [configuration preferences]._fullScreenEnabled = YES; >+ [configuration preferences]._allowsPictureInPictureMediaPlayback = YES; >+ RetainPtr<PictureInPictureUIDelegate> handler = adoptNS([[PictureInPictureUIDelegate alloc] init]); >+ [[configuration userContentController] addScriptMessageHandler:handler.get() name:@"pictureInPictureChangeHandler"]; >+ [webView setUIDelegate:handler.get()]; >+ [webView _forceRequestCandidates]; >+ >+ RetainPtr<NSWindow> window = adoptNS([[NSWindow alloc] initWithContentRect:[webView frame] styleMask:NSWindowStyleMaskBorderless backing:NSBackingStoreBuffered defer:NO]); >+ [[window contentView] addSubview:webView.get()]; >+ [window makeKeyAndOrderFront:nil]; >+ >+ [webView synchronouslyLoadTestPageNamed:@"audio-with-controls"]; >+ [webView evaluateJavaScript:@"play()" completionHandler:nil]; >+ >+ [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:1]]; >+ ASSERT_FALSE([webView _canTogglePictureInPictureForTesting]); >+} >+ >+#endif // HAVE(TOUCH_BAR) >+ > TEST(PictureInPicture, WKPageUIClient) > { > WKRetainPtr<WKContextRef> context = adoptWK(WKContextCreate()); >diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/audio-with-controls.html b/Tools/TestWebKitAPI/Tests/WebKitCocoa/audio-with-controls.html >new file mode 100644 >index 0000000000000000000000000000000000000000..2d727b7fac6d8b093c4173143a8ed32becd18545 >--- /dev/null >+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/audio-with-controls.html >@@ -0,0 +1,24 @@ >+<html> >+ <head> >+ <script> >+ function pageLoaded() { >+ try { >+ window.webkit.messageHandlers.pictureInPictureChangeHandler.postMessage('load'); >+ } catch(e) { } >+ } >+ >+ function play() { >+ document.getElementById("audio").play(); >+ } >+ >+ function didStartPlaying() { >+ try { >+ window.webkit.messageHandlers.pictureInPictureChangeHandler.postMessage('playing'); >+ } catch(e) { } >+ } >+ </script> >+ </head> >+ <body onload="pageLoaded()"> >+ <audio controls id="audio" src="silence-long.m4a" onplaying="didStartPlaying()" /> >+ </body> >+</html>
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=349216">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Flags:
eric.carlson
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 189433
:
349199
|
349206
|
349209
| 349216