Changeset 209582 in webkit


Ignore:
Timestamp:
Dec 8, 2016 4:53:32 PM (7 years ago)
Author:
rniwa@webkit.org
Message:

ASSERTION FAILED: m_items.isEmpty() in CustomElementReactionQueue destructor
​https://bugs.webkit.org/show_bug.cgi?id=162029
<rdar://problem/28945851>

Reviewed by Chris Dumez.

Source/WebCore:

The bug was caused by Document::removedLastRef enqueuing disconnectedCallback during a tear down.
Don't enqueue a disconnectedCallback while a document is getting torn down since that should not be
observable to author scripts. The connected, adopted, and attributeChanged callbacks are immune from
this problem since they don't happen during a document destruction.

Note that this was also the case prior to this patch since the disconnectedCallback would have been
added to the current CustomElementReactionQueue which will be destructed without invoking callbacks
(or hit a release assertion added in r208785 and r209426 for now).

Tests: fast/custom-elements/disconnected-callback-in-detached-iframe.html

fast/custom-elements/element-queue-during-document-destruction.html

  • dom/CustomElementReactionQueue.cpp:

(WebCore::CustomElementReactionQueue::enqueueConnectedCallbackIfNeeded): Added an assertion that
document's refCount hasn't reached zero yet.
(WebCore::CustomElementReactionQueue::enqueueDisconnectedCallbackIfNeeded): Fixed the bug.
(WebCore::CustomElementReactionQueue::enqueueAdoptedCallbackIfNeeded): Added the same assertion.
(WebCore::CustomElementReactionQueue::enqueueAttributeChangedCallbackIfNeeded): Ditto.

LayoutTests:

Added a regression test that reliably reproduces the crash in DumpRenderTree / WebKitTestRunner.

Also added a W3C style testharness.js test for the behavior I broke in an earlier iteration of the patch.

  • fast/custom-elements/disconnected-callback-in-detached-iframe-expected.txt: Added.
  • fast/custom-elements/disconnected-callback-in-detached-iframe.html: Added.
  • fast/custom-elements/element-queue-during-document-destruction-expected.txt: Added.
  • fast/custom-elements/element-queue-during-document-destruction.html: Added.
Location:
trunk
Files:
4 added
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r209580 r209582  
     12016-12-07  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        ASSERTION FAILED: m_items.isEmpty() in CustomElementReactionQueue destructor
     4        https://bugs.webkit.org/show_bug.cgi?id=162029
     5        <rdar://problem/28945851>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Added a regression test that reliably reproduces the crash in DumpRenderTree / WebKitTestRunner.
     10
     11        Also added a W3C style testharness.js test for the behavior I broke in an earlier iteration of the patch.
     12
     13        * fast/custom-elements/disconnected-callback-in-detached-iframe-expected.txt: Added.
     14        * fast/custom-elements/disconnected-callback-in-detached-iframe.html: Added.
     15        * fast/custom-elements/element-queue-during-document-destruction-expected.txt: Added.
     16        * fast/custom-elements/element-queue-during-document-destruction.html: Added.
     17
    1182016-12-08  Ryan Haddad  <ryanhaddad@apple.com>
    219

  • trunk/Source/WebCore/ChangeLog

    r209578 r209582  
     12016-12-07  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        ASSERTION FAILED: m_items.isEmpty() in CustomElementReactionQueue destructor
     4        https://bugs.webkit.org/show_bug.cgi?id=162029
     5        <rdar://problem/28945851>
     6
     7        Reviewed by Chris Dumez.
     8
     9        The bug was caused by Document::removedLastRef enqueuing disconnectedCallback during a tear down.
     10        Don't enqueue a disconnectedCallback while a document is getting torn down since that should not be
     11        observable to author scripts. The connected, adopted, and attributeChanged callbacks are immune from
     12        this problem since they don't happen during a document destruction.
     13
     14        Note that this was also the case prior to this patch since the disconnectedCallback would have been
     15        added to the current CustomElementReactionQueue which will be destructed without invoking callbacks
     16        (or hit a release assertion added in r208785 and r209426 for now).
     17
     18        Tests: fast/custom-elements/disconnected-callback-in-detached-iframe.html
     19               fast/custom-elements/element-queue-during-document-destruction.html
     20
     21        * dom/CustomElementReactionQueue.cpp:
     22        (WebCore::CustomElementReactionQueue::enqueueConnectedCallbackIfNeeded): Added an assertion that
     23        document's refCount hasn't reached zero yet.
     24        (WebCore::CustomElementReactionQueue::enqueueDisconnectedCallbackIfNeeded): Fixed the bug.
     25        (WebCore::CustomElementReactionQueue::enqueueAdoptedCallbackIfNeeded): Added the same assertion.
     26        (WebCore::CustomElementReactionQueue::enqueueAttributeChangedCallbackIfNeeded): Ditto.
     27
    1282016-12-08  Daniel Bates  <dabates@apple.com>
    229

  • trunk/Source/WebCore/dom/CustomElementReactionQueue.cpp

    r209426 r209582  
    142142{
    143143    ASSERT(element.isDefinedCustomElement());
     144    ASSERT(element.document().refCount() > 0);
    144145    auto& queue = CustomElementReactionStack::ensureCurrentQueue(element);
    145146    if (queue.m_interface->hasConnectedCallback())
    … …  
    150151{
    151152    ASSERT(element.isDefinedCustomElement());
     153    if (element.document().refCount() <= 0)
     154        return; // Don't enqueue disconnectedCallback if the entire document is getting destructed.
    152155    auto& queue = CustomElementReactionStack::ensureCurrentQueue(element);
    153156    if (queue.m_interface->hasDisconnectedCallback())
    … …  
    158161{
    159162    ASSERT(element.isDefinedCustomElement());
     163    ASSERT(element.document().refCount() > 0);
    160164    auto& queue = CustomElementReactionStack::ensureCurrentQueue(element);
    161165    if (queue.m_interface->hasAdoptedCallback())
    … …  
    166170{
    167171    ASSERT(element.isDefinedCustomElement());
     172    ASSERT(element.document().refCount() > 0);
    168173    auto& queue = CustomElementReactionStack::ensureCurrentQueue(element);
    169174    if (queue.m_interface->observesAttribute(attributeName.localName()))
Note: See TracChangeset for help on using the changeset viewer.