Changeset 202590 in webkit

Timestamp:

Jun 28, 2016 2:35:37 PM (8 years ago)

Author:

ggaren@apple.com

Message:

CrashTracer beneath JSC::MarkedBlock::specializedSweep
​https://bugs.webkit.org/show_bug.cgi?id=159223

Reviewed by Saam Barati.

This crash is caused by a media element re-entering JS during the GC
sweep phase.

In theory, other CachedResourceClients in the DOM might also trigger
similar bugs, but our data only implicates the media elements, so this
fix targets them.

• html/HTMLDocument.h: Document has no reason to inherit from

CachedResourceClient. I found this becuase I had to search for all
CachedResourceClients in researching this patch.

• platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp:

(WebCore::WebCoreAVCFResourceLoader::invalidate): Delay our call to
stopLoading because it might re-enter JS, and we might have been called
by the GC sweep phase destroying a media element.

• platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.mm:

(WebCore::WebCoreAVFResourceLoader::invalidate): Ditto.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/HTMLDocument.h (modified) (2 diffs)
• platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp (modified) (1 diff)
• platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-06-28  Geoffrey Garen  <ggaren@apple.com>
+
+        CrashTracer beneath JSC::MarkedBlock::specializedSweep
+        https://bugs.webkit.org/show_bug.cgi?id=159223
+
+        Reviewed by Saam Barati.
+
+        This crash is caused by a media element re-entering JS during the GC
+        sweep phase.
+
+        In theory, other CachedResourceClients in the DOM might also trigger
+        similar bugs, but our data only implicates the media elements, so this
+        fix targets them.
+
+        * html/HTMLDocument.h: Document has no reason to inherit from
+        CachedResourceClient. I found this becuase I had to search for all
+        CachedResourceClients in researching this patch.
+
+        * platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp:
+        (WebCore::WebCoreAVCFResourceLoader::invalidate): Delay our call to
+        stopLoading because it might re-enter JS, and we might have been called
+        by the GC sweep phase destroying a media element.
+
+        * platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.mm:
+        (WebCore::WebCoreAVFResourceLoader::invalidate): Ditto.
+
 2016-06-28  Saam Barati  <sbarati@apple.com>
 

trunk/Source/WebCore/html/HTMLDocument.h

@@ -24 +24 @@
 #define HTMLDocument_h
 
-#include "CachedResourceClient.h"
 #include "Document.h"
 #include <wtf/HashCountedSet.h>
@@ -30 +29 @@
 namespace WebCore {
 
-class HTMLDocument : public Document, public CachedResourceClient {
+class HTMLDocument : public Document {
 public:
     static Ref<HTMLDocument> create(Frame* frame, const URL& url)

trunk/Source/WebCore/platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp

@@ -100 +100 @@
 void WebCoreAVCFResourceLoader::invalidate()
 {
+    if (!m_parent)
+        return;
+
     m_parent = nullptr;
-    stopLoading();
+
+    callOnMainThread([protectedThis = Ref<WebCoreAVCFResourceLoader>(*this)] () mutable {
+        protectedThis->stopLoading();
+    });
 }
 

trunk/Source/WebCore/platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.mm

@@ -97 +97 @@
 void WebCoreAVFResourceLoader::invalidate()
 {
+    if (!m_parent)
+        return;
+
     m_parent = nullptr;
-    stopLoading();
+
+    callOnMainThread([protectedThis = Ref<WebCoreAVFResourceLoader>(*this)] () mutable {
+        protectedThis->stopLoading();
+    });
 }