Release assert in ScriptController::canExecuteScripts via HTMLMediaElement::~HTMLMedi...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 May 2018 23:58:04 +0000 (23:58 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 May 2018 23:58:04 +0000 (23:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185288

Reviewed by Jer Noble.

The crash is caused by HTMLMediaElement::~HTMLMediaElement canceling the resource load via CachedResource
which ends up calling FrameLoader::checkCompleted() and fire load event on the document synchronously.
Speculatively fix the crash by scheduling the check instead.

In long term, ResourceLoader::cancel should never fire load event synchronously: webkit.org/b/185284.

Unfortunately, no new tests since I can't get MediaResource to get destructed at the right time.

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::isRunningDestructor): Added to detect this specific case.
(WebCore::HTMLMediaElementDestructorScope): Added.
(WebCore::HTMLMediaElementDestructorScope::HTMLMediaElementDestructorScope): Added.
(WebCore::HTMLMediaElementDestructorScope::~HTMLMediaElementDestructorScope): Added.
(WebCore::HTMLMediaElement::~HTMLMediaElement): Instantiate HTMLMediaElement.
* html/HTMLMediaElement.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::checkCompleted): Call scheduleCheckCompleted instead of synchronously calling
checkCompleted if we're in the middle of destructing a HTMLMediaElement.

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

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/html/HTMLMediaElement.h
Source/WebCore/loader/FrameLoader.cpp

index 270c593..90bade2 100644 (file)
@@ -1,3 +1,29 @@
+2018-05-03  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Release assert in ScriptController::canExecuteScripts via HTMLMediaElement::~HTMLMediaElement()
+        https://bugs.webkit.org/show_bug.cgi?id=185288
+
+        Reviewed by Jer Noble.
+
+        The crash is caused by HTMLMediaElement::~HTMLMediaElement canceling the resource load via CachedResource
+        which ends up calling FrameLoader::checkCompleted() and fire load event on the document synchronously.
+        Speculatively fix the crash by scheduling the check instead.
+
+        In long term, ResourceLoader::cancel should never fire load event synchronously: webkit.org/b/185284.
+
+        Unfortunately, no new tests since I can't get MediaResource to get destructed at the right time.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::isRunningDestructor): Added to detect this specific case.
+        (WebCore::HTMLMediaElementDestructorScope): Added.
+        (WebCore::HTMLMediaElementDestructorScope::HTMLMediaElementDestructorScope): Added.
+        (WebCore::HTMLMediaElementDestructorScope::~HTMLMediaElementDestructorScope): Added.
+        (WebCore::HTMLMediaElement::~HTMLMediaElement): Instantiate HTMLMediaElement.
+        * html/HTMLMediaElement.h:
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::checkCompleted): Call scheduleCheckCompleted instead of synchronously calling
+        checkCompleted if we're in the middle of destructing a HTMLMediaElement.
+
 2018-05-04  Ryosuke Niwa  <rniwa@webkit.org>
 
         Rename DocumentOrderedMap to TreeScopeOrderedMap
index 9078ae5..96cf8b9 100644 (file)
@@ -576,8 +576,23 @@ void HTMLMediaElement::finishInitialization()
     mediaSession().clientWillBeginAutoplaying();
 }
 
+// FIXME: Remove this code once https://webkit.org/b/185284 is fixed.
+static unsigned s_destructorCount = 0;
+
+bool HTMLMediaElement::isRunningDestructor()
+{
+    return !!s_destructorCount;
+}
+
+class HTMLMediaElementDestructorScope {
+public:
+    HTMLMediaElementDestructorScope() { ++s_destructorCount; }
+    ~HTMLMediaElementDestructorScope() { --s_destructorCount; }
+};
+
 HTMLMediaElement::~HTMLMediaElement()
 {
+    HTMLMediaElementDestructorScope destructorScope;
     ALWAYS_LOG(LOGIDENTIFIER);
 
     beginIgnoringTrackDisplayUpdateRequests();
index cef41c3..ba9f59a 100644 (file)
@@ -157,6 +157,8 @@ public:
 
     static HTMLMediaElement* bestMediaElementForShowingPlaybackControlsManager(MediaElementSession::PlaybackControlsPurpose);
 
+    static bool isRunningDestructor();
+
     WEBCORE_EXPORT void rewind(double timeDelta);
     WEBCORE_EXPORT void returnToRealtime() override;
 
index e3eee58..606a0f1 100644 (file)
@@ -805,6 +805,13 @@ void FrameLoader::checkCompleted()
     // Have we completed before?
     if (m_isComplete)
         return;
+    
+    // FIXME: Remove this code once https://webkit.org/b/185284 is fixed.
+    if (HTMLMediaElement::isRunningDestructor()) {
+        ASSERT_NOT_REACHED();
+        scheduleCheckCompleted();
+        return;
+    }
 
     // FIXME: It would be better if resource loads were kicked off after render tree update (or didn't complete synchronously).
     //        https://bugs.webkit.org/show_bug.cgi?id=171729