Diff
Modified: trunk/LayoutTests/ChangeLog (88949 => 88950)
--- trunk/LayoutTests/ChangeLog 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/LayoutTests/ChangeLog 2011-06-15 17:02:00 UTC (rev 88950)
@@ -1,3 +1,15 @@
+2011-06-15 Alexander Pavlov <[email protected]>
+
+ Reviewed by Pavel Feldman.
+
+ Web Inspector: Serious performance regression during continuous focused element style updates
+ https://bugs.webkit.org/show_bug.cgi?id=61038
+
+ * inspector/elements/edit-dom-actions.html:
+ * inspector/elements/set-attribute.html:
+ * inspector/styles/styles-update-from-js-expected.txt:
+ * inspector/styles/styles-update-from-js.html:
+
2011-06-15 Martin Robinson <[email protected]>
Reclassify a failing GTK+ test.
Modified: trunk/LayoutTests/inspector/elements/edit-dom-actions.html (88949 => 88950)
--- trunk/LayoutTests/inspector/elements/edit-dom-actions.html 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/LayoutTests/inspector/elements/edit-dom-actions.html 2011-06-15 17:02:00 UTC (rev 88950)
@@ -54,7 +54,7 @@
function testBody(node, done)
{
- editNodePartAndRun(node, "webkit-html-attribute", "bar=\"edited attribute\"", done);
+ editNodePartAndRun(node, "webkit-html-attribute", "bar=\"edited attribute\"", done, true);
}
},
@@ -64,7 +64,7 @@
function testBody(node, done)
{
- editNodePartAndRun(node, "webkit-html-attribute", "", done);
+ editNodePartAndRun(node, "webkit-html-attribute", "", done, true);
}
},
@@ -84,7 +84,7 @@
var editorElement = window.getSelection().anchorNode.parentElement;
editorElement.textContent = "newattr=\"new-value\"";
editorElement.dispatchEvent(InspectorTest.createKeyEvent("Enter"));
- InspectorTest.runAfterPendingDispatches(done);
+ InspectorTest.addSniffer(WebInspector.ElementsPanel.prototype, "updateModifiedNodes", done);
}
}
},
@@ -127,7 +127,7 @@
function step2()
{
- InspectorTest.addResult("==== after ====");
+ InspectorTest.addResult("==== after ====");
InspectorTest.dumpElementsTree(testNode);
next();
}
@@ -141,12 +141,15 @@
return textElement;
}
- function editNodePartAndRun(node, className, newValue, step2)
+ function editNodePartAndRun(node, className, newValue, step2, useSniffer)
{
var editorElement = editNodePart(node, className);
editorElement.textContent = newValue;
editorElement.dispatchEvent(InspectorTest.createKeyEvent("Enter"));
- InspectorTest.runAfterPendingDispatches(step2);
+ if (useSniffer)
+ InspectorTest.addSniffer(WebInspector.ElementsPanel.prototype, "updateModifiedNodes", step2);
+ else
+ InspectorTest.runAfterPendingDispatches(step2);
}
}
Modified: trunk/LayoutTests/inspector/elements/set-attribute.html (88949 => 88950)
--- trunk/LayoutTests/inspector/elements/set-attribute.html 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/LayoutTests/inspector/elements/set-attribute.html 2011-06-15 17:02:00 UTC (rev 88950)
@@ -41,7 +41,8 @@
InspectorTest.dumpElementsTree(targetNode);
next();
}
- InspectorTest.evaluateInPage("setAttribute('name', 'value')", callback);
+ InspectorTest.evaluateInPage("setAttribute('name', 'value')");
+ InspectorTest.addSniffer(WebInspector.StylesSidebarPane.prototype, "_rebuildUpdate", callback);
},
function testRemoveAttribute(next) {
@@ -51,7 +52,8 @@
InspectorTest.dumpElementsTree(targetNode);
next();
}
- InspectorTest.evaluateInPage("removeAttribute('name')", callback);
+ InspectorTest.evaluateInPage("removeAttribute('name')");
+ InspectorTest.addSniffer(WebInspector.StylesSidebarPane.prototype, "_rebuildUpdate", callback);
}
]);
}
Modified: trunk/LayoutTests/inspector/styles/styles-update-from-js-expected.txt (88949 => 88950)
--- trunk/LayoutTests/inspector/styles/styles-update-from-js-expected.txt 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/LayoutTests/inspector/styles/styles-update-from-js-expected.txt 2011-06-15 17:02:00 UTC (rev 88950)
@@ -45,4 +45,5 @@
border-left-width: 3px;
+Inline style invalidated 3 times
Modified: trunk/LayoutTests/inspector/styles/styles-update-from-js.html (88949 => 88950)
--- trunk/LayoutTests/inspector/styles/styles-update-from-js.html 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/LayoutTests/inspector/styles/styles-update-from-js.html 2011-06-15 17:02:00 UTC (rev 88950)
@@ -23,10 +23,17 @@
function test()
{
var sniffCount = 0;
+ var inlineStyleInvalidationCount = 0;
InspectorTest.selectNodeWithId("container");
InspectorTest.addSniffer(WebInspector.StylesSidebarPane.prototype, "_rebuildUpdate", snifferCallback, true);
+ InspectorTest.addSniffer(WebInspector.DOMDispatcher.prototype, "inlineStyleInvalidated", inlineStyleInvalidatedSniffer, true);
+ function inlineStyleInvalidatedSniffer()
+ {
+ inlineStyleInvalidationCount += 1;
+ }
+
function snifferCallback()
{
function innerCallback()
@@ -48,6 +55,7 @@
case 3:
InspectorTest.addResult("Modified parsed attributes");
dumpAttributeAndStyles();
+ InspectorTest.addResult("Inline style invalidated " + inlineStyleInvalidationCount + " times");
InspectorTest.completeTest();
break;
}
Modified: trunk/Source/WebCore/ChangeLog (88949 => 88950)
--- trunk/Source/WebCore/ChangeLog 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/Source/WebCore/ChangeLog 2011-06-15 17:02:00 UTC (rev 88950)
@@ -1,3 +1,34 @@
+2011-06-15 Alexander Pavlov <[email protected]>
+
+ Reviewed by Pavel Feldman.
+
+ Web Inspector: Serious performance regression during continuous focused element style updates
+ https://bugs.webkit.org/show_bug.cgi?id=61038
+
+ Inline style invalidation events are coalesced in the backend and sent over the wire on timer.
+
+ * inspector/Inspector.json:
+ * inspector/InspectorDOMAgent.cpp:
+ (WebCore::RevalidateStyleAttributeTask::onTimer):
+ (WebCore::InspectorDOMAgent::getAttributes):
+ (WebCore::InspectorDOMAgent::didModifyDOMAttr):
+ (WebCore::InspectorDOMAgent::styleAttributeInvalidated):
+ * inspector/InspectorDOMAgent.h:
+ * inspector/InspectorStyleSheet.cpp:
+ (WebCore::InspectorStyleSheetForInlineStyle::didModifyElementAttribute):
+ (WebCore::InspectorStyleSheetForInlineStyle::text):
+ (WebCore::InspectorStyleSheetForInlineStyle::setStyleText):
+ (WebCore::InspectorStyleSheetForInlineStyle::ensureParsedDataReady):
+ (WebCore::InspectorStyleSheetForInlineStyle::getStyleAttributeRanges):
+ * inspector/InspectorStyleSheet.h:
+ * inspector/front-end/DOMAgent.js:
+ (WebInspector.DOMAgent):
+ (WebInspector.DOMAgent.prototype._attributesUpdated):
+ (WebInspector.DOMAgent.prototype._loadNodeAttributesSoon):
+ (WebInspector.DOMAgent.prototype._loadNodeAttributes):
+ (WebInspector.DOMDispatcher.prototype.attributesUpdated):
+ (WebInspector.DOMDispatcher.prototype.inlineStyleInvalidated):
+
2011-06-15 Jer Noble <[email protected]>
Reviewed by Timothy Hatcher.
Modified: trunk/Source/WebCore/inspector/Inspector.json (88949 => 88950)
--- trunk/Source/WebCore/inspector/Inspector.json 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/Source/WebCore/inspector/Inspector.json 2011-06-15 17:02:00 UTC (rev 88950)
@@ -782,6 +782,15 @@
{ "name": "lineNumber", "type": "number", "optional": true, "description": "Handler location line number." }
],
"description": "DOM interaction is implemented in terms of mirror objects that represent the actual DOM nodes. DOMNode is a base node mirror type."
+ },
+ {
+ "id": "Attributes",
+ "type": "object",
+ "properties": [
+ { "name": "id", "type": "integer", "description": "Element id to get attributes for." },
+ { "name": "attributes", "type": "array", "items": { "type": "string" }, "description": "An interleaved array of node attribute names and values." }
+ ],
+ "description": "A holder structure for all attributes of a single node."
}
],
"commands": [
@@ -973,6 +982,16 @@
{ "name": "object", "$ref": "Runtime.RemoteObject", "description": "_javascript_ object wrapper for given node." }
],
"description": "Resolves _javascript_ node object for given node id."
+ },
+ {
+ "name": "getAttributes",
+ "parameters": [
+ { "name": "nodeIds", "type": "array", "items": { "type": "integer" }, "description": "Ids of the nodes to retrieve attibutes for." }
+ ],
+ "returns": [
+ { "name": "attributes", "type": "array", "items": { "type": "Attributes" }, "description": "Attribute holders for the requested nodes." }
+ ],
+ "description": "Returns attributes for the specified nodes."
}
],
"events": [
@@ -991,12 +1010,18 @@
{
"name": "attributesUpdated",
"parameters": [
- { "name": "id", "type": "integer", "description": "Id of the node that has changed." },
- { "name": "attributes", "type": "array", "items": { "$ref": "DOMAttribute"}, "description": "New attributes value." }
+ { "name": "nodeId", "type": "integer", "description": "Id of the node that has changed." }
],
"description": "Fired when <code>Element</code>'s attributes are updated."
},
{
+ "name": "inlineStyleInvalidated",
+ "parameters": [
+ { "name": "nodeIds", "type": "array", "items": { "type": "integer" }, "description": "Ids of the nodes for which the inline styles have been invalidated." }
+ ],
+ "description": "Fired when <code>Element</code>'s inline style is modified via a CSS property modification."
+ },
+ {
"name": "characterDataModified",
"parameters": [
{ "name": "id", "type": "integer", "description": "Id of the node that has changed." },
Modified: trunk/Source/WebCore/inspector/InspectorDOMAgent.cpp (88949 => 88950)
--- trunk/Source/WebCore/inspector/InspectorDOMAgent.cpp 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/Source/WebCore/inspector/InspectorDOMAgent.cpp 2011-06-15 17:02:00 UTC (rev 88950)
@@ -249,8 +249,10 @@
void RevalidateStyleAttributeTask::onTimer(Timer<RevalidateStyleAttributeTask>*)
{
// The timer is stopped on m_domAgent destruction, so this method will never be called after m_domAgent has been destroyed.
+ Vector<Element*> elements;
for (HashSet<RefPtr<Element> >::iterator it = m_elements.begin(), end = m_elements.end(); it != end; ++it)
- m_domAgent->didModifyDOMAttr(it->get());
+ elements.append(it->get());
+ m_domAgent->styleAttributeInvalidated(elements);
m_elements.clear();
}
@@ -1022,6 +1024,23 @@
*result = resolveNode(node);
}
+void InspectorDOMAgent::getAttributes(ErrorString*, const RefPtr<InspectorArray>& nodeIds, RefPtr<InspectorArray>* result)
+{
+ for (unsigned i = 0, size = nodeIds->length(); i < size; ++i) {
+ RefPtr<InspectorValue> nodeIdValue = nodeIds->get(i);
+ int nodeId;
+ if (!nodeIdValue->asNumber(&nodeId))
+ continue;
+ Node* node = nodeForId(nodeId);
+ if (node && node->isElementNode()) {
+ RefPtr<InspectorObject> entry = InspectorObject::create();
+ entry->setNumber("id", nodeId);
+ entry->setArray("attributes", buildArrayForElementAttributes(static_cast<Element*>(node)));
+ (*result)->pushObject(entry.release());
+ }
+ }
+}
+
void InspectorDOMAgent::pushNodeToFrontend(ErrorString*, const String& objectId, int* nodeId)
{
InjectedScript injectedScript = m_injectedScriptManager->injectedScriptForObjectId(objectId);
@@ -1302,7 +1321,7 @@
void InspectorDOMAgent::didModifyDOMAttr(Element* element)
{
- int id = m_documentNodeToIdMap.get(element);
+ int id = boundNodeId(element);
// If node is not mapped yet -> ignore the event.
if (!id)
return;
@@ -1310,9 +1329,26 @@
if (m_domListener)
m_domListener->didModifyDOMAttr(element);
- m_frontend->attributesUpdated(id, buildArrayForElementAttributes(element));
+ m_frontend->attributesUpdated(id);
}
+void InspectorDOMAgent::styleAttributeInvalidated(const Vector<Element*>& elements)
+{
+ RefPtr<InspectorArray> nodeIds = InspectorArray::create();
+ for (unsigned i = 0, size = elements.size(); i < size; ++i) {
+ Element* element = elements.at(i);
+ int id = boundNodeId(element);
+ // If node is not mapped yet -> ignore the event.
+ if (!id)
+ continue;
+
+ if (m_domListener)
+ m_domListener->didModifyDOMAttr(element);
+ nodeIds->pushNumber(id);
+ }
+ m_frontend->inlineStyleInvalidated(nodeIds.release());
+}
+
void InspectorDOMAgent::characterDataModified(CharacterData* characterData)
{
int id = m_documentNodeToIdMap.get(characterData);
Modified: trunk/Source/WebCore/inspector/InspectorDOMAgent.h (88949 => 88950)
--- trunk/Source/WebCore/inspector/InspectorDOMAgent.h 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/Source/WebCore/inspector/InspectorDOMAgent.h 2011-06-15 17:02:00 UTC (rev 88950)
@@ -126,6 +126,7 @@
void performSearch(ErrorString*, const String& whitespaceTrimmedQuery, const bool* const runSynchronously);
void cancelSearch(ErrorString*);
void resolveNode(ErrorString*, int nodeId, RefPtr<InspectorObject>* result);
+ void getAttributes(ErrorString*, const RefPtr<InspectorArray>& nodeIds, RefPtr<InspectorArray>* result);
void setInspectModeEnabled(ErrorString*, bool enabled);
void pushNodeToFrontend(ErrorString*, const String& objectId, int* nodeId);
void pushNodeByPathToFrontend(ErrorString*, const String& path, int* nodeId);
@@ -146,6 +147,7 @@
void didInsertDOMNode(Node*);
void didRemoveDOMNode(Node*);
void didModifyDOMAttr(Element*);
+ void styleAttributeInvalidated(const Vector<Element*>& elements);
void characterDataModified(CharacterData*);
void didInvalidateStyleAttr(Node*);
Modified: trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp (88949 => 88950)
--- trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp 2011-06-15 17:02:00 UTC (rev 88950)
@@ -1184,22 +1184,18 @@
void InspectorStyleSheetForInlineStyle::didModifyElementAttribute()
{
- String newStyleText = elementStyleText();
- bool shouldDropSourceData = false;
- if (m_element->isStyledElement() && m_element->style() != m_inspectorStyle->cssStyle()) {
+ m_isStyleTextValid = false;
+ if (m_element->isStyledElement() && m_element->style() != m_inspectorStyle->cssStyle())
m_inspectorStyle = InspectorStyle::create(InspectorCSSId(id(), 0), inlineStyle(), this);
- shouldDropSourceData = true;
- }
- if (newStyleText != m_styleText) {
- m_styleText = newStyleText;
- shouldDropSourceData = true;
- }
- if (shouldDropSourceData)
- m_ruleSourceData.clear();
+ m_ruleSourceData.clear();
}
bool InspectorStyleSheetForInlineStyle::text(String* result) const
{
+ if (!m_isStyleTextValid) {
+ m_styleText = elementStyleText();
+ m_isStyleTextValid = true;
+ }
*result = m_styleText;
return true;
}
@@ -1210,6 +1206,7 @@
ExceptionCode ec = 0;
m_element->setAttribute("style", text, ec);
m_styleText = text;
+ m_isStyleTextValid = true;
m_ruleSourceData.clear();
return !ec;
}
@@ -1226,6 +1223,7 @@
if (m_styleText != currentStyleText) {
m_ruleSourceData.clear();
m_styleText = currentStyleText;
+ m_isStyleTextValid = true;
}
if (m_ruleSourceData)
@@ -1257,7 +1255,7 @@
return m_element->getAttribute("style").string();
}
-bool InspectorStyleSheetForInlineStyle::getStyleAttributeRanges(RefPtr<CSSStyleSourceData>* result)
+bool InspectorStyleSheetForInlineStyle::getStyleAttributeRanges(RefPtr<CSSStyleSourceData>* result) const
{
if (!m_element->isStyledElement())
return false;
Modified: trunk/Source/WebCore/inspector/InspectorStyleSheet.h (88949 => 88950)
--- trunk/Source/WebCore/inspector/InspectorStyleSheet.h 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/Source/WebCore/inspector/InspectorStyleSheet.h 2011-06-15 17:02:00 UTC (rev 88950)
@@ -247,14 +247,15 @@
private:
CSSStyleDeclaration* inlineStyle() const;
const String& elementStyleText() const;
- bool getStyleAttributeRanges(RefPtr<CSSStyleSourceData>* result);
+ bool getStyleAttributeRanges(RefPtr<CSSStyleSourceData>* result) const;
RefPtr<Element> m_element;
RefPtr<CSSRuleSourceData> m_ruleSourceData;
RefPtr<InspectorStyle> m_inspectorStyle;
- // Contains "style" attribute value and should always be up-to-date.
- String m_styleText;
+ // Contains "style" attribute value.
+ mutable String m_styleText;
+ mutable bool m_isStyleTextValid;
};
#endif
Modified: trunk/Source/WebCore/inspector/front-end/DOMAgent.js (88949 => 88950)
--- trunk/Source/WebCore/inspector/front-end/DOMAgent.js 2011-06-15 16:55:00 UTC (rev 88949)
+++ trunk/Source/WebCore/inspector/front-end/DOMAgent.js 2011-06-15 17:02:00 UTC (rev 88950)
@@ -325,6 +325,7 @@
WebInspector.DOMAgent = function() {
this._idToDOMNode = null;
this._document = null;
+ this._attributeLoadNodeIds = {};
InspectorBackend.registerDomainDispatcher("DOM", new WebInspector.DOMDispatcher(this));
}
@@ -413,13 +414,43 @@
this.requestDocument(onDocumentAvailable.bind(this));
},
- _attributesUpdated: function(nodeId, attrsArray)
+ _attributesUpdated: function(nodeIds)
{
- var node = this._idToDOMNode[nodeId];
- node._setAttributesPayload(attrsArray);
- this.dispatchEventToListeners(WebInspector.DOMAgent.Events.AttrModified, node);
+ this._loadNodeAttributesSoon(nodeIds);
},
+ _loadNodeAttributesSoon: function(nodeIds)
+ {
+ for (var i = 0; i < nodeIds.length; ++i)
+ this._attributeLoadNodeIds[nodeIds[i]] = true;
+ if ("_loadNodeAttributesTimeout" in this)
+ return;
+ this._loadNodeAttributesTimeout = setTimeout(this._loadNodeAttributes.bind(this), 0);
+ },
+
+ _loadNodeAttributes: function()
+ {
+ function callback(nodeAttributes)
+ {
+ if (!nodeAttributes)
+ return;
+ for (var i = 0; i < nodeAttributes.length; ++i) {
+ var entry = nodeAttributes[i];
+ var node = this._idToDOMNode[entry.id];
+ node._setAttributesPayload(entry.attributes);
+ this.dispatchEventToListeners(WebInspector.DOMAgent.Events.AttrModified, node);
+ }
+ }
+
+ delete this._loadNodeAttributesTimeout;
+
+ var nodeIds = [];
+ for (var nodeId in this._attributeLoadNodeIds)
+ nodeIds.push(Number(nodeId));
+ DOMAgent.getAttributes(nodeIds, this._wrapClientCallback(callback.bind(this)));
+ this._attributeLoadNodeIds = {};
+ },
+
_characterDataModified: function(nodeId, newValue)
{
var node = this._idToDOMNode[nodeId];
@@ -542,11 +573,16 @@
this._domAgent._documentUpdated();
},
- attributesUpdated: function(nodeId, attrsArray)
+ attributesUpdated: function(nodeId)
{
- this._domAgent._attributesUpdated(nodeId, attrsArray);
+ this._domAgent._attributesUpdated([nodeId]);
},
+ inlineStyleInvalidated: function(nodeIds)
+ {
+ this._domAgent._attributesUpdated(nodeIds);
+ },
+
characterDataModified: function(nodeId, newValue)
{
this._domAgent._characterDataModified(nodeId, newValue);
