⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 280840 in webkit


Ignore:
Timestamp:
Aug 10, 2021, 7:31:51 AM (5 years ago)
Author:
Devin Rousso
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.

Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r280826 r280840  
     12021-08-10  Devin Rousso  <drousso@apple.com>
     2
     3        [macOS] REGRESSION(r278850): modifying `playbackRate` via JS on a TouchBar mac only sometimes works and also causes the `defaultPlaybackRate` to change
     4        https://bugs.webkit.org/show_bug.cgi?id=228939
     5        <rdar://problem/80427578>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Test: VideoControlsManager.VideoControlsManagerDoesNotChangeValuesExposedToJavaScript
     10
     11        * platform/mac/PlaybackSessionInterfaceMac.mm:
     12        (WebCore::PlaybackSessionInterfaceMac::rateChanged):
     13        Make sure to change the `defaultPlaybackRate` before the `rate` as otherwise the former will
     14        override changes made to the latter (see r278850 (and r277203) for details as to why).
     15
     16        * platform/mac/WebPlaybackControlsManager.h:
     17        * platform/mac/WebPlaybackControlsManager.mm:
     18        (-[WebPlaybackControlsManager setDefaultPlaybackRate:]):
     19        (-[WebPlaybackControlsManager setDefaultPlaybackRate:fromJavaScript:]): Added.
     20        (-[WebPlaybackControlsManager setRate:]):
     21        (-[WebPlaybackControlsManager setRate:fromJavaScript:]): Added.
     22        Add a way to distinguish whether the change to the `rate`/`defaultPlaybackRate` came from JS
     23        or some native call (e.g. AVKit). If the former, don't propagate the change to the playback
     24        model (which eventually is the `HTMLMediaElement` in the WebProcess) as that's where the
     25        value came from in the first place. Note that we still want to propagate the `rate` to the
     26        `defaultPlaybackRate` (and vice versa when `isPlaying`) in the UIProcess for the reasons
     27        described in r278850 (and r277203), but we don't want to expose it to JS.
     28
     29        * platform/ios/PlaybackSessionInterfaceAVKit.mm:
     30        (WebCore::PlaybackSessionInterfaceAVKit::rateChanged):
     31        * platform/ios/WebAVPlayerController.h:
     32        * platform/ios/WebAVPlayerController.mm:
     33        (-[WebAVPlayerController setDefaultPlaybackRate:]):
     34        (-[WebAVPlayerController setDefaultPlaybackRate:fromJavaScript:]): Added.
     35        (-[WebAVPlayerController setRate:]):
     36        (-[WebAVPlayerController setRate:fromJavaScript:]): Added.
     37        Drive-by: r278850 was based r277203 (which added the above), so fix that code now too.
     38
    1392021-08-09  Yusuke Suzuki  <ysuzuki@apple.com>
    240

  • trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.mm

    r279335 r280840  
    131131void PlaybackSessionInterfaceAVKit::rateChanged(OptionSet<PlaybackSessionModel::PlaybackState> playbackState, double playbackRate, double defaultPlaybackRate)
    132132{
    133     [m_playerController setDefaultPlaybackRate:defaultPlaybackRate];
     133    [m_playerController setDefaultPlaybackRate:defaultPlaybackRate fromJavaScript:YES];
    134134    if (!playbackState.contains(PlaybackSessionModel::PlaybackState::Stalled))
    135         [m_playerController setRate:playbackState.contains(PlaybackSessionModel::PlaybackState::Playing) ? playbackRate : 0.];
     135        [m_playerController setRate:playbackState.contains(PlaybackSessionModel::PlaybackState::Playing) ? playbackRate : 0. fromJavaScript:YES];
    136136}
    137137

  • trunk/Source/WebCore/platform/ios/WebAVPlayerController.h

    r274249 r280840  
    104104@property (NS_NONATOMIC_IOSONLY, retain, readwrite) AVValueTiming *minTiming;
    105105@property (NS_NONATOMIC_IOSONLY, retain, readwrite) AVValueTiming *maxTiming;
     106
     107- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate fromJavaScript:(BOOL)fromJavaScript;
     108- (void)setRate:(double)rate fromJavaScript:(BOOL)fromJavaScript;
    106109@end
    107110

  • trunk/Source/WebCore/platform/ios/WebAVPlayerController.mm

    r277203 r280840  
    166166- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate
    167167{
     168    [self setDefaultPlaybackRate:defaultPlaybackRate fromJavaScript:NO];
     169}
     170
     171- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate fromJavaScript:(BOOL)fromJavaScript
     172{
    168173    if (defaultPlaybackRate == _defaultPlaybackRate)
    169174        return;
    … …  
    171176    _defaultPlaybackRate = defaultPlaybackRate;
    172177
    173     if (self.delegate && self.delegate->defaultPlaybackRate() != _defaultPlaybackRate)
     178    if (!fromJavaScript && self.delegate && self.delegate->defaultPlaybackRate() != _defaultPlaybackRate)
    174179        self.delegate->setDefaultPlaybackRate(_defaultPlaybackRate);
    175180
    176181    if ([self isPlaying])
    177         [self setRate:_defaultPlaybackRate];
     182        [self setRate:_defaultPlaybackRate fromJavaScript:fromJavaScript];
    178183}
    179184
    … …  
    184189
    185190- (void)setRate:(double)rate
     191{
     192    [self setRate:rate fromJavaScript:NO];
     193}
     194
     195- (void)setRate:(double)rate fromJavaScript:(BOOL)fromJavaScript
    186196{
    187197    if (rate == _rate)
    … …  
    202212    // resuming after pausing. As such, WebKit should return the `playbackRate` instead of the
    203213    // `defaultPlaybackRate` in these cases when communicating with AVKit.
    204     [self setDefaultPlaybackRate:_rate];
    205 
    206     if (self.delegate && self.delegate->playbackRate() != _rate)
     214    [self setDefaultPlaybackRate:_rate fromJavaScript:fromJavaScript];
     215
     216    if (!fromJavaScript && self.delegate && self.delegate->playbackRate() != _rate)
    207217        self.delegate->setPlaybackRate(_rate);
    208218}

  • trunk/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.mm

    r279043 r280840  
    9898    auto isPlaying = playbackState.contains(PlaybackSessionModel::PlaybackState::Playing);
    9999    WebPlaybackControlsManager* controlsManager = playBackControlsManager();
    100     [controlsManager setRate:isPlaying ? playbackRate : 0.];
    101     [controlsManager setDefaultPlaybackRate:defaultPlaybackRate];
     100    [controlsManager setDefaultPlaybackRate:defaultPlaybackRate fromJavaScript:YES];
     101    [controlsManager setRate:isPlaying ? playbackRate : 0. fromJavaScript:YES];
    102102    [controlsManager setPlaying:isPlaying];
    103103    updatePlaybackControlsManagerTiming(m_playbackSessionModel ? m_playbackSessionModel->currentTime() : 0, [[NSProcessInfo processInfo] systemUptime], playbackRate, isPlaying);

  • trunk/Source/WebCore/platform/mac/WebPlaybackControlsManager.h

    r275100 r280840  
    8585- (void)setAudioMediaSelectionIndex:(NSUInteger)selectedIndex;
    8686- (void)setLegibleMediaSelectionIndex:(NSUInteger)selectedIndex;
     87
     88- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate fromJavaScript:(BOOL)fromJavaScript;
     89- (void)setRate:(double)rate fromJavaScript:(BOOL)fromJavaScript;
    8790@end
    8891

  • trunk/Source/WebCore/platform/mac/WebPlaybackControlsManager.mm

    r278850 r280840  
    337337- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate
    338338{
     339    [self setDefaultPlaybackRate:defaultPlaybackRate fromJavaScript:NO];
     340}
     341
     342- (void)setDefaultPlaybackRate:(double)defaultPlaybackRate fromJavaScript:(BOOL)fromJavaScript
     343{
    339344    if (defaultPlaybackRate == _defaultPlaybackRate)
    340345        return;
    … …  
    342347    _defaultPlaybackRate = defaultPlaybackRate;
    343348
    344     if (_playbackSessionInterfaceMac) {
     349    if (!fromJavaScript && _playbackSessionInterfaceMac) {
    345350        if (auto* model = _playbackSessionInterfaceMac->playbackSessionModel(); model && model->defaultPlaybackRate() != _defaultPlaybackRate)
    346351            model->setDefaultPlaybackRate(_defaultPlaybackRate);
    … …  
    348353
    349354    if ([self isPlaying])
    350         [self setRate:_defaultPlaybackRate];
     355        [self setRate:_defaultPlaybackRate fromJavaScript:fromJavaScript];
    351356}
    352357
    … …  
    357362
    358363- (void)setRate:(float)rate
     364{
     365    [self setRate:rate fromJavaScript:NO];
     366}
     367
     368- (void)setRate:(double)rate fromJavaScript:(BOOL)fromJavaScript
    359369{
    360370    if (rate == _rate)
    … …  
    375385    // resuming after pausing. As such, WebKit should return the `playbackRate` instead of the
    376386    // `defaultPlaybackRate` in these cases when communicating with AVKit.
    377     [self setDefaultPlaybackRate:_rate];
    378 
    379     if (_playbackSessionInterfaceMac) {
     387    [self setDefaultPlaybackRate:_rate fromJavaScript:fromJavaScript];
     388
     389    if (!fromJavaScript && _playbackSessionInterfaceMac) {
    380390        if (auto* model = _playbackSessionInterfaceMac->playbackSessionModel(); model && model->playbackRate() != _rate)
    381391            model->setPlaybackRate(_rate);

  • trunk/Tools/ChangeLog

    r280767 r280840  
     12021-08-10  Devin Rousso  <drousso@apple.com>
     2
     3        [macOS] REGRESSION(r278850): modifying `playbackRate` via JS on a TouchBar mac only sometimes works and also causes the `defaultPlaybackRate` to change
     4        https://bugs.webkit.org/show_bug.cgi?id=228939
     5        <rdar://problem/80427578>
     6
     7        Reviewed by Eric Carlson.
     8
     9        * TestWebKitAPI/Tests/WebKitCocoa/VideoControlsManager.mm:
     10        (TestWebKitAPI::TEST.VideoControlsManager.VideoControlsManagerDoesNotChangeValuesExposedToJavaScript): Added.
     11
    1122021-08-08  Wenson Hsieh  <wenson_hsieh@apple.com>
    213

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/VideoControlsManager.mm

    r272919 r280840  
    2929#import "TestWKWebView.h"
    3030#import <WebKit/WKWebViewConfigurationPrivate.h>
     31#import <WebKit/WKWebViewPrivate.h>
    3132#import <WebKit/WKWebViewPrivateForTesting.h>
    3233#import <wtf/RetainPtr.h>
    … …  
    465466}
    466467
     468TEST(VideoControlsManager, VideoControlsManagerDoesNotChangeValuesExposedToJavaScript)
     469{
     470    RetainPtr<VideoControlsManagerTestWebView> webView = setUpWebViewForTestingVideoControlsManager(NSMakeRect(0, 0, 500, 500));
     471
     472    // A large video with audio should have a controls manager even if it is played via script like this video.
     473    // So the expectation is YES.
     474    [webView loadTestPageNamed:@"large-video-with-audio"];
     475    [webView waitForMediaControlsToShow];
     476    [webView _updateMediaPlaybackControlsManager];
     477
     478    EXPECT_EQ(1.0, [[webView objectByEvaluatingJavaScript:@"document.getElementsByTagName('video')[0].playbackRate"] doubleValue]);
     479    EXPECT_EQ(1.0, [[webView objectByEvaluatingJavaScript:@"document.getElementsByTagName('video')[0].defaultPlaybackRate"] doubleValue]);
     480
     481    [webView objectByEvaluatingJavaScript:@"document.getElementsByTagName('video')[0].playbackRate = 2.0;"];
     482
     483    EXPECT_EQ(2.0, [[webView objectByEvaluatingJavaScript:@"document.getElementsByTagName('video')[0].playbackRate"] doubleValue]);
     484    EXPECT_EQ(1.0, [[webView objectByEvaluatingJavaScript:@"document.getElementsByTagName('video')[0].defaultPlaybackRate"] doubleValue]);
     485}
     486
    467487} // namespace TestWebKitAPI
    468488
Note: See TracChangeset for help on using the changeset viewer.