Changeset 243337 in webkit

Timestamp:

Mar 21, 2019 3:36:08 PM (5 years ago)

Author:

Brent Fulgham

Message:

Hardening: Use WeakPtrs in PlaybackSessionInterface{Mac,AVKit}
​https://bugs.webkit.org/show_bug.cgi?id=195935
<rdar://problem/49007015>

Reviewed by Eric Carlson.

The PlaybackSessionInterface{Mac,AVKit} implementations store their playback session model
and playback controls manager members as bare pointers, something we've been working
to eliminate.

This patch corrects this oversight.

No new tests since no changes in behavior.

• platform/cocoa/PlaybackSessionModel.h:
• platform/ios/PlaybackSessionInterfaceAVKit.h:
• platform/ios/PlaybackSessionInterfaceAVKit.mm:

(WebCore::PlaybackSessionInterfaceAVKit::PlaybackSessionInterfaceAVKit):
(WebCore::playbackSessionModel const): Moved to implementation since WEBCORE_EXPORT is not
supposed to be used with inline methods.

• platform/mac/PlaybackSessionInterfaceMac.h:
• platform/mac/PlaybackSessionInterfaceMac.mm:

(WebCore::PlaybackSessionInterfaceMac::PlaybackSessionInterfaceMac):
(WebCore::PlaybackSessionInterfaceMac::playbackSessionModel const):
(WebCore::PlaybackSessionInterfaceMac::beginScrubbing):
(WebCore::PlaybackSessionInterfaceMac::endScrubbing):
(WebCore::PlaybackSessionInterfaceMac::playBackControlsManager):

• platform/mac/VideoFullscreenInterfaceMac.mm:

(WebCore::VideoFullscreenInterfaceMac::~VideoFullscreenInterfaceMac):

• platform/mac/WebPlaybackControlsManager.mm:

(-[WebPlaybackControlsManager seekToTime:toleranceBefore:toleranceAfter:]):
(-[WebPlaybackControlsManager setCurrentAudioTouchBarMediaSelectionOption:]):
(-[WebPlaybackControlsManager setCurrentLegibleTouchBarMediaSelectionOption:]):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/cocoa/PlaybackSessionModel.h (modified) (3 diffs)
• platform/ios/PlaybackSessionInterfaceAVKit.h (modified) (3 diffs)
• platform/ios/PlaybackSessionInterfaceAVKit.mm (modified) (3 diffs)
• platform/mac/PlaybackSessionInterfaceMac.h (modified) (3 diffs)
• platform/mac/PlaybackSessionInterfaceMac.mm (modified) (5 diffs)
• platform/mac/VideoFullscreenInterfaceMac.mm (modified) (1 diff)
• platform/mac/WebPlaybackControlsManager.mm (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-03-21  Brent Fulgham  <bfulgham@apple.com>
+
+        Hardening: Use WeakPtrs in PlaybackSessionInterface{Mac,AVKit}
+        https://bugs.webkit.org/show_bug.cgi?id=195935
+        <rdar://problem/49007015>
+
+        Reviewed by Eric Carlson.
+
+        The PlaybackSessionInterface{Mac,AVKit} implementations store their playback session model
+        and playback controls manager members as bare pointers, something we've been working
+        to eliminate.
+        
+        This patch corrects this oversight.
+
+        No new tests since no changes in behavior.
+
+        * platform/cocoa/PlaybackSessionModel.h:
+        * platform/ios/PlaybackSessionInterfaceAVKit.h:
+        * platform/ios/PlaybackSessionInterfaceAVKit.mm:
+        (WebCore::PlaybackSessionInterfaceAVKit::PlaybackSessionInterfaceAVKit):
+        (WebCore::playbackSessionModel const): Moved to implementation since WEBCORE_EXPORT is not
+        supposed to be used with inline methods.
+        * platform/mac/PlaybackSessionInterfaceMac.h:
+        * platform/mac/PlaybackSessionInterfaceMac.mm:
+        (WebCore::PlaybackSessionInterfaceMac::PlaybackSessionInterfaceMac):
+        (WebCore::PlaybackSessionInterfaceMac::playbackSessionModel const):
+        (WebCore::PlaybackSessionInterfaceMac::beginScrubbing):
+        (WebCore::PlaybackSessionInterfaceMac::endScrubbing):
+        (WebCore::PlaybackSessionInterfaceMac::playBackControlsManager):
+        * platform/mac/VideoFullscreenInterfaceMac.mm:
+        (WebCore::VideoFullscreenInterfaceMac::~VideoFullscreenInterfaceMac):
+        * platform/mac/WebPlaybackControlsManager.mm:
+        (-[WebPlaybackControlsManager seekToTime:toleranceBefore:toleranceAfter:]):
+        (-[WebPlaybackControlsManager setCurrentAudioTouchBarMediaSelectionOption:]):
+        (-[WebPlaybackControlsManager setCurrentLegibleTouchBarMediaSelectionOption:]):
+
 2019-03-21  Said Abou-Hallawa  <said@apple.com>
 

trunk/Source/WebCore/platform/cocoa/PlaybackSessionModel.h

@@ -1 +1 @@
 /*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -31 +31 @@
 #include <wtf/Ref.h>
 #include <wtf/Vector.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
@@ -38 +39 @@
 struct MediaSelectionOption;
 
-class PlaybackSessionModel {
+class PlaybackSessionModel : public CanMakeWeakPtr<PlaybackSessionModel> {
 public:
     virtual ~PlaybackSessionModel() { };

trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.h

@@ -1 +1 @@
 /*
- * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -63 +63 @@
     }
     virtual ~PlaybackSessionInterfaceAVKit();
-    PlaybackSessionModel* playbackSessionModel() const { return m_playbackSessionModel; }
+    PlaybackSessionModel* playbackSessionModel() const;
 
     // PlaybackSessionModelClient
@@ -87 +87 @@
 
     RetainPtr<WebAVPlayerController> m_playerController;
-    PlaybackSessionModel* m_playbackSessionModel { nullptr };
+    WeakPtr<PlaybackSessionModel> m_playbackSessionModel;
 };
 

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

@@ -1 +1 @@
 /*
- * Copyright (C) 2014, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -52 +52 @@
 PlaybackSessionInterfaceAVKit::PlaybackSessionInterfaceAVKit(PlaybackSessionModel& model)
     : m_playerController(adoptNS([[WebAVPlayerController alloc] init]))
-    , m_playbackSessionModel(&model)
+    , m_playbackSessionModel(makeWeakPtr(model))
 {
     model.addClient(*this);
@@ -78 +78 @@
 }
 
+PlaybackSessionModel* playbackSessionModel() const
+{
+    return m_playbackSessionModel.get();
+}
+
 void PlaybackSessionInterfaceAVKit::durationChanged(double duration)
 {

trunk/Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.h

@@ -1 +1 @@
 /*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -33 +33 @@
 #include <wtf/RefCounted.h>
 #include <wtf/RetainPtr.h>
+#include <wtf/WeakObjCPtr.h>
+#include <wtf/WeakPtr.h>
 #include <wtf/text/WTFString.h>
 
@@ -75 +77 @@
 private:
     PlaybackSessionInterfaceMac(PlaybackSessionModel&);
-    PlaybackSessionModel* m_playbackSessionModel { nullptr };
+    WeakPtr<PlaybackSessionModel> m_playbackSessionModel;
 #if ENABLE(WEB_PLAYBACK_CONTROLS_MANAGER)
-    WebPlaybackControlsManager *m_playbackControlsManager  { nullptr };
+    WeakObjCPtr<WebPlaybackControlsManager> m_playbackControlsManager;
 
     void updatePlaybackControlsManagerTiming(double currentTime, double anchorTime, double playbackRate, bool isPlaying);

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

@@ -1 +1 @@
 /*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -53 +53 @@
 
 PlaybackSessionInterfaceMac::PlaybackSessionInterfaceMac(PlaybackSessionModel& model)
-    : m_playbackSessionModel(&model)
+    : m_playbackSessionModel(makeWeakPtr(model))
 {
 }
@@ -64 +64 @@
 PlaybackSessionModel* PlaybackSessionInterfaceMac::playbackSessionModel() const
 {
-    return m_playbackSessionModel;
+    return m_playbackSessionModel.get();
 }
 
@@ -111 +111 @@
     updatePlaybackControlsManagerTiming(m_playbackSessionModel ? m_playbackSessionModel->currentTime() : 0, [[NSProcessInfo processInfo] systemUptime], 0, false);
 #endif
-    playbackSessionModel()->beginScrubbing();
+    if (auto* model = playbackSessionModel())
+        model->beginScrubbing();
 }
 
 void PlaybackSessionInterfaceMac::endScrubbing()
 {
-    playbackSessionModel()->endScrubbing();
+    if (auto* model = playbackSessionModel())
+        model->endScrubbing();
 }
 
@@ -215 +217 @@
 WebPlaybackControlsManager *PlaybackSessionInterfaceMac::playBackControlsManager()
 {
-    return m_playbackControlsManager;
+    return m_playbackControlsManager.getAutoreleased();
 }
 

trunk/Source/WebCore/platform/mac/VideoFullscreenInterfaceMac.mm

@@ -367 +367 @@
 VideoFullscreenInterfaceMac::~VideoFullscreenInterfaceMac()
 {
-    if (m_playbackSessionInterface->playbackSessionModel())
-        m_playbackSessionInterface->playbackSessionModel()->removeClient(*this);
+    if (auto* model = m_playbackSessionInterface->playbackSessionModel())
+        model->removeClient(*this);
     if (m_videoFullscreenModel)
         m_videoFullscreenModel->removeClient(*this);

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

@@ -1 +1 @@
 /*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
  * 
  * Redistribution and use in source and binary forms, with or without
@@ -95 +95 @@
     UNUSED_PARAM(toleranceBefore);
     UNUSED_PARAM(toleranceAfter);
-    _playbackSessionInterfaceMac->playbackSessionModel()->seekToTime(time);
+    if (auto* model = _playbackSessionInterfaceMac->playbackSessionModel())
+        model->seekToTime(time);
 }
 
@@ -194 +195 @@
         index = [_audioTouchBarMediaSelectionOptions indexOfObject:audioMediaSelectionOption];
 
-    _playbackSessionInterfaceMac->playbackSessionModel()->selectAudioMediaOption(index != NSNotFound ? index : UINT64_MAX);
+    if (auto* model = _playbackSessionInterfaceMac->playbackSessionModel())
+        model->selectAudioMediaOption(index != NSNotFound ? index : UINT64_MAX);
 }
 
@@ -224 +226 @@
         index = [_legibleTouchBarMediaSelectionOptions indexOfObject:legibleMediaSelectionOption];
 
-    _playbackSessionInterfaceMac->playbackSessionModel()->selectLegibleMediaOption(index != NSNotFound ? index : UINT64_MAX);
+    if (auto* model = _playbackSessionInterfaceMac->playbackSessionModel())
+        model->selectLegibleMediaOption(index != NSNotFound ? index : UINT64_MAX);
 }