Hardening: Use WeakPtrs in VideoFullscreenInterface{Mac,AVKit}
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 17:21:38 +0000 (17:21 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 17:21:38 +0000 (17:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196052
<rdar://problem/48778571>

Reviewed by Eric Carlson.

The VideoFullscreenInterface{Mac,AVKit} implementations store their fullscreen model
and fullscreen change observer 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/VideoFullscreenChangeObserver.h:
* platform/cocoa/VideoFullscreenModel.h:
* platform/ios/VideoFullscreenInterfaceAVKit.h:
* platform/ios/VideoFullscreenInterfaceAVKit.mm:
(VideoFullscreenInterfaceAVKit::setVideoFullscreenModel):
(VideoFullscreenInterfaceAVKit::setVideoFullscreenChangeObserver):
(VideoFullscreenInterfaceAVKit::presentingViewController):
(VideoFullscreenInterfaceAVKit::invalidate):
(VideoFullscreenInterfaceAVKit::preparedToExitFullscreen):
(VideoFullscreenInterfaceAVKit::shouldExitFullscreenWithReason):
(VideoFullscreenInterfaceAVKit::doSetup):
* platform/mac/VideoFullscreenInterfaceMac.h:
(WebCore::VideoFullscreenInterfaceMac::videoFullscreenModel const):
(WebCore::VideoFullscreenInterfaceMac::videoFullscreenChangeObserver const):
* platform/mac/VideoFullscreenInterfaceMac.mm:
(WebCore::VideoFullscreenInterfaceMac::setVideoFullscreenModel):
(WebCore::VideoFullscreenInterfaceMac::setVideoFullscreenChangeObserver):
(WebCore::VideoFullscreenInterfaceMac::enterFullscreen):
(WebCore::VideoFullscreenInterfaceMac::invalidate):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243298 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/cocoa/VideoFullscreenChangeObserver.h
Source/WebCore/platform/cocoa/VideoFullscreenModel.h
Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.h
Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm
Source/WebCore/platform/mac/VideoFullscreenInterfaceMac.h
Source/WebCore/platform/mac/VideoFullscreenInterfaceMac.mm

index 75be131..ff5194f 100644 (file)
@@ -1,3 +1,39 @@
+2019-03-21  Brent Fulgham  <bfulgham@apple.com>
+
+        Hardening: Use WeakPtrs in VideoFullscreenInterface{Mac,AVKit}
+        https://bugs.webkit.org/show_bug.cgi?id=196052
+        <rdar://problem/48778571>
+
+        Reviewed by Eric Carlson.
+
+        The VideoFullscreenInterface{Mac,AVKit} implementations store their fullscreen model
+        and fullscreen change observer 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/VideoFullscreenChangeObserver.h:
+        * platform/cocoa/VideoFullscreenModel.h:
+        * platform/ios/VideoFullscreenInterfaceAVKit.h:
+        * platform/ios/VideoFullscreenInterfaceAVKit.mm:
+        (VideoFullscreenInterfaceAVKit::setVideoFullscreenModel):
+        (VideoFullscreenInterfaceAVKit::setVideoFullscreenChangeObserver):
+        (VideoFullscreenInterfaceAVKit::presentingViewController):
+        (VideoFullscreenInterfaceAVKit::invalidate):
+        (VideoFullscreenInterfaceAVKit::preparedToExitFullscreen):
+        (VideoFullscreenInterfaceAVKit::shouldExitFullscreenWithReason):
+        (VideoFullscreenInterfaceAVKit::doSetup):
+        * platform/mac/VideoFullscreenInterfaceMac.h:
+        (WebCore::VideoFullscreenInterfaceMac::videoFullscreenModel const):
+        (WebCore::VideoFullscreenInterfaceMac::videoFullscreenChangeObserver const):
+        * platform/mac/VideoFullscreenInterfaceMac.mm:
+        (WebCore::VideoFullscreenInterfaceMac::setVideoFullscreenModel):
+        (WebCore::VideoFullscreenInterfaceMac::setVideoFullscreenChangeObserver):
+        (WebCore::VideoFullscreenInterfaceMac::enterFullscreen):
+        (WebCore::VideoFullscreenInterfaceMac::invalidate):
+
 2019-03-21  Megan Gardner  <megan_gardner@apple.com>
 
         Smart delete for paragraphs.
index bb74838..6e8152d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 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
  * modification, are permitted provided that the following conditions
@@ -30,7 +30,7 @@
 
 namespace WebCore {
 
-class VideoFullscreenChangeObserver {
+class VideoFullscreenChangeObserver : public CanMakeWeakPtr<VideoFullscreenChangeObserver> {
 public:
     virtual ~VideoFullscreenChangeObserver() { };
     virtual void requestUpdateInlineRect() = 0;
index f671490..f5829c2 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 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
  * modification, are permitted provided that the following conditions
@@ -44,7 +44,7 @@ namespace WebCore {
 
 class VideoFullscreenModelClient;
 
-class VideoFullscreenModel {
+class VideoFullscreenModel : public CanMakeWeakPtr<VideoFullscreenModel> {
 public:
     virtual ~VideoFullscreenModel() { };
     virtual void addClient(VideoFullscreenModelClient&) = 0;
index 477ee4e..5d494f8 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014-2018 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
  * modification, are permitted provided that the following conditions
@@ -127,7 +127,7 @@ public:
     Mode m_currentMode;
     Mode m_targetMode;
 
-    VideoFullscreenModel* videoFullscreenModel() const { return m_videoFullscreenModel; }
+    VideoFullscreenModel* videoFullscreenModel() const { return m_videoFullscreenModel.get(); }
     bool shouldExitFullscreenWithReason(ExitFullScreenReason);
     HTMLMediaElementEnums::VideoFullscreenMode mode() const { return m_currentMode.mode(); }
     bool allowsPictureInPicturePlayback() const { return m_allowsPictureInPicturePlayback; }
@@ -171,8 +171,8 @@ protected:
     Ref<PlaybackSessionInterfaceAVKit> m_playbackSessionInterface;
     RetainPtr<WebAVPlayerViewControllerDelegate> m_playerViewControllerDelegate;
     RetainPtr<WebAVPlayerViewController> m_playerViewController;
-    VideoFullscreenModel* m_videoFullscreenModel { nullptr };
-    VideoFullscreenChangeObserver* m_fullscreenChangeObserver { nullptr };
+    WeakPtr<VideoFullscreenModel> m_videoFullscreenModel;
+    WeakPtr<VideoFullscreenChangeObserver> m_fullscreenChangeObserver;
 
     // These are only used when fullscreen is presented in a separate window.
     RetainPtr<UIWindow> m_window;
index 9f1ba79..08724eb 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014-2018 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
  * modification, are permitted provided that the following conditions
@@ -763,7 +763,7 @@ void VideoFullscreenInterfaceAVKit::setVideoFullscreenModel(VideoFullscreenModel
     if (m_videoFullscreenModel)
         m_videoFullscreenModel->removeClient(*this);
 
-    m_videoFullscreenModel = model;
+    m_videoFullscreenModel = makeWeakPtr(model);
 
     if (m_videoFullscreenModel) {
         m_videoFullscreenModel->addClient(*this);
@@ -782,7 +782,7 @@ void VideoFullscreenInterfaceAVKit::setVideoFullscreenModel(VideoFullscreenModel
 
 void VideoFullscreenInterfaceAVKit::setVideoFullscreenChangeObserver(VideoFullscreenChangeObserver* observer)
 {
-    m_fullscreenChangeObserver = observer;
+    m_fullscreenChangeObserver = makeWeakPtr(observer);
 }
 
 void VideoFullscreenInterfaceAVKit::hasVideoChanged(bool hasVideo)
@@ -833,7 +833,7 @@ static UIViewController *fallbackViewController(UIView *view)
 
 UIViewController *VideoFullscreenInterfaceAVKit::presentingViewController()
 {
-    auto *controller = videoFullscreenModel()->presentingViewController();
+    auto *controller = videoFullscreenModel() ? videoFullscreenModel()->presentingViewController() : nil;
     if (!controller)
         controller = fallbackViewController(m_parentView.get());
 
@@ -927,8 +927,8 @@ void VideoFullscreenInterfaceAVKit::cleanupFullscreen()
 
 void VideoFullscreenInterfaceAVKit::invalidate()
 {
-    m_videoFullscreenModel = nil;
-    m_fullscreenChangeObserver = nil;
+    m_videoFullscreenModel = nullptr;
+    m_fullscreenChangeObserver = nullptr;
     
     cleanupFullscreen();
 }
@@ -966,7 +966,9 @@ void VideoFullscreenInterfaceAVKit::preparedToExitFullscreen()
         return;
 
     m_waitingForPreparedToExit = false;
-    m_videoFullscreenModel->requestFullscreenMode(HTMLMediaElementEnums::VideoFullscreenModeNone, true);
+    ASSERT(m_videoFullscreenModel);
+    if (m_videoFullscreenModel)
+        m_videoFullscreenModel->requestFullscreenMode(HTMLMediaElementEnums::VideoFullscreenModeNone, true);
 #endif
 }
 
@@ -1135,7 +1137,9 @@ bool VideoFullscreenInterfaceAVKit::shouldExitFullscreenWithReason(VideoFullscre
 #endif
     
     BOOL finished = reason == ExitFullScreenReason::DoneButtonTapped || reason == ExitFullScreenReason::PinchGestureHandled;
-    m_videoFullscreenModel->requestFullscreenMode(HTMLMediaElementEnums::VideoFullscreenModeNone, finished);
+    ASSERT(m_videoFullscreenModel);
+    if (m_videoFullscreenModel)
+        m_videoFullscreenModel->requestFullscreenMode(HTMLMediaElementEnums::VideoFullscreenModeNone, finished);
 
     return false;
 }
@@ -1242,7 +1246,7 @@ void VideoFullscreenInterfaceAVKit::doSetup()
         [m_playerViewController setWebKitOverrideRouteSharingPolicy:(NSUInteger)m_routeSharingPolicy routingContextUID:m_routingContextUID];
 
 #if PLATFORM(WATCHOS)
-    m_viewController = videoFullscreenModel()->createVideoFullscreenViewController(m_playerViewController.get().avPlayerViewController);
+    m_viewController = videoFullscreenModel() ? videoFullscreenModel()->createVideoFullscreenViewController(m_playerViewController.get().avPlayerViewController) : nil;
 #endif
 
     if (m_viewController) {
index af2c649..db5d863 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 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
  * modification, are permitted provided that the following conditions
@@ -56,10 +56,10 @@ public:
     }
     virtual ~VideoFullscreenInterfaceMac();
     PlaybackSessionInterfaceMac& playbackSessionInterface() const { return m_playbackSessionInterface.get(); }
-    VideoFullscreenModel* videoFullscreenModel() const { return m_videoFullscreenModel; }
+    VideoFullscreenModel* videoFullscreenModel() const { return m_videoFullscreenModel.get(); }
     PlaybackSessionModel* playbackSessionModel() const { return m_playbackSessionInterface->playbackSessionModel(); }
     WEBCORE_EXPORT void setVideoFullscreenModel(VideoFullscreenModel*);
-    VideoFullscreenChangeObserver* videoFullscreenChangeObserver() const { return m_fullscreenChangeObserver; }
+    VideoFullscreenChangeObserver* videoFullscreenChangeObserver() const { return m_fullscreenChangeObserver.get(); }
     WEBCORE_EXPORT void setVideoFullscreenChangeObserver(VideoFullscreenChangeObserver*);
 
     // PlaybackSessionModelClient
@@ -99,8 +99,8 @@ public:
 private:
     WEBCORE_EXPORT VideoFullscreenInterfaceMac(PlaybackSessionInterfaceMac&);
     Ref<PlaybackSessionInterfaceMac> m_playbackSessionInterface;
-    VideoFullscreenModel* m_videoFullscreenModel { nullptr };
-    VideoFullscreenChangeObserver* m_fullscreenChangeObserver { nullptr };
+    WeakPtr<VideoFullscreenModel> m_videoFullscreenModel;
+    WeakPtr<VideoFullscreenChangeObserver> m_fullscreenChangeObserver;
     HTMLMediaElementEnums::VideoFullscreenMode m_mode { HTMLMediaElementEnums::VideoFullscreenModeNone };
     RetainPtr<WebVideoFullscreenInterfaceMacObjC> m_webVideoFullscreenInterfaceObjC;
 };
index 6e981d2..41818a5 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 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
  * modification, are permitted provided that the following conditions
@@ -376,14 +376,14 @@ void VideoFullscreenInterfaceMac::setVideoFullscreenModel(VideoFullscreenModel*
 {
     if (m_videoFullscreenModel)
         m_videoFullscreenModel->removeClient(*this);
-    m_videoFullscreenModel = model;
+    m_videoFullscreenModel = makeWeakPtr(model);
     if (m_videoFullscreenModel)
         m_videoFullscreenModel->addClient(*this);
 }
 
 void VideoFullscreenInterfaceMac::setVideoFullscreenChangeObserver(VideoFullscreenChangeObserver* observer)
 {
-    m_fullscreenChangeObserver = observer;
+    m_fullscreenChangeObserver = makeWeakPtr(observer);
 }
 
 void VideoFullscreenInterfaceMac::setMode(HTMLMediaElementEnums::VideoFullscreenMode mode)
@@ -447,6 +447,7 @@ void VideoFullscreenInterfaceMac::enterFullscreen()
 {
     LOG(Fullscreen, "VideoFullscreenInterfaceMac::enterFullscreen(%p)", this);
 
+    RELEASE_ASSERT(m_videoFullscreenModel);
     if (mode() == HTMLMediaElementEnums::VideoFullscreenModePictureInPicture) {
         m_videoFullscreenModel->willEnterPictureInPicture();
         [m_webVideoFullscreenInterfaceObjC enterPIP];
@@ -510,8 +511,8 @@ void VideoFullscreenInterfaceMac::invalidate()
 {
     LOG(Fullscreen, "VideoFullscreenInterfaceMac::invalidate(%p)", this);
 
-    m_videoFullscreenModel = nil;
-    m_fullscreenChangeObserver = nil;
+    m_videoFullscreenModel = nullptr;
+    m_fullscreenChangeObserver = nullptr;
 
     cleanupFullscreen();