Bug 82701

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

Created attachment 135805 [details] Patch

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.

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.

(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.

(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.

Created attachment 136854 [details] Patch

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.

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

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

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

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

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

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.

Created attachment 137060 [details] Patch

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

Created attachment 137272 [details] Patch

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.

Created attachment 137300 [details] Patch

(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.

Comment on attachment 137300 [details] Patch Oh wow. This patch looks so much better than what I looked at a week ago.

(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 ;-)

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.

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?

Created attachment 137479 [details] Patch

(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.

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.

Created attachment 137480 [details] Patch for landing

Comment on attachment 137480 [details] Patch for landing Clearing flags on attachment: 137480 Committed r114351: <http://trac.webkit.org/changeset/114351>

All reviewed patches have been landed. Closing bug.

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?

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

Thanks for the fix, Ossy.

(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.

(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 ;-)