Changeset 234577 in webkit


Ignore:
Timestamp:
Aug 3, 2018 11:35:37 PM (6 years ago)
Author:
rniwa@webkit.org
Message:

innerHTML should not synchronously create a custom element
​https://bugs.webkit.org/show_bug.cgi?id=188327
<rdar://problem/42923114>

Reviewed by Daniel Bates.

LayoutTests/imported/w3c:

Rebaselined the test now that all test cases are passing.

  • web-platform-tests/custom-elements/connected-callbacks-html-fragment-parsing-expected.txt:

Source/WebCore:

Fixed the bug that the fragment parsing algorithm was synchronously constructing a custom element instead of
enqueuing an element to upgrade.

The fragment parsing algorithm creates an element for a token with *will execute script* flag set to false:
​https://html.spec.whatwg.org/multipage/parsing.html#create-an-element-for-the-token
which results in creating an element with synchronous custom elements flag *not* set:
​https://dom.spec.whatwg.org/#concept-create-element

When synchronous custom elements flag is false, we're supposed to create an element and enqueue a custom element
upgrade reaction. createHTMLElementOrFindCustomElementInterface was missing this last logic.

Also fixed a bug that Element::enqueueToUpgrade would hit a debug assertion when a custom element which has been
enqueued to upgrade is enqueued to upgrade for the second time. In this case, we need to put the element into the
current element queue (​https://html.spec.whatwg.org/multipage/custom-elements.html#current-element-queue) again.

While the specification simply enqueues another upgrade reaction and bails out immediately in the first step of
the upgrade, WebKit's implementation simply avoids this redundancy in the first place:
​https://html.spec.whatwg.org/multipage/custom-elements.html#concept-upgrade-an-element

Existing tests such as imported/w3c/web-platform-tests/custom-elements/reactions/Document.html exercises this
code path after the fragment parsing algorithm fix.

Tests: imported/w3c/web-platform-tests/custom-elements/connected-callbacks-html-fragment-parsing.html

  • dom/CustomElementReactionQueue.cpp:

(WebCore::CustomElementReactionQueueItem::type const): Added for an assertion.
(WebCore::CustomElementReactionQueue::enqueueElementUpgrade): Enqueue this element to the current element queue
by calling ensureCurrentQueue and avoid inserting a redundant upgrade reaction.

  • dom/CustomElementReactionQueue.h:
  • dom/Element.cpp:

(WebCore::Element::enqueueToUpgrade): Handle the case when a custom element is enqueued to upgrade for the second
time while it had been waiting in some element queue. In this case, the reaction queue for this element has
already been created and we simply need to put this element back into the current element queue (i.e. this element
now belongs to both element queues).

  • html/parser/HTMLConstructionSite.cpp:

(WebCore::findCustomElementInterface): Extracted out of createHTMLElementOrFindCustomElementInterface.
(WebCore::HTMLConstructionSite::createHTMLElementOrFindCustomElementInterface): Fixed the bug that the HTML parser
was synchronously constructing a custom element even for the fragment parsing algorithm.

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r234539 r234577  
     12018-08-03  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        innerHTML should not synchronously create a custom element
     4        https://bugs.webkit.org/show_bug.cgi?id=188327
     5        <rdar://problem/42923114>
     6
     7        Reviewed by Daniel Bates.
     8
     9        Rebaselined the test now that all test cases are passing.
     10
     11        * web-platform-tests/custom-elements/connected-callbacks-html-fragment-parsing-expected.txt:
     12
    1132018-08-02  Ryosuke Niwa  <rniwa@webkit.org>
    214

  • trunk/LayoutTests/imported/w3c/web-platform-tests/custom-elements/connected-callbacks-html-fragment-parsing-expected.txt

    r234039 r234577  
    11
    2 FAIL Inserting a custom element into the document using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor assert_equals: expected 1 but got 2
    3 FAIL Inserting a custom element into the document of the template elements using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor assert_equals: expected 1 but got 2
    4 FAIL Inserting a custom element into a new document using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor assert_equals: expected 1 but got 2
    5 FAIL Inserting a custom element into a cloned document using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor assert_equals: expected 1 but got 2
    6 FAIL Inserting a custom element into a document created by createHTMLDocument using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor assert_equals: expected 1 but got 2
    7 FAIL Inserting a custom element into an HTML document created by createDocument using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor assert_equals: expected 1 but got 2
    8 FAIL Inserting a custom element into the document of an iframe using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor assert_equals: expected 1 but got 2
    9 FAIL Inserting a custom element into an HTML document fetched by XHR using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor assert_equals: expected 1 but got 2
     2PASS Inserting a custom element into the document using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor
     3PASS Inserting a custom element into the document of the template elements using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor
     4PASS Inserting a custom element into a new document using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor
     5PASS Inserting a custom element into a cloned document using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor
     6PASS Inserting a custom element into a document created by createHTMLDocument using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor
     7PASS Inserting a custom element into an HTML document created by createDocument using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor
     8PASS Inserting a custom element into the document of an iframe using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor
     9PASS Inserting a custom element into an HTML document fetched by XHR using HTML fragment parsing must enqueue a custom element upgrade reaction, not synchronously invoke its constructor
    1010

  • trunk/Source/WebCore/ChangeLog

    r234569 r234577  
     12018-08-03  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        innerHTML should not synchronously create a custom element
     4        https://bugs.webkit.org/show_bug.cgi?id=188327
     5        <rdar://problem/42923114>
     6
     7        Reviewed by Daniel Bates.
     8
     9        Fixed the bug that the fragment parsing algorithm was synchronously constructing a custom element instead of
     10        enqueuing an element to upgrade.
     11
     12        The fragment parsing algorithm creates an element for a token with *will execute script* flag set to false:
     13        https://html.spec.whatwg.org/multipage/parsing.html#create-an-element-for-the-token
     14        which results in creating an element with synchronous custom elements flag *not* set:
     15        https://dom.spec.whatwg.org/#concept-create-element
     16
     17        When synchronous custom elements flag is false, we're supposed to create an element and enqueue a custom element
     18        upgrade reaction. createHTMLElementOrFindCustomElementInterface was missing this last logic.
     19
     20        Also fixed a bug that Element::enqueueToUpgrade would hit a debug assertion when a custom element which has been
     21        enqueued to upgrade is enqueued to upgrade for the second time. In this case, we need to put the element into the
     22        current element queue (https://html.spec.whatwg.org/multipage/custom-elements.html#current-element-queue) again.
     23
     24        While the specification simply enqueues another upgrade reaction and bails out immediately in the first step of
     25        the upgrade, WebKit's implementation simply avoids this redundancy in the first place:
     26        https://html.spec.whatwg.org/multipage/custom-elements.html#concept-upgrade-an-element
     27
     28        Existing tests such as imported/w3c/web-platform-tests/custom-elements/reactions/Document.html exercises this
     29        code path after the fragment parsing algorithm fix.
     30
     31        Tests: imported/w3c/web-platform-tests/custom-elements/connected-callbacks-html-fragment-parsing.html
     32
     33        * dom/CustomElementReactionQueue.cpp:
     34        (WebCore::CustomElementReactionQueueItem::type const): Added for an assertion.
     35        (WebCore::CustomElementReactionQueue::enqueueElementUpgrade): Enqueue this element to the current element queue
     36        by calling ensureCurrentQueue and avoid inserting a redundant upgrade reaction.
     37        * dom/CustomElementReactionQueue.h:
     38        * dom/Element.cpp:
     39        (WebCore::Element::enqueueToUpgrade): Handle the case when a custom element is enqueued to upgrade for the second
     40        time while it had been waiting in some element queue. In this case, the reaction queue for this element has
     41        already been created and we simply need to put this element back into the current element queue (i.e. this element
     42        now belongs to both element queues).
     43        * html/parser/HTMLConstructionSite.cpp:
     44        (WebCore::findCustomElementInterface): Extracted out of createHTMLElementOrFindCustomElementInterface.
     45        (WebCore::HTMLConstructionSite::createHTMLElementOrFindCustomElementInterface): Fixed the bug that the HTML parser
     46        was synchronously constructing a custom element even for the fragment parsing algorithm.
     47
    1482018-08-03  Ben Richards  <benton_richards@apple.com>
    249

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

    r234539 r234577  
    7171    { }
    7272
     73    Type type() const { return m_type; }
     74
    7375    void invoke(Element& element, JSCustomElementInterface& elementInterface)
    7476    {
    … …  
    116118}
    117119
    118 void CustomElementReactionQueue::enqueueElementUpgrade(Element& element)
    119 {
    120     auto& queue = ensureCurrentQueue(element);
    121     queue.m_items.append({CustomElementReactionQueueItem::Type::ElementUpgrade});
     120void CustomElementReactionQueue::enqueueElementUpgrade(Element& element, bool alreadyScheduledToUpgrade)
     121{
     122    auto& queue = ensureCurrentQueue(element);
     123    if (alreadyScheduledToUpgrade) {
     124        ASSERT(queue.m_items.size() == 1);
     125        ASSERT(queue.m_items[0].type() == CustomElementReactionQueueItem::Type::ElementUpgrade);
     126    } else
     127        queue.m_items.append({CustomElementReactionQueueItem::Type::ElementUpgrade});
    122128}
    123129

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

    r234539 r234577  
    5050    ~CustomElementReactionQueue();
    5151
    52     static void enqueueElementUpgrade(Element&);
     52    static void enqueueElementUpgrade(Element&, bool alreadyScheduledToUpgrade);
    5353    static void enqueueElementUpgradeIfDefined(Element&);
    5454    static void enqueueConnectedCallbackIfNeeded(Element&);

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

    r234507 r234577  
    20102010
    20112011    auto& data = ensureElementRareData();
    2012     ASSERT(!data.customElementReactionQueue());
    2013 
    2014     data.setCustomElementReactionQueue(std::make_unique<CustomElementReactionQueue>(elementInterface));
    2015     data.customElementReactionQueue()->enqueueElementUpgrade(*this);
     2012    bool alreadyScheduledToUpgrade = data.customElementReactionQueue();
     2013    if (!alreadyScheduledToUpgrade)
     2014        data.setCustomElementReactionQueue(std::make_unique<CustomElementReactionQueue>(elementInterface));
     2015    data.customElementReactionQueue()->enqueueElementUpgrade(*this, alreadyScheduledToUpgrade);
    20162016}
    20172017

  • trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp

    r225117 r234577  
    650650}
    651651
     652static inline JSCustomElementInterface* findCustomElementInterface(Document& ownerDocument, const AtomicString& localName)
     653{
     654    auto* window = ownerDocument.domWindow();
     655    if (!window)
     656        return nullptr;
     657
     658    auto* registry = window->customElementRegistry();
     659    if (LIKELY(!registry))
     660        return nullptr;
     661
     662    return registry->findInterface(localName);
     663}
     664
    652665RefPtr<Element> HTMLConstructionSite::createHTMLElementOrFindCustomElementInterface(AtomicHTMLToken& token, JSCustomElementInterface** customElementInterface)
    653666{
    … …  
    661674    RefPtr<Element> element = HTMLElementFactory::createKnownElement(localName, ownerDocument, insideTemplateElement ? nullptr : form(), true);
    662675    if (UNLIKELY(!element)) {
    663         auto window = makeRefPtr(ownerDocument.domWindow());
    664         if (customElementInterface && window) {
    665             auto registry = makeRefPtr(window->customElementRegistry());
    666             if (UNLIKELY(registry)) {
    667                 if (auto elementInterface = makeRefPtr(registry->findInterface(localName))) {
    668                     *customElementInterface = elementInterface.get();
    669                     return nullptr;
    670                 }
     676        if (auto* elementInterface = findCustomElementInterface(ownerDocument, localName)) {
     677            if (!m_isParsingFragment) {
     678                *customElementInterface = elementInterface;
     679                return nullptr;
    671680            }
     681            element = HTMLElement::create(QualifiedName { nullAtom(), localName, xhtmlNamespaceURI }, ownerDocument);
     682            element->setIsCustomElementUpgradeCandidate();
     683            element->enqueueToUpgrade(*elementInterface);
     684        } else {
     685            QualifiedName qualifiedName { nullAtom(), localName, xhtmlNamespaceURI };
     686            if (Document::validateCustomElementName(localName) == CustomElementNameValidationStatus::Valid) {
     687                element = HTMLElement::create(qualifiedName, ownerDocument);
     688                element->setIsCustomElementUpgradeCandidate();
     689            } else
     690                element = HTMLUnknownElement::create(qualifiedName, ownerDocument);
    672691        }
    673 
    674         QualifiedName qualifiedName(nullAtom(), localName, xhtmlNamespaceURI);
    675         if (Document::validateCustomElementName(localName) == CustomElementNameValidationStatus::Valid) {
    676             element = HTMLElement::create(qualifiedName, ownerDocument);
    677             element->setIsCustomElementUpgradeCandidate();
    678         } else
    679             element = HTMLUnknownElement::create(qualifiedName, ownerDocument);
    680692    }
    681693    ASSERT(element);
Note: See TracChangeset for help on using the changeset viewer.