- Revision
- 280840
- Author
- [email protected]
- Date
- 2021-08-10 07:31:51 -0700 (Tue, 10 Aug 2021)
Log Message
[macOS] REGRESSION(r278850): modifying `playbackRate` via JS on a TouchBar mac only sometimes works and also causes the `defaultPlaybackRate` to change
https://bugs.webkit.org/show_bug.cgi?id=228939
<rdar://problem/80427578>
Reviewed by Eric Carlson.
Source/WebCore:
Test: VideoControlsManager.VideoControlsManagerDoesNotChangeValuesExposedToJavaScript
* platform/mac/PlaybackSessionInterfaceMac.mm:
(WebCore::PlaybackSessionInterfaceMac::rateChanged):
Make sure to change the `defaultPlaybackRate` before the `rate` as otherwise the former will
override changes made to the latter (see r278850 (and r277203) for details as to why).
* platform/mac/WebPlaybackControlsManager.h:
* platform/mac/WebPlaybackControlsManager.mm:
(-[WebPlaybackControlsManager setDefaultPlaybackRate:]):
(-[WebPlaybackControlsManager setDefaultPlaybackRate:fromJavaScript:]): Added.
(-[WebPlaybackControlsManager setRate:]):
(-[WebPlaybackControlsManager setRate:fromJavaScript:]): Added.
Add a way to distinguish whether the change to the `rate`/`defaultPlaybackRate` came from JS
or some native call (e.g. AVKit). If the former, don't propagate the change to the playback
model (which eventually is the `HTMLMediaElement` in the WebProcess) as that's where the
value came from in the first place. Note that we still want to propagate the `rate` to the
`defaultPlaybackRate` (and vice versa when `isPlaying`) in the UIProcess for the reasons
described in r278850 (and r277203), but we don't want to expose it to JS.
* platform/ios/PlaybackSessionInterfaceAVKit.mm:
(WebCore::PlaybackSessionInterfaceAVKit::rateChanged):
* platform/ios/WebAVPlayerController.h:
* platform/ios/WebAVPlayerController.mm:
(-[WebAVPlayerController setDefaultPlaybackRate:]):
(-[WebAVPlayerController setDefaultPlaybackRate:fromJavaScript:]): Added.
(-[WebAVPlayerController setRate:]):
(-[WebAVPlayerController setRate:fromJavaScript:]): Added.
Drive-by: r278850 was based r277203 (which added the above), so fix that code now too.
Tools:
* TestWebKitAPI/Tests/WebKitCocoa/VideoControlsManager.mm:
(TestWebKitAPI::TEST.VideoControlsManager.VideoControlsManagerDoesNotChangeValuesExposedToJavaScript): Added.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (280839 => 280840)
--- trunk/Source/WebCore/ChangeLog 2021-08-10 09:35:09 UTC (rev 280839)
+++ trunk/Source/WebCore/ChangeLog 2021-08-10 14:31:51 UTC (rev 280840)
@@ -1,3 +1,41 @@
+2021-08-10 Devin Rousso <[email protected]>
+
+ [macOS] REGRESSION(r278850): modifying `playbackRate` via JS on a TouchBar mac only sometimes works and also causes the `defaultPlaybackRate` to change
+ https://bugs.webkit.org/show_bug.cgi?id=228939
+ <rdar://problem/80427578>
+
+ Reviewed by Eric Carlson.
+
+ Test: VideoControlsManager.VideoControlsManagerDoesNotChangeValuesExposedToJavaScript
+
+ * platform/mac/PlaybackSessionInterfaceMac.mm:
+ (WebCore::PlaybackSessionInterfaceMac::rateChanged):
+ Make sure to change the `defaultPlaybackRate` before the `rate` as otherwise the former will
+ override changes made to the latter (see r278850 (and r277203) for details as to why).
+
+ * platform/mac/WebPlaybackControlsManager.h:
+ * platform/mac/WebPlaybackControlsManager.mm:
+ (-[WebPlaybackControlsManager setDefaultPlaybackRate:]):
+ (-[WebPlaybackControlsManager setDefaultPlaybackRate:fromJavaScript:]): Added.
+ (-[WebPlaybackControlsManager setRate:]):
+ (-[WebPlaybackControlsManager setRate:fromJavaScript:]): Added.
+ Add a way to distinguish whether the change to the `rate`/`defaultPlaybackRate` came from JS
+ or some native call (e.g. AVKit). If the former, don't propagate the change to the playback
+ model (which eventually is the `HTMLMediaElement` in the WebProcess) as that's where the
+ value came from in the first place. Note that we still want to propagate the `rate` to the
+ `defaultPlaybackRate` (and vice versa when `isPlaying`) in the UIProcess for the reasons
+ described in r278850 (and r277203), but we don't want to expose it to JS.
+
+ * platform/ios/PlaybackSessionInterfaceAVKit.mm:
+ (WebCore::PlaybackSessionInterfaceAVKit::rateChanged):
+ * platform/ios/WebAVPlayerController.h:
+ * platform/ios/WebAVPlayerController.mm:
+ (-[WebAVPlayerController setDefaultPlaybackRate:]):
+ (-[WebAVPlayerController setDefaultPlaybackRate:fromJavaScript:]): Added.
+ (-[WebAVPlayerController setRate:]):
+ (-[WebAVPlayerController setRate:fromJavaScript:]): Added.
+ Drive-by: r278850 was based r277203 (which added the above), so fix that code now too.
+
2021-08-09 Yusuke Suzuki <[email protected]>
Partially revert r280256 behavior change since it broke Facetime
Modified: trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.mm (280839 => 280840)
--- trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.mm 2021-08-10 09:35:09 UTC (rev 280839)
+++ trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.mm 2021-08-10 14:31:51 UTC (rev 280840)
@@ -130,9 +130,9 @@
void PlaybackSessionInterfaceAVKit::rateChanged(OptionSet<PlaybackSessionModel::PlaybackState> playbackState, double playbackRate, double defaultPlaybackRate)
{
- [m_playerController setDefaultPlaybackRate:defaultPlaybackRate];
+ [m_playerController setDefaultPlaybackRate:defaultPlaybackRate fromJavaScript:YES];
if (!playbackState.contains(PlaybackSessionModel::PlaybackState::Stalled))
- [m_playerController setRate:playbackState.contains(PlaybackSessionModel::PlaybackState::Playing) ? playbackRate : 0.];
+ [m_playerController setRate:playbackState.contains(PlaybackSessionModel::PlaybackState::Playing) ? playbackRate : 0. fromJavaScript:YES];
}
void PlaybackSessionInterfaceAVKit::seekableRangesChanged(const TimeRanges& timeRanges, double lastModifiedTime, double liveUpdateInterval)
Modified: trunk/Source/WebCore/platform/ios/WebAVPlayerController.h (280839 => 280840)
--- trunk/Source/WebCore/platform/ios/WebAVPlayerController.h 2021-08-10 09:35:09 UTC (rev 280839)
+++ trunk/Source/WebCore/platform/ios/WebAVPlayerController.h 2021-08-10 14:31:51 UTC (rev 280840)
@@ -103,6 +103,9 @@
@property (NS_NONATOMIC_IOSONLY, retain, readwrite) AVValueTiming *minTiming;
@property (NS_NONATOMIC_IOSONLY, retain, readwrite) AVValueTiming *maxTiming;
+
+- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate fromJavaScript:(BOOL)fromJavaScript;
+- (void)setRate:(double)rate fromJavaScript:(BOOL)fromJavaScript;
@end
#endif
Modified: trunk/Source/WebCore/platform/ios/WebAVPlayerController.mm (280839 => 280840)
--- trunk/Source/WebCore/platform/ios/WebAVPlayerController.mm 2021-08-10 09:35:09 UTC (rev 280839)
+++ trunk/Source/WebCore/platform/ios/WebAVPlayerController.mm 2021-08-10 14:31:51 UTC (rev 280840)
@@ -165,16 +165,21 @@
- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate
{
+ [self setDefaultPlaybackRate:defaultPlaybackRate fromJavaScript:NO];
+}
+
+- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate fromJavaScript:(BOOL)fromJavaScript
+{
if (defaultPlaybackRate == _defaultPlaybackRate)
return;
_defaultPlaybackRate = defaultPlaybackRate;
- if (self.delegate && self.delegate->defaultPlaybackRate() != _defaultPlaybackRate)
+ if (!fromJavaScript && self.delegate && self.delegate->defaultPlaybackRate() != _defaultPlaybackRate)
self.delegate->setDefaultPlaybackRate(_defaultPlaybackRate);
if ([self isPlaying])
- [self setRate:_defaultPlaybackRate];
+ [self setRate:_defaultPlaybackRate fromJavaScript:fromJavaScript];
}
- (double)rate
@@ -184,6 +189,11 @@
- (void)setRate:(double)rate
{
+ [self setRate:rate fromJavaScript:NO];
+}
+
+- (void)setRate:(double)rate fromJavaScript:(BOOL)fromJavaScript
+{
if (rate == _rate)
return;
@@ -201,9 +211,9 @@
// ending scanning, with the `playbackRate` being used in all other cases, including when
// resuming after pausing. As such, WebKit should return the `playbackRate` instead of the
// `defaultPlaybackRate` in these cases when communicating with AVKit.
- [self setDefaultPlaybackRate:_rate];
+ [self setDefaultPlaybackRate:_rate fromJavaScript:fromJavaScript];
- if (self.delegate && self.delegate->playbackRate() != _rate)
+ if (!fromJavaScript && self.delegate && self.delegate->playbackRate() != _rate)
self.delegate->setPlaybackRate(_rate);
}
Modified: trunk/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.mm (280839 => 280840)
--- trunk/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.mm 2021-08-10 09:35:09 UTC (rev 280839)
+++ trunk/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.mm 2021-08-10 14:31:51 UTC (rev 280840)
@@ -97,8 +97,8 @@
#if ENABLE(WEB_PLAYBACK_CONTROLS_MANAGER)
auto isPlaying = playbackState.contains(PlaybackSessionModel::PlaybackState::Playing);
WebPlaybackControlsManager* controlsManager = playBackControlsManager();
- [controlsManager setRate:isPlaying ? playbackRate : 0.];
- [controlsManager setDefaultPlaybackRate:defaultPlaybackRate];
+ [controlsManager setDefaultPlaybackRate:defaultPlaybackRate fromJavaScript:YES];
+ [controlsManager setRate:isPlaying ? playbackRate : 0. fromJavaScript:YES];
[controlsManager setPlaying:isPlaying];
updatePlaybackControlsManagerTiming(m_playbackSessionModel ? m_playbackSessionModel->currentTime() : 0, [[NSProcessInfo processInfo] systemUptime], playbackRate, isPlaying);
#else
Modified: trunk/Source/WebCore/platform/mac/WebPlaybackControlsManager.h (280839 => 280840)
--- trunk/Source/WebCore/platform/mac/WebPlaybackControlsManager.h 2021-08-10 09:35:09 UTC (rev 280839)
+++ trunk/Source/WebCore/platform/mac/WebPlaybackControlsManager.h 2021-08-10 14:31:51 UTC (rev 280840)
@@ -84,6 +84,9 @@
- (void)setLegibleMediaSelectionOptions:(const Vector<WebCore::MediaSelectionOption>&)options withSelectedIndex:(NSUInteger)selectedIndex;
- (void)setAudioMediaSelectionIndex:(NSUInteger)selectedIndex;
- (void)setLegibleMediaSelectionIndex:(NSUInteger)selectedIndex;
+
+- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate fromJavaScript:(BOOL)fromJavaScript;
+- (void)setRate:(double)rate fromJavaScript:(BOOL)fromJavaScript;
@end
#endif // ENABLE(WEB_PLAYBACK_CONTROLS_MANAGER)
Modified: trunk/Source/WebCore/platform/mac/WebPlaybackControlsManager.mm (280839 => 280840)
--- trunk/Source/WebCore/platform/mac/WebPlaybackControlsManager.mm 2021-08-10 09:35:09 UTC (rev 280839)
+++ trunk/Source/WebCore/platform/mac/WebPlaybackControlsManager.mm 2021-08-10 14:31:51 UTC (rev 280840)
@@ -336,18 +336,23 @@
- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate
{
+ [self setDefaultPlaybackRate:defaultPlaybackRate fromJavaScript:NO];
+}
+
+- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate fromJavaScript:(BOOL)fromJavaScript
+{
if (defaultPlaybackRate == _defaultPlaybackRate)
return;
_defaultPlaybackRate = defaultPlaybackRate;
- if (_playbackSessionInterfaceMac) {
+ if (!fromJavaScript && _playbackSessionInterfaceMac) {
if (auto* model = _playbackSessionInterfaceMac->playbackSessionModel(); model && model->defaultPlaybackRate() != _defaultPlaybackRate)
model->setDefaultPlaybackRate(_defaultPlaybackRate);
}
if ([self isPlaying])
- [self setRate:_defaultPlaybackRate];
+ [self setRate:_defaultPlaybackRate fromJavaScript:fromJavaScript];
}
- (float)rate
@@ -357,6 +362,11 @@
- (void)setRate:(float)rate
{
+ [self setRate:rate fromJavaScript:NO];
+}
+
+- (void)setRate:(double)rate fromJavaScript:(BOOL)fromJavaScript
+{
if (rate == _rate)
return;
@@ -374,9 +384,9 @@
// ending scanning, with the `playbackRate` being used in all other cases, including when
// resuming after pausing. As such, WebKit should return the `playbackRate` instead of the
// `defaultPlaybackRate` in these cases when communicating with AVKit.
- [self setDefaultPlaybackRate:_rate];
+ [self setDefaultPlaybackRate:_rate fromJavaScript:fromJavaScript];
- if (_playbackSessionInterfaceMac) {
+ if (!fromJavaScript && _playbackSessionInterfaceMac) {
if (auto* model = _playbackSessionInterfaceMac->playbackSessionModel(); model && model->playbackRate() != _rate)
model->setPlaybackRate(_rate);
}
Modified: trunk/Tools/ChangeLog (280839 => 280840)
--- trunk/Tools/ChangeLog 2021-08-10 09:35:09 UTC (rev 280839)
+++ trunk/Tools/ChangeLog 2021-08-10 14:31:51 UTC (rev 280840)
@@ -1,3 +1,14 @@
+2021-08-10 Devin Rousso <[email protected]>
+
+ [macOS] REGRESSION(r278850): modifying `playbackRate` via JS on a TouchBar mac only sometimes works and also causes the `defaultPlaybackRate` to change
+ https://bugs.webkit.org/show_bug.cgi?id=228939
+ <rdar://problem/80427578>
+
+ Reviewed by Eric Carlson.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/VideoControlsManager.mm:
+ (TestWebKitAPI::TEST.VideoControlsManager.VideoControlsManagerDoesNotChangeValuesExposedToJavaScript): Added.
+
2021-08-08 Wenson Hsieh <[email protected]>
caret-color does not work on first click in ios
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/VideoControlsManager.mm (280839 => 280840)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/VideoControlsManager.mm 2021-08-10 09:35:09 UTC (rev 280839)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/VideoControlsManager.mm 2021-08-10 14:31:51 UTC (rev 280840)
@@ -28,6 +28,7 @@
#import "PlatformUtilities.h"
#import "TestWKWebView.h"
#import <WebKit/WKWebViewConfigurationPrivate.h>
+#import <WebKit/WKWebViewPrivate.h>
#import <WebKit/WKWebViewPrivateForTesting.h>
#import <wtf/RetainPtr.h>
@@ -464,6 +465,25 @@
[webView expectControlsManager:NO afterReceivingMessage:@"playing"];
}
+TEST(VideoControlsManager, VideoControlsManagerDoesNotChangeValuesExposedToJavaScript)
+{
+ RetainPtr<VideoControlsManagerTestWebView> webView = setUpWebViewForTestingVideoControlsManager(NSMakeRect(0, 0, 500, 500));
+
+ // A large video with audio should have a controls manager even if it is played via script like this video.
+ // So the expectation is YES.
+ [webView loadTestPageNamed:@"large-video-with-audio"];
+ [webView waitForMediaControlsToShow];
+ [webView _updateMediaPlaybackControlsManager];
+
+ EXPECT_EQ(1.0, [[webView objectByEvaluatingJavaScript:@"document.getElementsByTagName('video')[0].playbackRate"] doubleValue]);
+ EXPECT_EQ(1.0, [[webView objectByEvaluatingJavaScript:@"document.getElementsByTagName('video')[0].defaultPlaybackRate"] doubleValue]);
+
+ [webView objectByEvaluatingJavaScript:@"document.getElementsByTagName('video')[0].playbackRate = 2.0;"];
+
+ EXPECT_EQ(2.0, [[webView objectByEvaluatingJavaScript:@"document.getElementsByTagName('video')[0].playbackRate"] doubleValue]);
+ EXPECT_EQ(1.0, [[webView objectByEvaluatingJavaScript:@"document.getElementsByTagName('video')[0].defaultPlaybackRate"] doubleValue]);
+}
+
} // namespace TestWebKitAPI
#endif // PLATFORM(MAC)
