Title: [278448] trunk/Source/WebCore
Revision
278448
Author
j...@apple.com
Date
2021-06-03 21:16:39 -0700 (Thu, 03 Jun 2021)

Log Message

fast/dom/Window/property-access-on-cached-window-after-frame-removed.html (layout-test) may crash
https://bugs.webkit.org/show_bug.cgi?id=226612
rdar://78846264

Reviewed by Eric Carlson.

It is possible under some circumstances for a MediaSession to be constructed
when no page or frame exists (such as when we move in/out bfcache).
The MediaSession constructor incorrectly only created the MediaSessionCoordinatorPrivate
if a Page and Frame existed.
To avoid any ambiguities on when MediaSession::m_coordinator could be set, we
make this member a const Ref<>.
Test is covered by fast/dom/Window/property-access-on-cached-window-after-frame-removed.html

* Modules/mediasession/MediaSession.cpp:
(WebCore::MediaSession::MediaSession): Create MediaSessionCoordinatorPrivate in initializer list.
(WebCore::MediaSession::suspend): Remove unnecessary null check
(WebCore::MediaSession::stop): Remove unnecessary null check
* Modules/mediasession/MediaSession.h: Make m_coordinator a const Ref<>
(WebCore::MediaSession::coordinator const):
* Modules/mediasession/MediaSessionCoordinator.cpp:
(WebCore::MediaSessionCoordinator::create): Remove MediaSessionCoordinatorPrivate argument to constructor.
(WebCore::MediaSessionCoordinator::MediaSessionCoordinator):
* Modules/mediasession/MediaSessionCoordinator.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (278447 => 278448)


--- trunk/Source/WebCore/ChangeLog	2021-06-04 03:45:04 UTC (rev 278447)
+++ trunk/Source/WebCore/ChangeLog	2021-06-04 04:16:39 UTC (rev 278448)
@@ -1,3 +1,30 @@
+2021-06-03  Jean-Yves Avenard  <j...@apple.com>
+
+        fast/dom/Window/property-access-on-cached-window-after-frame-removed.html (layout-test) may crash
+        https://bugs.webkit.org/show_bug.cgi?id=226612
+        rdar://78846264
+
+        Reviewed by Eric Carlson.
+
+        It is possible under some circumstances for a MediaSession to be constructed
+        when no page or frame exists (such as when we move in/out bfcache).
+        The MediaSession constructor incorrectly only created the MediaSessionCoordinatorPrivate
+        if a Page and Frame existed.
+        To avoid any ambiguities on when MediaSession::m_coordinator could be set, we
+        make this member a const Ref<>.
+        Test is covered by fast/dom/Window/property-access-on-cached-window-after-frame-removed.html
+
+        * Modules/mediasession/MediaSession.cpp:
+        (WebCore::MediaSession::MediaSession): Create MediaSessionCoordinatorPrivate in initializer list.
+        (WebCore::MediaSession::suspend): Remove unnecessary null check
+        (WebCore::MediaSession::stop): Remove unnecessary null check
+        * Modules/mediasession/MediaSession.h: Make m_coordinator a const Ref<>
+        (WebCore::MediaSession::coordinator const):
+        * Modules/mediasession/MediaSessionCoordinator.cpp:
+        (WebCore::MediaSessionCoordinator::create): Remove MediaSessionCoordinatorPrivate argument to constructor.
+        (WebCore::MediaSessionCoordinator::MediaSessionCoordinator):
+        * Modules/mediasession/MediaSessionCoordinator.h:
+
 2021-06-03  Alan Bujtas  <za...@apple.com>
 
         Crack in hero text on https://www.apple.com/mac-mini/

Modified: trunk/Source/WebCore/Modules/mediasession/MediaSession.cpp (278447 => 278448)


--- trunk/Source/WebCore/Modules/mediasession/MediaSession.cpp	2021-06-04 03:45:04 UTC (rev 278447)
+++ trunk/Source/WebCore/Modules/mediasession/MediaSession.cpp	2021-06-04 04:16:39 UTC (rev 278448)
@@ -136,6 +136,9 @@
 MediaSession::MediaSession(Navigator& navigator)
     : ActiveDOMObject(navigator.scriptExecutionContext())
     , m_navigator(makeWeakPtr(navigator))
+#if ENABLE(MEDIA_SESSION_COORDINATOR)
+    , m_coordinator(MediaSessionCoordinator::create(navigator.scriptExecutionContext()))
+#endif
 {
     m_logger = makeRefPtr(Document::sharedLogger());
     m_logIdentifier = nextLogIdentifier();
@@ -142,8 +145,10 @@
 
 #if ENABLE(MEDIA_SESSION_COORDINATOR)
     auto* frame = navigator.frame();
-    if (auto* page = frame ? frame->page() : nullptr)
-        createCoordinator(page->mediaSessionCoordinator());
+    auto* page = frame ? frame->page() : nullptr;
+    if (page && page->mediaSessionCoordinator())
+        m_coordinator->setMediaSessionCoordinatorPrivate(*page->mediaSessionCoordinator());
+    m_coordinator->setMediaSession(this);
 #endif
 
     ALWAYS_LOG(LOGIDENTIFIER);
@@ -154,7 +159,7 @@
 void MediaSession::suspend(ReasonForSuspension reason)
 {
 #if ENABLE(MEDIA_SESSION_COORDINATOR)
-    if (m_coordinator && reason == ReasonForSuspension::BackForwardCache)
+    if (reason == ReasonForSuspension::BackForwardCache)
         m_coordinator->leave();
 #else
     UNUSED_PARAM(reason);
@@ -164,8 +169,7 @@
 void MediaSession::stop()
 {
 #if ENABLE(MEDIA_SESSION_COORDINATOR)
-    if (m_coordinator)
-        m_coordinator->close();
+    m_coordinator->close();
 #endif
 }
 
@@ -191,17 +195,6 @@
     m_readyState = state;
     notifyReadyStateObservers();
 }
-
-void MediaSession::createCoordinator(MediaSessionCoordinatorPrivate* coordinatorPrivate)
-{
-    ALWAYS_LOG(LOGIDENTIFIER);
-
-    if (m_coordinator)
-        m_coordinator->setMediaSession(nullptr);
-
-    m_coordinator = MediaSessionCoordinator::create(scriptExecutionContext(), coordinatorPrivate);
-    m_coordinator->setMediaSession(this);
-}
 #endif
 
 #if ENABLE(MEDIA_SESSION_PLAYLIST)

Modified: trunk/Source/WebCore/Modules/mediasession/MediaSession.h (278447 => 278448)


--- trunk/Source/WebCore/Modules/mediasession/MediaSession.h	2021-06-04 03:45:04 UTC (rev 278447)
+++ trunk/Source/WebCore/Modules/mediasession/MediaSession.h	2021-06-04 04:16:39 UTC (rev 278448)
@@ -78,7 +78,7 @@
     MediaSessionReadyState readyState() const { return m_readyState; };
     void setReadyState(MediaSessionReadyState);
 
-    MediaSessionCoordinator& coordinator() const { return *m_coordinator; }
+    MediaSessionCoordinator& coordinator() const { return m_coordinator.get(); }
 #endif
 
 #if ENABLE(MEDIA_SESSION_PLAYLIST)
@@ -123,10 +123,6 @@
     void notifyActionHandlerObservers();
     void notifyReadyStateObservers();
 
-#if ENABLE(MEDIA_SESSION_COORDINATOR)
-    void createCoordinator(MediaSessionCoordinatorPrivate*);
-#endif
-
     // ActiveDOMObject
     const char* activeDOMObjectName() const final { return "MediaSession"; }
     void suspend(ReasonForSuspension) final;
@@ -148,7 +144,7 @@
 
 #if ENABLE(MEDIA_SESSION_COORDINATOR)
     MediaSessionReadyState m_readyState { MediaSessionReadyState::Havenothing };
-    RefPtr<MediaSessionCoordinator> m_coordinator;
+    const Ref<MediaSessionCoordinator> m_coordinator;
 #endif
 
 #if ENABLE(MEDIA_SESSION_PLAYLIST)

Modified: trunk/Source/WebCore/Modules/mediasession/MediaSessionCoordinator.cpp (278447 => 278448)


--- trunk/Source/WebCore/Modules/mediasession/MediaSessionCoordinator.cpp	2021-06-04 03:45:04 UTC (rev 278447)
+++ trunk/Source/WebCore/Modules/mediasession/MediaSessionCoordinator.cpp	2021-06-04 04:16:39 UTC (rev 278448)
@@ -48,14 +48,14 @@
     return reinterpret_cast<const void*>(++logIdentifier);
 }
 
-Ref<MediaSessionCoordinator> MediaSessionCoordinator::create(ScriptExecutionContext* context, RefPtr<MediaSessionCoordinatorPrivate>&& privateCoordinator)
+Ref<MediaSessionCoordinator> MediaSessionCoordinator::create(ScriptExecutionContext* context)
 {
-    auto coordinator = adoptRef(*new MediaSessionCoordinator(context, WTFMove(privateCoordinator)));
+    auto coordinator = adoptRef(*new MediaSessionCoordinator(context));
     coordinator->suspendIfNeeded();
     return coordinator;
 }
 
-MediaSessionCoordinator::MediaSessionCoordinator(ScriptExecutionContext* context, RefPtr<MediaSessionCoordinatorPrivate>&& privateCoordinator)
+MediaSessionCoordinator::MediaSessionCoordinator(ScriptExecutionContext* context)
     : ActiveDOMObject(context)
     , m_logger(makeRef(Document::sharedLogger()))
     , m_logIdentifier(nextCoordinatorLogIdentifier())
@@ -62,9 +62,6 @@
     , m_asyncEventQueue(MainThreadGenericEventQueue::create(*this))
 {
     ALWAYS_LOG(LOGIDENTIFIER);
-
-    if (privateCoordinator)
-        setMediaSessionCoordinatorPrivate(*privateCoordinator);
 }
 
 void MediaSessionCoordinator::setMediaSessionCoordinatorPrivate(Ref<MediaSessionCoordinatorPrivate>&& privateCoordinator)

Modified: trunk/Source/WebCore/Modules/mediasession/MediaSessionCoordinator.h (278447 => 278448)


--- trunk/Source/WebCore/Modules/mediasession/MediaSessionCoordinator.h	2021-06-04 03:45:04 UTC (rev 278447)
+++ trunk/Source/WebCore/Modules/mediasession/MediaSessionCoordinator.h	2021-06-04 04:16:39 UTC (rev 278448)
@@ -48,7 +48,7 @@
     , public EventTargetWithInlineData  {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    WEBCORE_EXPORT static Ref<MediaSessionCoordinator> create(ScriptExecutionContext*, RefPtr<MediaSessionCoordinatorPrivate>&&);
+    WEBCORE_EXPORT static Ref<MediaSessionCoordinator> create(ScriptExecutionContext*);
     WEBCORE_EXPORT ~MediaSessionCoordinator();
     WEBCORE_EXPORT void setMediaSessionCoordinatorPrivate(Ref<MediaSessionCoordinatorPrivate>&&);
 
@@ -72,7 +72,7 @@
     using RefCounted::deref;
 
 private:
-    MediaSessionCoordinator(ScriptExecutionContext*, RefPtr<MediaSessionCoordinatorPrivate>&&);
+    MediaSessionCoordinator(ScriptExecutionContext*);
 
     // EventTarget
     void refEventTarget() final { ref(); }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to