Turn assertion preventing removal of ActiveDOMObjects while we iterate over them...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jul 2018 16:12:43 +0000 (16:12 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jul 2018 16:12:43 +0000 (16:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187978

Reviewed by Eric Carlson.

Turn assertion preventing removal of ActiveDOMObjects while we iterate over them into a RELEASE_ASSERT.
If code does this, this leads to hard to investigate crashes such as rdar://problem/42160890. With a
release assertion, we would find the culprits right away. The assertion guarding against addition of
ActiveDOMObjects while we iterate is already a RELEASE_ASSERT.

* dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::canSuspendActiveDOMObjectsForDocumentSuspension):
(WebCore::ScriptExecutionContext::suspendActiveDOMObjects):
(WebCore::ScriptExecutionContext::resumeActiveDOMObjects):
(WebCore::ScriptExecutionContext::willDestroyActiveDOMObject):
* dom/ScriptExecutionContext.h:

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

Source/WebCore/ChangeLog
Source/WebCore/dom/ScriptExecutionContext.cpp
Source/WebCore/dom/ScriptExecutionContext.h

index c3399d8..28fb29f 100644 (file)
@@ -1,3 +1,22 @@
+2018-07-25  Chris Dumez  <cdumez@apple.com>
+
+        Turn assertion preventing removal of ActiveDOMObjects while we iterate over them into a RELEASE_ASSERT
+        https://bugs.webkit.org/show_bug.cgi?id=187978
+
+        Reviewed by Eric Carlson.
+
+        Turn assertion preventing removal of ActiveDOMObjects while we iterate over them into a RELEASE_ASSERT.
+        If code does this, this leads to hard to investigate crashes such as rdar://problem/42160890. With a
+        release assertion, we would find the culprits right away. The assertion guarding against addition of
+        ActiveDOMObjects while we iterate is already a RELEASE_ASSERT.
+
+        * dom/ScriptExecutionContext.cpp:
+        (WebCore::ScriptExecutionContext::canSuspendActiveDOMObjectsForDocumentSuspension):
+        (WebCore::ScriptExecutionContext::suspendActiveDOMObjects):
+        (WebCore::ScriptExecutionContext::resumeActiveDOMObjects):
+        (WebCore::ScriptExecutionContext::willDestroyActiveDOMObject):
+        * dom/ScriptExecutionContext.h:
+
 2018-07-24  Chris Dumez  <cdumez@apple.com>
 
         REGRESSION (r219757): Accessing response getter of XHR instance from IFRAME sets constructor to Object from the IFRAME
index eb62b16..4394494 100644 (file)
@@ -229,9 +229,7 @@ bool ScriptExecutionContext::canSuspendActiveDOMObjectsForDocumentSuspension(Vec
     bool canSuspend = true;
 
     m_activeDOMObjectAdditionForbidden = true;
-#if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
     m_activeDOMObjectRemovalForbidden = true;
-#endif
 
     // We assume that m_activeDOMObjects will not change during iteration: canSuspend
     // functions should not add new active DOM objects, nor execute arbitrary JavaScript.
@@ -249,9 +247,7 @@ bool ScriptExecutionContext::canSuspendActiveDOMObjectsForDocumentSuspension(Vec
     }
 
     m_activeDOMObjectAdditionForbidden = false;
-#if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
     m_activeDOMObjectRemovalForbidden = false;
-#endif
 
     return canSuspend;
 }
@@ -271,9 +267,7 @@ void ScriptExecutionContext::suspendActiveDOMObjects(ReasonForSuspension why)
     m_activeDOMObjectsAreSuspended = true;
 
     m_activeDOMObjectAdditionForbidden = true;
-#if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
     m_activeDOMObjectRemovalForbidden = true;
-#endif
 
     // We assume that m_activeDOMObjects will not change during iteration: suspend
     // functions should not add new active DOM objects, nor execute arbitrary JavaScript.
@@ -284,9 +278,7 @@ void ScriptExecutionContext::suspendActiveDOMObjects(ReasonForSuspension why)
         activeDOMObject->suspend(why);
 
     m_activeDOMObjectAdditionForbidden = false;
-#if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
     m_activeDOMObjectRemovalForbidden = false;
-#endif
 
     m_reasonForSuspendingActiveDOMObjects = why;
 }
@@ -300,9 +292,7 @@ void ScriptExecutionContext::resumeActiveDOMObjects(ReasonForSuspension why)
     m_activeDOMObjectsAreSuspended = false;
 
     m_activeDOMObjectAdditionForbidden = true;
-#if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
     m_activeDOMObjectRemovalForbidden = true;
-#endif
 
     // We assume that m_activeDOMObjects will not change during iteration: resume
     // functions should not add new active DOM objects, nor execute arbitrary JavaScript.
@@ -313,9 +303,7 @@ void ScriptExecutionContext::resumeActiveDOMObjects(ReasonForSuspension why)
         activeDOMObject->resume();
 
     m_activeDOMObjectAdditionForbidden = false;
-#if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
     m_activeDOMObjectRemovalForbidden = false;
-#endif
 }
 
 void ScriptExecutionContext::stopActiveDOMObjects()
@@ -371,7 +359,7 @@ void ScriptExecutionContext::didCreateActiveDOMObject(ActiveDOMObject& activeDOM
 
 void ScriptExecutionContext::willDestroyActiveDOMObject(ActiveDOMObject& activeDOMObject)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(!m_activeDOMObjectRemovalForbidden);
+    RELEASE_ASSERT(!m_activeDOMObjectRemovalForbidden);
     m_activeDOMObjects.remove(&activeDOMObject);
 }
 
index 0724eb5..224b3af 100644 (file)
@@ -321,14 +321,12 @@ private:
     bool m_activeDOMObjectsAreStopped { false };
     bool m_inDispatchErrorEvent { false };
     bool m_activeDOMObjectAdditionForbidden { false };
+    bool m_activeDOMObjectRemovalForbidden { false };
     bool m_willprocessMessageWithMessagePortsSoon { false };
 
 #if !ASSERT_DISABLED
     bool m_inScriptExecutionContextDestructor { false };
 #endif
-#if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
-    bool m_activeDOMObjectRemovalForbidden { false };
-#endif
 
 #if ENABLE(SERVICE_WORKER)
     RefPtr<ServiceWorker> m_activeServiceWorker;