Don't cause a crash even when some IDL attribute is missing CEReactions
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Sep 2018 00:28:42 +0000 (00:28 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Sep 2018 00:28:42 +0000 (00:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189937

Reviewed by Simon Fraser.

Replaced release assertions in ElementQueue::add and ElementQueue::invokeAll by debug assertions
since a missing CEReactions resulting in a crash is a terrible user experience.

Also made the iteration in invokeAll safe when more elements were added to m_elements.

No new tests since we would still hit debug assertions, and this behavior should only come up
when some IDL attribute is erroneously missing CEReactions.

* dom/CustomElementReactionQueue.cpp:
(WebCore::CustomElementReactionQueue::ElementQueue::add):
(WebCore::CustomElementReactionQueue::ElementQueue::invokeAll):

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

Source/WebCore/ChangeLog
Source/WebCore/dom/CustomElementReactionQueue.cpp

index 7779461..22c8cc3 100644 (file)
@@ -1,3 +1,22 @@
+2018-09-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Don't cause a crash even when some IDL attribute is missing CEReactions
+        https://bugs.webkit.org/show_bug.cgi?id=189937
+
+        Reviewed by Simon Fraser.
+
+        Replaced release assertions in ElementQueue::add and ElementQueue::invokeAll by debug assertions
+        since a missing CEReactions resulting in a crash is a terrible user experience.
+
+        Also made the iteration in invokeAll safe when more elements were added to m_elements.
+
+        No new tests since we would still hit debug assertions, and this behavior should only come up
+        when some IDL attribute is erroneously missing CEReactions.
+
+        * dom/CustomElementReactionQueue.cpp:
+        (WebCore::CustomElementReactionQueue::ElementQueue::add):
+        (WebCore::CustomElementReactionQueue::ElementQueue::invokeAll):
+
 2018-09-24  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Refactor Editor::fontAttributesForSelectionStart to be platform-agnostic
index 243a806..4007c47 100644 (file)
@@ -225,7 +225,7 @@ void CustomElementReactionQueue::invokeAll(Element& element)
 
 inline void CustomElementReactionQueue::ElementQueue::add(Element& element)
 {
-    RELEASE_ASSERT(!m_invoking);
+    ASSERT(!m_invoking);
     // FIXME: Avoid inserting the same element multiple times.
     m_elements.append(element);
 }
@@ -234,13 +234,16 @@ inline void CustomElementReactionQueue::ElementQueue::invokeAll()
 {
     RELEASE_ASSERT(!m_invoking);
     SetForScope<bool> invoking(m_invoking, true);
-    auto originalSize = m_elements.size();
-    for (auto& element : m_elements) {
-        auto* queue = element->reactionQueue();
+    unsigned originalSize = m_elements.size();
+    // It's possible for more elements to be enqueued if some IDL attributes were missing CEReactions.
+    // Invoke callbacks slightly later here instead of crashing / ignoring those cases.
+    for (unsigned i = 0; i < m_elements.size(); ++i) {
+        auto& element = m_elements[i].get();
+        auto* queue = element.reactionQueue();
         ASSERT(queue);
-        queue->invokeAll(element.get());
+        queue->invokeAll(element);
     }
-    RELEASE_ASSERT(m_elements.size() == originalSize);
+    ASSERT_UNUSED(originalSize, m_elements.size() == originalSize);
     m_elements.clear();
 }