Changeset 236446 in webkit

Timestamp:

Sep 24, 2018 5:28:42 PM (6 years ago)

Author:

rniwa@webkit.org

Message:

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/CustomElementReactionQueue.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/dom/CustomElementReactionQueue.cpp

@@ -226 +226 @@
 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);
@@ -235 +235 @@
     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());
-    }
-    RELEASE_ASSERT(m_elements.size() == originalSize);
+        queue->invokeAll(element);
+    }
+    ASSERT_UNUSED(originalSize, m_elements.size() == originalSize);
     m_elements.clear();
 }