Bug 82701 – insertedIntoDocument and insertedIntoTree should be unified

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Hajime Morrita]

These two are both virtual. and merging these into one will help speedup.
This also will give a cleaner hook to hook tree insertion, which I need for Bug 82681.

[Adam Klein]

Sounds reasonable to me. But note that you probably don't want to merge removedFromTree/removedFromDocument, even though that seems more symmetric (see https://bugs.webkit.org/show_bug.cgi?id=80628 for some of the problems with removedFromDocument).

[Hajime Morrita]

Created attachment 135805 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 135805 [details]
Patch

The proliferation of *Context pattern is now starting to alarm me (even though I think I first plopped it in with EventContext). However, I agree that this is not bad at all. The deep/not-deep distinction was never clear and this makes it a lot more intuitive.

[Hajime Morrita]

Thanks for your feedback Dimitri!

> (From update of attachment 135805 [details])
> The proliferation of *Context pattern is now starting to alarm me (even though I think I first plopped it in with EventContext). However, I agree that this is not bad at all. The deep/not-deep distinction was never clear and this makes it a lot more intuitive.
Yeah, "Context" seems like "Manager" in certain level, which give little color for the class...
The tree traversal code could be stayed in ContainerNode.cpp or could be part of
ContainerNodeAlgorithms.h.

[Dimitri Glazkov (Google)]

(In reply to comment #4)
> Thanks for your feedback Dimitri!
> 
> > (From update of attachment 135805 [details] [details])
> > The proliferation of *Context pattern is now starting to alarm me (even though I think I first plopped it in with EventContext). However, I agree that this is not bad at all. The deep/not-deep distinction was never clear and this makes it a lot more intuitive.
> Yeah, "Context" seems like "Manager" in certain level, which give little color for the class...
> The tree traversal code could be stayed in ContainerNode.cpp or could be part of
> ContainerNodeAlgorithms.h.

I am fine either way.

[Ryosuke Niwa]

(In reply to comment #4)
> Thanks for your feedback Dimitri!
> 
> > (From update of attachment 135805 [details] [details])
> > The proliferation of *Context pattern is now starting to alarm me (even though I think I first plopped it in with EventContext). However, I agree that this is not bad at all. The deep/not-deep distinction was never clear and this makes it a lot more intuitive.
> Yeah, "Context" seems like "Manager" in certain level, which give little color for the class...
> The tree traversal code could be stayed in ContainerNode.cpp or could be part of
> ContainerNodeAlgorithms.h.

Putting it in ContainerNodeAlgorithms makes most sense.

[Hajime Morrita]

Created attachment 136854 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 136854 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=136854&action=review

> Source/WebCore/ChangeLog:11
> +        getting together them into one, to gain some speedup. Such

Nit: combining them together? merging them into one function?

> Source/WebCore/ChangeLog:12
> +        unification also could clarify the semantics of the hook.

Nit: also is redundant here.

> Source/WebCore/ChangeLog:14
> +        This change makes it happen by:

"it" here is ambiguous. I think it's probably better to say "This patch makes the following changes" instead.

> Source/WebCore/ChangeLog:37
> +        There is possible situation where a node need to hook both before
> +        and after its children is nofitied.  It was represented naturally
> +        with previous traversal-by-ContainerNode pattern, but is no longer
> +        simple with this new external traversal. To support this scenario,

I don't fully understand these two paragraphs. What are you referring to by "hook"?

> Source/WebCore/ChangeLog:41
> +        how it works. In fast, that is the only instance where this
> +        mechanism is employed.

Is it possible to get rid of setNeedsNotificationAfterChildrenNotified by refactoring the form element code?

> Source/WebCore/ChangeLog:43
> +        Reviewed by NOBODY (OOPS!).

This line should appear before the description.

> Source/WebCore/dom/ContainerNode.h:84
>      // -----------------------------------------------------------------------------
>      // Notification of document structure changes (see Node.h for more notification methods)
>  

I'm not sure if this comment is helpful now given you've removed two out of the three methods we had.

> Source/WebCore/dom/ContainerNodeAlgorithms.cpp:34
> +namespace NodeInsertionPrivate {

Do we really want to introduce new namespaces for these functions?

> Source/WebCore/dom/ContainerNodeAlgorithms.cpp:36
> +void traverseChildrenForDocument(ContainerNode* node)

Instead of hiding these functions inside a namespace and giving a generic name like traverseChildrenForDocument, can we just add them to WebCore with much more descriptive function names (e.g. notifyDescendentsInsertedIntoDocument)?

> Source/WebCore/dom/ContainerNodeAlgorithms.cpp:45
> +            traverseForDocument(children[i].get());

notifyInsertedIntoDocument?

> Source/WebCore/dom/ContainerNodeAlgorithms.cpp:78
> +namespace NodeRemovalPrivate {
> +
> +void traverseChildrenForDocument(ContainerNode* node)

Ditto about namespace & function name.

> Source/WebCore/dom/ContainerNodeAlgorithms.h:184
> +namespace NodeInsertionPrivate {
> +
> +void traverseChildrenForDocument(ContainerNode*);
> +void traverseChildrenForTree(ContainerNode*);
> +
> +inline void traverseForDocument(Node* node)
> +{
> +    NodeInsertionContext context(NodeMovingContext::MoveTypeDocument);
> +    RefPtr<Node> protect(node);
> +    node->insertedInto(context);
> +    if (node->isContainerNode())
> +        traverseChildrenForDocument(toContainerNode(node));
> +    if (context.needsNotificationAfterChildrenNotified())
> +        node->notifiedInsertedInto(context);
> +}
> +
> +inline void traverseForTree(ContainerNode* node)
> +{
> +    NodeInsertionContext context(NodeMovingContext::MoveTypeTree);
> +    forbidEventDispatch();
> +    node->insertedInto(context);
> +    traverseChildrenForTree(node);
> +    if (context.needsNotificationAfterChildrenNotified())
> +        node->notifiedInsertedInto(context);
> +    allowEventDispatch();
> +}

Do we really need to keep these functions inline? We'll end up calling functions anyways, right?
Can't we just move these functions to ContainerNodeAlgorithms.cpp ?

> Source/WebCore/dom/ContainerNodeAlgorithms.h:192
> +namespace NodeRemovalPrivate {
> +
> +void traverseChildrenForDocument(ContainerNode*);
> +void traverseChildrenForTree(ContainerNode*);
> +

These proliferations of namespaces and prototypes don't look great to me.

> Source/WebCore/dom/NodeMovingContext.h:41
> +    bool isAcrossDocument() const { return m_type == MoveTypeDocument; }

I'm not sure if "across document" is an accurate description here. It's more like moving out from and into a document...

> Source/WebCore/dom/NodeMovingContext.h:66
> +class NodeInsertionContext : public NodeMovingContext {
> +public:
> +    NodeInsertionContext(MoveType type)
> +        : NodeMovingContext(type)
> +        , m_needsNotificationAfterChildrenNotified(false)
> +    { }
> +
> +    void setNeedsNotificationAfterChildrenNotified() { m_needsNotificationAfterChildrenNotified = true; }
> +    bool needsNotificationAfterChildrenNotified() const { return m_needsNotificationAfterChildrenNotified; }
> +
> +private:
> +    bool m_needsNotificationAfterChildrenNotified;
> +};
> +
> +class NodeRemovalContext : public NodeMovingContext {
> +public:
> +    NodeRemovalContext(MoveType type)
> +        : NodeMovingContext(type)
> +    { }
> +};

Do we really need 3 classes for this? I feel like the only reason we do is because of setNeedsNotificationAfterChildrenNotified and we can just use a bit flag otherwise.

[WebKit Review Bot]

Comment on attachment 136854 [details]
Patch

Attachment 136854 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12396135

New failing tests:
tables/mozilla/bugs/bug4527.html
fast/frames/no-frame-borders.html

[WebKit Review Bot]

Created attachment 136865 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[WebKit Review Bot]

Comment on attachment 136854 [details]
Patch

Attachment 136854 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12395198

New failing tests:
tables/mozilla/bugs/bug4527.html
fast/frames/no-frame-borders.html

[WebKit Review Bot]

Created attachment 136875 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Hajime Morrita]

Created attachment 136879 [details]
Updated with a test failure fix

[Hajime Morrita]

Comment on attachment 136854 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=136854&action=review

Hi Ryosuke, thanks for taking a look! I updated the patch.

>> Source/WebCore/ChangeLog:11
>> +        getting together them into one, to gain some speedup. Such
> 
> Nit: combining them together? merging them into one function?

Fixed.

>> Source/WebCore/ChangeLog:12
>> +        unification also could clarify the semantics of the hook.
> 
> Nit: also is redundant here.

Removed

>> Source/WebCore/ChangeLog:14
>> +        This change makes it happen by:
> 
> "it" here is ambiguous. I think it's probably better to say "This patch makes the following changes" instead.

clarified.

>> Source/WebCore/ChangeLog:37
>> +        simple with this new external traversal. To support this scenario,
> 
> I don't fully understand these two paragraphs. What are you referring to by "hook"?

Clarified.

>> Source/WebCore/ChangeLog:41
>> +        mechanism is employed.
> 
> Is it possible to get rid of setNeedsNotificationAfterChildrenNotified by refactoring the form element code?

Possibly. But it isn't trivial change. Could be attacked by a separate change.

>> Source/WebCore/ChangeLog:43
>> +        Reviewed by NOBODY (OOPS!).
> 
> This line should appear before the description.

Oops.

>> Source/WebCore/dom/ContainerNode.h:84
>>  
> 
> I'm not sure if this comment is helpful now given you've removed two out of the three methods we had.

Moved and reworded the comment to Node.h

>> Source/WebCore/dom/ContainerNodeAlgorithms.cpp:36
>> +void traverseChildrenForDocument(ContainerNode* node)
> 
> Instead of hiding these functions inside a namespace and giving a generic name like traverseChildrenForDocument, can we just add them to WebCore with much more descriptive function names (e.g. notifyDescendentsInsertedIntoDocument)?

What I'd intended was that the private functions are just an implementation detail and shouldn't be used.
I put these class instead to clarify the intention.

>> Source/WebCore/dom/ContainerNodeAlgorithms.h:184
>> +}
> 
> Do we really need to keep these functions inline? We'll end up calling functions anyways, right?
> Can't we just move these functions to ContainerNodeAlgorithms.cpp ?

That prevents compiler to make them actually inline because these function has recursion. 
Also, the functions are not called when the node is a Text object.

>> Source/WebCore/dom/ContainerNodeAlgorithms.h:192
>> +
> 
> These proliferations of namespaces and prototypes don't look great to me.

Turned them to classes.

>> Source/WebCore/dom/NodeMovingContext.h:41
>> +    bool isAcrossDocument() const { return m_type == MoveTypeDocument; }
> 
> I'm not sure if "across document" is an accurate description here. It's more like moving out from and into a document...

Good point. Will move them to subclasses to give appropriate names.

>> Source/WebCore/dom/NodeMovingContext.h:66
>> +};
> 
> Do we really need 3 classes for this? I feel like the only reason we do is because of setNeedsNotificationAfterChildrenNotified and we can just use a bit flag otherwise.

I'm planning to merge willRemove() later where m_originalParent will be added to NodeRemovalContext.

[Hajime Morrita]

Created attachment 137060 [details]
Patch

[Hajime Morrita]

Ryosuke, Could you take a look? This patch addressed your points at #webkit.

[Hajime Morrita]

Created attachment 137272 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 137272 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137272&action=review

The patch looks much better. I think we want one more iteration of name tweaking.

> Source/WebCore/dom/ContainerNodeAlgorithms.cpp:34
> +void ChildNodeInsertionHelper::traverseChildrenForDocument(ContainerNode* node)

Can we call this class ChildNodeInsertionNotifier instead? "helper" sounds too generic to me. Also, it's probably better to call this function notifyDescendentsInsertedIntoDocument to be more explicit.

> Source/WebCore/dom/ContainerNodeAlgorithms.cpp:56
> +void ChildNodeInsertionHelper::traverseChildrenForTree(ContainerNode* node)

Ditto about class & function names.

> Source/WebCore/dom/ContainerNodeAlgorithms.cpp:72
> +void ChildNodeRemovalHelper::traverseChildrenForDocument(ContainerNode* node)

Ditto.

> Source/WebCore/dom/ContainerNodeAlgorithms.cpp:97
> +void ChildNodeRemovalHelper::traverseChildrenForTree(ContainerNode* node)

Ditto.

> Source/WebCore/dom/Node.h:511
> +    enum InsertionResult {
> +        InsertionDone,
> +        InsertionNeedsNotification
> +    };

I think we need to rename these flags so that the semantics is clearer.
When is InsertionNeedsNotification returned and why?

> Source/WebCore/dom/Node.h:518
> +    // This first type of nofification comes from NodeMovingContext::MoveTypeDocument.

This comment is out of date, right?

> Source/WebCore/dom/Node.h:523
> +    // In that case, the notification type is NodeMovingContext::MoveTypeTree.

Ditto.

[Hajime Morrita]

Created attachment 137300 [details]
Patch

[Hajime Morrita]

(In reply to comment #18)
> (From update of attachment 137272 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=137272&action=review
> 
> The patch looks much better. I think we want one more iteration of name tweaking.
> 
> > Source/WebCore/dom/ContainerNodeAlgorithms.cpp:34
> > +void ChildNodeInsertionHelper::traverseChildrenForDocument(ContainerNode* node)
> 
> Can we call this class ChildNodeInsertionNotifier instead? "helper" sounds too generic to me. Also, it's probably better to call this function notifyDescendentsInsertedIntoDocument to be more explicit.
Right.renamed.

> > Source/WebCore/dom/Node.h:511
> > +    enum InsertionResult {
> > +        InsertionDone,
> > +        InsertionNeedsNotification
> > +    };
> 
> I think we need to rename these flags so that the semantics is clearer.
> When is InsertionNeedsNotification returned and why?
A few element implementation needs this for their own reasons.
But It should clarify "when" indeed. I renamed it to more descriptive name.

> 
> > Source/WebCore/dom/Node.h:518
> > +    // This first type of nofification comes from NodeMovingContext::MoveTypeDocument.
> 
> This comment is out of date, right?
> 
Oops. Rewrote the comment.

[Dimitri Glazkov (Google)]

Comment on attachment 137300 [details]
Patch

Oh wow. This patch looks so much better than what I looked at a week ago.

[Hajime Morrita]

(In reply to comment #21)
> (From update of attachment 137300 [details])
> Oh wow. This patch looks so much better than what I looked at a week ago.
Yeah, you should thank Ryosuke ;-)

[Ryosuke Niwa]

Comment on attachment 137300 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137300&action=review

> Source/WebCore/ChangeLog:14
> +        unification could clarify the semantics of the hook.

I would get rid of "of the hook" and replace it by "as well".

> Source/WebCore/dom/ContainerNode.cpp:408
> +    ChildNodeRemovalNotifier(this).notifyRemoved(child.get());

Given that the class is called ChildNodeRemovalNotifier, we can probably just call this method "notify".

> Source/WebCore/dom/ContainerNode.cpp:593
>      // FIXME: Why doesn't this use notifyChildInserted(newChild) instead?

Maybe we should update this comment?

> Source/WebCore/dom/Element.cpp:892
> +    if (containsFullScreenElement() && parentElement() && !parentElement()->containsFullScreenElement())
> +        setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(true);

This looks like horrifyingly inefficient :(  Note that this code was just moved from Element::insertedIntoTree so it's not morrita's fault.

> Source/WebCore/dom/Node.h:522
> +    enum InsertionResult {

Instead of calling it "result" can it name it to something like ShouldBlah? Because it isn't really a result. It's more of a decision/command as to something should happen or not.

> Source/WebCore/html/FormAssociatedElement.cpp:63
> +void FormAssociatedElement::insertedInto(Node* rootParent)

I don't think "rootParent" is semantically clear. How about something like insertionPointContainer?

> Source/WebCore/html/FormAssociatedElement.cpp:74
> +static inline Node* findRoot(Node* n)

Can we rename this function to highestAncestor since that's what this function is doing?
Also, please rename n to node.

[Ryosuke Niwa]

Comment on attachment 137300 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137300&action=review

> Source/WebCore/html/HTMLStyleElement.cpp:-173
> -    HTMLElement::removedFromDocument();
> -    StyleElement::removedFromDocument(document(), this);

It seems like we used to call StyleElement::removedFromDocument after HTMLElement::removedFromDocument?

[Hajime Morrita]

Created attachment 137479 [details]
Patch

[Hajime Morrita]

(In reply to comment #23)
> (From update of attachment 137300 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=137300&action=review
> 
> > Source/WebCore/ChangeLog:14
> > +        unification could clarify the semantics of the hook.
> 
> I would get rid of "of the hook" and replace it by "as well".
Fixed. Sounds more like English... 

> 
> > Source/WebCore/dom/ContainerNode.cpp:408
> > +    ChildNodeRemovalNotifier(this).notifyRemoved(child.get());
> 
> Given that the class is called ChildNodeRemovalNotifier, we can probably just call this method "notify".
Renamed.

> 
> > Source/WebCore/dom/ContainerNode.cpp:593
> >      // FIXME: Why doesn't this use notifyChildInserted(newChild) instead?
> 
> Maybe we should update this comment?
True. updated to align the method name.

> 
> > Source/WebCore/dom/Element.cpp:892
> > +    if (containsFullScreenElement() && parentElement() && !parentElement()->containsFullScreenElement())
> > +        setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(true);
> 
> This looks like horrifyingly inefficient :(  Note that this code was just moved from Element::insertedIntoTree so it's not morrita's fault.

So I'd keep this for homework for someone else...

> 
> > Source/WebCore/dom/Node.h:522
> > +    enum InsertionResult {
> 
> Instead of calling it "result" can it name it to something like ShouldBlah? Because it isn't really a result. It's more of a decision/command as to something should happen or not.
Hmm. How about InsertionNotificationRequest? I tried this and looks reasonable.

> 
> > Source/WebCore/html/FormAssociatedElement.cpp:63
> > +void FormAssociatedElement::insertedInto(Node* rootParent)
> 
> I don't think "rootParent" is semantically clear. How about something like insertionPointContainer?
Renamed to insertionPoint - it's necessarily a container.
> 
> > Source/WebCore/html/FormAssociatedElement.cpp:74
> > +static inline Node* findRoot(Node* n)
> 
> Can we rename this function to highestAncestor since that's what this function is doing?
> Also, please rename n to node.
Done.


(In reply to comment #24)
> (From update of attachment 137300 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=137300&action=review
> 
> > Source/WebCore/html/HTMLStyleElement.cpp:-173
> > -    HTMLElement::removedFromDocument();
> > -    StyleElement::removedFromDocument(document(), this);
> 
> It seems like we used to call StyleElement::removedFromDocument after HTMLElement::removedFromDocument?
Ah, yes. It looks. Aligned to existing code.

[Ryosuke Niwa]

Comment on attachment 137479 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137479&action=review

> Source/WebCore/dom/ContainerNodeAlgorithms.h:198
> +    ASSERT(m_insertionPoint->inDocument());
> +    RefPtr<Node> protect(node);
> +    Node::InsertionNotificationRequest request = node->insertedInto(m_insertionPoint);
> +    if (node->isContainerNode())
> +        notifyDescendantInsertedIntoDocument(toContainerNode(node));
> +    if (request == Node::InsertionShouldCallDidNotifyDescendantInseretions)
> +        node->didNotifyDescendantInseretions(m_insertionPoint);

Maybe some blank lines would improve the readability?

> Source/WebCore/dom/ContainerNodeAlgorithms.h:209
> +    ASSERT(!m_insertionPoint->inDocument());
> +    forbidEventDispatch();
> +    Node::InsertionNotificationRequest request = node->insertedInto(m_insertionPoint);
> +    notifyDescendantInsertedIntoTree(node);
> +    if (request == Node::InsertionShouldCallDidNotifyDescendantInseretions)
> +        node->didNotifyDescendantInseretions(m_insertionPoint);
> +    allowEventDispatch();

Ditto.

[Hajime Morrita]

Created attachment 137480 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 137480 [details]
Patch for landing

Clearing flags on attachment: 137480

Committed r114351: <http://trac.webkit.org/changeset/114351>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

Reopen, because it made 5 tests assert on Mac and on Qt:
- http://build.webkit.org/results/Lion%20Debug%20%28Tests%29/r114351%20%285606%29/results.html
- http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Debug/r114351%20%2822316%29/results.html

crash log for DumpRenderTree (pid 11595):
STDOUT: <empty>
STDERR: ASSERTION FAILED: !HashTranslator::equal(KeyTraits::emptyValue(), key)
STDERR: ../../../../Source/WTF/wtf/HashTable.h(487) : void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::checkKey(const T&) [with HashTranslator = WTF::HashMapTranslator<WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >, WTF::PtrHash<WTF::RefPtr<WebCore::Node> > >, T = WebCore::Node*, Key = WTF::RefPtr<WebCore::Node>, Value = std::pair<WTF::RefPtr<WebCore::Node>, int>, Extractor = WTF::PairFirstExtractor<std::pair<WTF::RefPtr<WebCore::Node>, int> >, HashFunctions = WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, Traits = WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >, KeyTraits = WTF::HashTraits<WTF::RefPtr<WebCore::Node> >]
STDERR: 1   0x7f230938a257 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(void WTF::HashTable<WTF::RefPtr<WebCore::Node>, std::pair<WTF::RefPtr<WebCore::Node>, int>, WTF::PairFirstExtractor<std::pair<WTF::RefPtr<WebCore::Node>, int> >, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> > >::checkKey<WTF::HashMapTranslator<WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >, WTF::PtrHash<WTF::RefPtr<WebCore::Node> > >, WebCore::Node*>(WebCore::Node* const&)+0x89) [0x7f230938a257]
STDERR: 2   0x7f2309386ce7 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(std::pair<WTF::RefPtr<WebCore::Node>, int>* WTF::HashTable<WTF::RefPtr<WebCore::Node>, std::pair<WTF::RefPtr<WebCore::Node>, int>, WTF::PairFirstExtractor<std::pair<WTF::RefPtr<WebCore::Node>, int> >, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> > >::lookup<WTF::HashMapTranslator<WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >, WTF::PtrHash<WTF::RefPtr<WebCore::Node> > >, WebCore::Node*>(WebCore::Node* const&)+0x23) [0x7f2309386ce7]
STDERR: 3   0x7f230938338f /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >::inlineGet(WebCore::Node*) const+0x23) [0x7f230938338f]
STDERR: 4   0x7f230937fdd3 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >::get(WebCore::Node*) const+0x23) [0x7f230937fdd3]
STDERR: 5   0x7f230937db73 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::InspectorDOMAgent::didInsertDOMNode(WebCore::Node*)+0x69) [0x7f230937db73]
STDERR: 6   0x7f23093a1258 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::InspectorInstrumentation::didInsertDOMNodeImpl(WebCore::InstrumentingAgents*, WebCore::Node*)+0x3a) [0x7f23093a1258]
STDERR: 7   0x7f2308fe169a /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::InspectorInstrumentation::didInsertDOMNode(WebCore::Document*, WebCore::Node*)+0x46) [0x7f2308fe169a]
STDERR: 8   0x7f2308fe19ba /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::ChildNodeInsertionNotifier::notify(WebCore::Node*)+0x72) [0x7f2308fe19ba]
STDERR: 9   0x7f23090dbc1f /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::ShadowTree::addShadowRoot(WebCore::Element*, WTF::PassRefPtr<WebCore::ShadowRoot>, int&)+0x127) [0x7f23090dbc1f]
STDERR: 10  0x7f23090daf81 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::ShadowRoot::create(WebCore::Element*, WebCore::ShadowRoot::ShadowRootCreationPurpose, int&)+0x15f) [0x7f23090daf81]
STDERR: 11  0x7f230924aca8 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLInputElement::createShadowSubtree()+0x9c) [0x7f230924aca8]
STDERR: 12  0x7f230924abbc /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLInputElement::create(WebCore::QualifiedName const&, WebCore::Document*, WebCore::HTMLFormElement*, bool)+0xd0) [0x7f230924abbc]
STDERR: 13  0x7f230a180284 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(+0x341f284) [0x7f230a180284]
STDERR: 14  0x7f230a182339 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLElementFactory::createHTMLElement(WebCore::QualifiedName const&, WebCore::Document*, WebCore::HTMLFormElement*, bool)+0xab) [0x7f230a182339]
STDERR: 15  0x7f23092b944a /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLConstructionSite::createHTMLElement(WebCore::AtomicHTMLToken&)+0x7e) [0x7f23092b944a]
STDERR: 16  0x7f23092b8755 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLConstructionSite::insertSelfClosingHTMLElement(WebCore::AtomicHTMLToken&)+0x7b) [0x7f23092b8755]
STDERR: 17  0x7f23092e31ba /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLTreeBuilder::processStartTagForInBody(WebCore::AtomicHTMLToken&)+0x140a) [0x7f23092e31ba]
STDERR: 18  0x7f23092e4cd3 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLTreeBuilder::processStartTag(WebCore::AtomicHTMLToken&)+0x82d) [0x7f23092e4cd3]
STDERR: 19  0x7f23092e05c2 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLTreeBuilder::processToken(WebCore::AtomicHTMLToken&)+0xc8) [0x7f23092e05c2]
STDERR: 20  0x7f23092e03fe /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken(WebCore::AtomicHTMLToken&)+0x50) [0x7f23092e03fe]
STDERR: 21  0x7f23092e02f8 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLTreeBuilder::constructTreeFromToken(WebCore::HTMLToken&)+0x66) [0x7f23092e02f8]
STDERR: 22  0x7f23092bf023 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)+0x327) [0x7f23092bf023]
STDERR: 23  0x7f23092be8e9 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode)+0xb1) [0x7f23092be8e9]
STDERR: 24  0x7f23092bfce6 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution()+0xe0) [0x7f23092bfce6]
STDERR: 25  0x7f23092c002d /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLDocumentParser::notifyFinished(WebCore::CachedResource*)+0x1c7) [0x7f23092c002d]
STDERR: 26  0x7f23094493fd /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::CachedResource::checkNotify()+0x6b) [0x7f23094493fd]
STDERR: 27  0x7f2309453677 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::CachedScript::data(WTF::PassRefPtr<WebCore::SharedBuffer>, bool)+0x9d) [0x7f2309453677]
STDERR: 28  0x7f23094bdbc8 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::SubresourceLoader::didFinishLoading(double)+0x244) [0x7f23094bdbc8]
STDERR: 29  0x7f23094b26a9 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*, double)+0x33) [0x7f23094b26a9]
STDERR: 30  0x7f23099379e9 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::QNetworkReplyHandler::finish()+0x1d3) [0x7f23099379e9]
STDERR: 31  0x7f230993598e /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::QNetworkReplyHandlerCallQueue::flush()+0x9c) [0x7f230993598e]


Could you check what happened?

[Csaba Osztrogonác]

Fixed by http://trac.webkit.org/changeset/114388

[Hajime Morrita]

Thanks for the fix, Ossy.

[Csaba Osztrogonác]

(In reply to comment #33)
> Thanks for the fix, Ossy.

It wasn't me, apavlov was the bug fixer. ;-) I only 
noticed that he fixed it without reference to this bug.

[Hajime Morrita]

(In reply to comment #34)
> It wasn't me, apavlov was the bug fixer. ;-) I only 
> noticed that he fixed it without reference to this bug.
Thanks for letting me know then ;-)