Changeset 158708 in webkit


Ignore:
Timestamp:
Nov 5, 2013 6:15:50 PM (10 years ago)
Author:
Alexandru Chiculita
Message:

Web Inspector: Moving an element while in the DOMNodeRemoved handler will hide it in the inspector
​https://bugs.webkit.org/show_bug.cgi?id=123516

Reviewed by Timothy Hatcher.

Source/WebCore:

InspectorInstrumentation::willRemoveDOMNode was actually calling both willRemoveDOMNodeImpl and
didRemoveDOMNodeImpl, making the DOMAgent unbind the element even if it was still part of the DOM.

Because of that the DOMAgent was sending two events:

  1. When the element was about to be removed, just before JS "DOMNodeRemoved" was triggered.
  2. When the element was actually removed.

Note that inspector's event #2 will not know about the node, as it just removed it from the
internal hashmap, so it will just use a nodeID == 0 for it.

This patch adds a separate call to InspectorInstrumentation::didRemoveDOMNode, just before the
element is about to be removed. The InspectorInstrumentation::willRemoveDOMNode call is now only used
by the DOMDebugger to trigger the DOM breakpoints in the Web Inspector. That feature is not exposed
in the new Inspector UI, but can be used/tested using the protocol directly.

Tests: inspector-protocol/dom-debugger/node-removed.html

inspector-protocol/dom/dom-remove-events.html
inspector-protocol/dom/remove-multiple-nodes.html

  • dom/ContainerNode.cpp:

(WebCore::ContainerNode::removeBetween):

  • inspector/InspectorInstrumentation.h:

(WebCore::InspectorInstrumentation::willRemoveDOMNode):
(WebCore::InspectorInstrumentation::didRemoveDOMNode):

LayoutTests:

Added tests to check that the DOM.childNodeRemoved inspector-protocol message is dispatched
correctly when DOM nodes are moved while inside the "DOMNodeRemoved" event handler.

  • inspector-protocol/dom-debugger/node-removed-expected.txt: Added.
  • inspector-protocol/dom-debugger/node-removed.html: Added. Checking that the DOMDebugger agent

is still sending the node-removed events.

  • inspector-protocol/dom/dom-remove-events-expected.txt: Added.
  • inspector-protocol/dom/dom-remove-events.html: Added. Test with a single DOM remove event.
  • inspector-protocol/dom/remove-multiple-nodes-expected.txt: Added.
  • inspector-protocol/dom/remove-multiple-nodes.html: Added. Test case when multiple children

are removed at once with parentNode.textContent = "String".

Location:
trunk
Files:
7 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r158703 r158708  
     12013-11-05  Alexandru Chiculita  <achicu@adobe.com>
     2
     3        Web Inspector: Moving an element while in the DOMNodeRemoved handler will hide it in the inspector
     4        https://bugs.webkit.org/show_bug.cgi?id=123516
     5
     6        Reviewed by Timothy Hatcher.
     7
     8        Added tests to check that the DOM.childNodeRemoved inspector-protocol message is dispatched
     9        correctly when DOM nodes are moved while inside the "DOMNodeRemoved" event handler.
     10
     11        * inspector-protocol/dom-debugger/node-removed-expected.txt: Added.
     12        * inspector-protocol/dom-debugger/node-removed.html: Added. Checking that the DOMDebugger agent
     13        is still sending the node-removed events.
     14        * inspector-protocol/dom/dom-remove-events-expected.txt: Added.
     15        * inspector-protocol/dom/dom-remove-events.html: Added. Test with a single DOM remove event.
     16        * inspector-protocol/dom/remove-multiple-nodes-expected.txt: Added.
     17        * inspector-protocol/dom/remove-multiple-nodes.html: Added. Test case when multiple children
     18        are removed at once with parentNode.textContent = "String".
     19
    1202013-11-05  Myles C. Maxfield  <mmaxfield@apple.com>
    221

  • trunk/Source/WebCore/ChangeLog

    r158705 r158708  
     12013-11-05  Alexandru Chiculita  <achicu@adobe.com>
     2
     3        Web Inspector: Moving an element while in the DOMNodeRemoved handler will hide it in the inspector
     4        https://bugs.webkit.org/show_bug.cgi?id=123516
     5
     6        Reviewed by Timothy Hatcher.
     7
     8        InspectorInstrumentation::willRemoveDOMNode was actually calling both willRemoveDOMNodeImpl and
     9        didRemoveDOMNodeImpl, making the DOMAgent unbind the element even if it was still part of the DOM.
     10
     11        Because of that the DOMAgent was sending two events:
     12        1. When the element was about to be removed, just before JS "DOMNodeRemoved" was triggered.
     13        2. When the element was actually removed.
     14
     15        Note that inspector's event #2 will not know about the node, as it just removed it from the
     16        internal hashmap, so it will just use a nodeID == 0 for it.
     17
     18        This patch adds a separate call to InspectorInstrumentation::didRemoveDOMNode, just before the
     19        element is about to be removed. The InspectorInstrumentation::willRemoveDOMNode call is now only used
     20        by the DOMDebugger to trigger the DOM breakpoints in the Web Inspector. That feature is not exposed
     21        in the new Inspector UI, but can be used/tested using the protocol directly.
     22
     23        Tests: inspector-protocol/dom-debugger/node-removed.html
     24               inspector-protocol/dom/dom-remove-events.html
     25               inspector-protocol/dom/remove-multiple-nodes.html
     26
     27        * dom/ContainerNode.cpp:
     28        (WebCore::ContainerNode::removeBetween):
     29        * inspector/InspectorInstrumentation.h:
     30        (WebCore::InspectorInstrumentation::willRemoveDOMNode):
     31        (WebCore::InspectorInstrumentation::didRemoveDOMNode):
     32
    1332013-11-05  Ryuan Choi  <ryuan.choi@samsung.com>
    234

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

    r158663 r158708  
    586586void ContainerNode::removeBetween(Node* previousChild, Node* nextChild, Node& oldChild)
    587587{
     588    InspectorInstrumentation::didRemoveDOMNode(&oldChild.document(), &oldChild);
     589
    588590    NoEventDispatchAssertion assertNoEventDispatch;
    589591

  • trunk/Source/WebCore/inspector/InspectorInstrumentation.h

    r158432 r158708  
    120120    static void didInsertDOMNode(Document*, Node*);
    121121    static void willRemoveDOMNode(Document*, Node*);
     122    static void didRemoveDOMNode(Document*, Node*);
    122123    static void willModifyDOMAttr(Document*, Element*, const AtomicString& oldValue, const AtomicString& newValue);
    123124    static void didModifyDOMAttr(Document*, Element*, const AtomicString& name, const AtomicString& value);
    … …  
    566567#if ENABLE(INSPECTOR)
    567568    FAST_RETURN_IF_NO_FRONTENDS(void());
    568     if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForDocument(document)) {
     569    if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForDocument(document))
    569570        willRemoveDOMNodeImpl(instrumentingAgents, node);
     571#else
     572    UNUSED_PARAM(document);
     573    UNUSED_PARAM(node);
     574#endif
     575}
     576
     577inline void InspectorInstrumentation::didRemoveDOMNode(Document* document, Node* node)
     578{
     579#if ENABLE(INSPECTOR)
     580    FAST_RETURN_IF_NO_FRONTENDS(void());
     581    if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForDocument(document))
    570582        didRemoveDOMNodeImpl(instrumentingAgents, node);
    571     }
    572583#else
    573584    UNUSED_PARAM(document);
Note: See TracChangeset for help on using the changeset viewer.