Changeset 158854 in webkit

Timestamp:

Nov 7, 2013 10:05:55 AM (10 years ago)

Author:

Alexandru Chiculita

Message:

Web Inspector: CSS Regions: Removing a content node of a ContentFlow from the DOM will send a 0 nodeId
​https://bugs.webkit.org/show_bug.cgi?id=123577

Source/WebCore:

Reviewed by Timothy Hatcher.

Test: inspector-protocol/model/content-flow-content-removal.html

Do not send unregister events for the content nodes of a flow when the element is not part of the DOM
anymore. We already send an unbind event, so the inspector is already notified that the node was removed.

• inspector/InspectorCSSAgent.cpp:

(WebCore::InspectorCSSAgent::didUnregisterNamedFlowContentElement):

Source/WebInspectorUI:

Reviewed by Timothy Hatcher.

Fixed the content node removal from the content flow.

• UserInterface/ContentFlowTreeContentView.js:
• UserInterface/DOMTreeManager.js:

(WebInspector.DOMTreeManager):
(WebInspector.DOMTreeManager.prototype._createContentFlowFromPayload): Registered all the content nodes
in the _contentNodesToFlowsMap.
(WebInspector.DOMTreeManager.prototype._unbind): Added call to _removeContentNodeFromFlowIfNeeded.
(WebInspector.DOMTreeManager.prototype._removeContentNodeFromFlowIfNeeded): Called from _unbind to check
and remove a node from it's parent content flow if needed.
(WebInspector.DOMTreeManager.prototype.unregisteredNamedFlowContentElement):

LayoutTests:

Reviewed by Timothy Hatcher.

Added test to check that the notification that an element was removed from the ContentFlow is handled
correctly in the WebInspector even if the element is not part of the DOM anymore.

• inspector-protocol/model/content-flow-content-removal-expected.txt: Added.
• inspector-protocol/model/content-flow-content-removal.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/inspector-protocol/model/content-flow-content-removal-expected.txt (added)
• LayoutTests/inspector-protocol/model/content-flow-content-removal.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/inspector/InspectorCSSAgent.cpp (modified) (1 diff)
• Source/WebInspectorUI/ChangeLog (modified) (1 diff)
• Source/WebInspectorUI/UserInterface/ContentFlowTreeContentView.js (modified) (1 diff)
• Source/WebInspectorUI/UserInterface/DOMTreeManager.js (modified) (6 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-11-07  Alexandru Chiculita  <achicu@adobe.com>
+
+        Web Inspector: CSS Regions: Removing a content node of a ContentFlow from the DOM will send a 0 nodeId
+        https://bugs.webkit.org/show_bug.cgi?id=123577
+
+        Reviewed by Timothy Hatcher.
+
+        Added test to check that the notification that an element was removed from the ContentFlow is handled
+        correctly in the WebInspector even if the element is not part of the DOM anymore.
+
+        * inspector-protocol/model/content-flow-content-removal-expected.txt: Added.
+        * inspector-protocol/model/content-flow-content-removal.html: Added.
+
 2013-10-24  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-11-07  Alexandru Chiculita  <achicu@adobe.com>
+
+        Web Inspector: CSS Regions: Removing a content node of a ContentFlow from the DOM will send a 0 nodeId
+        https://bugs.webkit.org/show_bug.cgi?id=123577
+
+        Reviewed by Timothy Hatcher.
+
+        Test: inspector-protocol/model/content-flow-content-removal.html
+
+        Do not send unregister events for the content nodes of a flow when the element is not part of the DOM
+        anymore. We already send an unbind event, so the inspector is already notified that the node was removed.
+
+        * inspector/InspectorCSSAgent.cpp:
+        (WebCore::InspectorCSSAgent::didUnregisterNamedFlowContentElement):
+
 2013-10-30  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/inspector/InspectorCSSAgent.cpp

@@ -776 +776 @@
     ErrorString errorString;
     int contentElementNodeId = m_domAgent->pushNodeToFrontend(&errorString, documentNodeId, contentElement);
+    if (!contentElementNodeId) {
+        // We've already notified that the DOM node was removed from the DOM, so there's no need to send another event.
+        return;
+    }
     m_frontend->unregisteredNamedFlowContentElement(documentNodeId, namedFlow->name().string(), contentElementNodeId);
 }

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2013-11-07  Alexandru Chiculita  <achicu@adobe.com>
+
+        Web Inspector: CSS Regions: Removing a content node of a ContentFlow from the DOM will send a 0 nodeId
+        https://bugs.webkit.org/show_bug.cgi?id=123577
+
+        Reviewed by  Timothy Hatcher.
+
+        Fixed the content node removal from the content flow.
+
+        * UserInterface/ContentFlowTreeContentView.js:
+        * UserInterface/DOMTreeManager.js:
+        (WebInspector.DOMTreeManager):
+        (WebInspector.DOMTreeManager.prototype._createContentFlowFromPayload): Registered all the content nodes
+        in the _contentNodesToFlowsMap.
+        (WebInspector.DOMTreeManager.prototype._unbind): Added call to _removeContentNodeFromFlowIfNeeded.
+        (WebInspector.DOMTreeManager.prototype._removeContentNodeFromFlowIfNeeded): Called from _unbind to check
+        and remove a node from it's parent content flow if needed.
+        (WebInspector.DOMTreeManager.prototype.unregisteredNamedFlowContentElement):
+
 2013-11-06  Timothy Hatcher  <timothy@apple.com>
 

trunk/Source/WebInspectorUI/UserInterface/ContentFlowTreeContentView.js

@@ -184 +184 @@
         contentNodeOutline.element.remove();
 
-        this._nodesMap.remove(event.data.node.id);
+        this._nodesMap.delete(event.data.node.id);
     }
 };

trunk/Source/WebInspectorUI/UserInterface/DOMTreeManager.js

@@ -41 +41 @@
     this._attributeLoadNodeIds = {};
     this._flows = {};
+    this._contentNodesToFlowsMap = new Map;
 }
 
@@ -338 +339 @@
     _unbind: function(node)
     {
+        this._removeContentNodeFromFlowIfNeeded(node);
+
         delete this._idToDOMNode[node.id];
         for (var i = 0; node.children && i < node.children.length; ++i)
@@ -537 +540 @@
     {
         // FIXME: Collect the regions from the payload.
-        return new WebInspector.ContentFlow(flowPayload.documentNodeId, flowPayload.name, flowPayload.overset, flowPayload.content.map(this.nodeForId.bind(this)));
+        var flow = new WebInspector.ContentFlow(flowPayload.documentNodeId, flowPayload.name, flowPayload.overset, flowPayload.content.map(this.nodeForId.bind(this)));
+
+        for (var contentNode of flow.contentNodes) {
+            console.assert(!this._contentNodesToFlowsMap.has(contentNode.id));
+            this._contentNodesToFlowsMap.set(contentNode.id, flow);
+        }
+
+        return flow;
     },
 
@@ -558 +568 @@
                 return;
             }
+            this._contentNodesToFlowsMap.clear();
             var contentFlows = [];
             for (var i = 0; i < flows.length; ++i) {
@@ -615 +626 @@
         var flowKey = WebInspector.DOMTreeManager._flowPayloadHashKey({documentNodeId: documentNodeIdentifier, name: flowName});
         console.assert(this._flows.hasOwnProperty(flowKey));
+        console.assert(!this._contentNodesToFlowsMap.has(contentNodeId));
 
         var flow = this._flows[flowKey];
         var contentNode = this.nodeForId(contentNodeId);
+
+        this._contentNodesToFlowsMap.set(contentNode.id, flow);
 
         if (nextContentElementNodeId)
@@ -625 +639 @@
     },
 
+    _removeContentNodeFromFlowIfNeeded: function(node)
+    {
+        if (!this._contentNodesToFlowsMap.has(node.id))
+            return;
+        var flow = this._contentNodesToFlowsMap.get(node.id);
+        this._contentNodesToFlowsMap.delete(node.id);
+        flow.removeContentNode(node);
+    },
+
     unregisteredNamedFlowContentElement: function(documentNodeIdentifier, flowName, contentNodeId)
     {
-        var flowKey = WebInspector.DOMTreeManager._flowPayloadHashKey({documentNodeId: documentNodeIdentifier, name: flowName});
-        console.assert(this._flows.hasOwnProperty(flowKey));
-        this._flows[flowKey].removeContentNode(this.nodeForId(contentNodeId));
+        console.assert(this._contentNodesToFlowsMap.has(contentNodeId));
+
+        var flow = this._contentNodesToFlowsMap.get(contentNodeId);
+        console.assert(flow.id === WebInspector.DOMTreeManager._flowPayloadHashKey({documentNodeId: documentNodeIdentifier, name: flowName}));
+
+        this._contentNodesToFlowsMap.delete(contentNodeId);
+        flow.removeContentNode(this.nodeForId(contentNodeId));
     }
 }