Changeset 88950 in webkit


Ignore:
Timestamp:
Jun 15, 2011 10:02:00 AM (13 years ago)
Author:
apavlov@chromium.org
Message:

2011-06-15 Alexander Pavlov <​apavlov@chromium.org>

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 Alexander Pavlov <​apavlov@chromium.org>

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):
Location:
trunk
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r88949 r88950  
     12011-06-15  Alexander Pavlov  <apavlov@chromium.org>
     2
     3        Reviewed by Pavel Feldman.
     4
     5        Web Inspector: Serious performance regression during continuous focused element style updates
     6        https://bugs.webkit.org/show_bug.cgi?id=61038
     7
     8        * inspector/elements/edit-dom-actions.html:
     9        * inspector/elements/set-attribute.html:
     10        * inspector/styles/styles-update-from-js-expected.txt:
     11        * inspector/styles/styles-update-from-js.html:
     12
    1132011-06-15  Martin Robinson  <mrobinson@igalia.com>
    214

  • trunk/LayoutTests/inspector/elements/edit-dom-actions.html

    r86570 r88950  
    5555            function testBody(node, done)
    5656            {
    57                 editNodePartAndRun(node, "webkit-html-attribute", "bar=\"edited attribute\"", done);
     57                editNodePartAndRun(node, "webkit-html-attribute", "bar=\"edited attribute\"", done, true);
    5858            }
    5959        },
    … …  
    6565            function testBody(node, done)
    6666            {
    67                 editNodePartAndRun(node, "webkit-html-attribute", "", done);
     67                editNodePartAndRun(node, "webkit-html-attribute", "", done, true);
    6868            }
    6969        },
    … …  
    8585                    editorElement.textContent = "newattr=\"new-value\"";
    8686                    editorElement.dispatchEvent(InspectorTest.createKeyEvent("Enter"));
    87                     InspectorTest.runAfterPendingDispatches(done);
     87                    InspectorTest.addSniffer(WebInspector.ElementsPanel.prototype, "updateModifiedNodes", done);
    8888                }
    8989            }
    … …  
    128128        function step2()
    129129        {
    130                 InspectorTest.addResult("==== after ====");
     130            InspectorTest.addResult("==== after ====");
    131131            InspectorTest.dumpElementsTree(testNode);
    132132            next();
    … …  
    142142    }
    143143
    144     function editNodePartAndRun(node, className, newValue, step2)
     144    function editNodePartAndRun(node, className, newValue, step2, useSniffer)
    145145    {
    146146        var editorElement = editNodePart(node, className);
    147147        editorElement.textContent = newValue;
    148148        editorElement.dispatchEvent(InspectorTest.createKeyEvent("Enter"));
    149         InspectorTest.runAfterPendingDispatches(step2);
     149        if (useSniffer)
     150            InspectorTest.addSniffer(WebInspector.ElementsPanel.prototype, "updateModifiedNodes", step2);
     151        else
     152            InspectorTest.runAfterPendingDispatches(step2);
    150153    }
    151154}

  • trunk/LayoutTests/inspector/elements/set-attribute.html

    r78576 r88950  
    4242                next();
    4343            }
    44             InspectorTest.evaluateInPage("setAttribute('name', 'value')", callback);
     44            InspectorTest.evaluateInPage("setAttribute('name', 'value')");
     45            InspectorTest.addSniffer(WebInspector.StylesSidebarPane.prototype, "_rebuildUpdate", callback);
    4546        },
    4647
    … …  
    5253                next();
    5354            }
    54             InspectorTest.evaluateInPage("removeAttribute('name')", callback);
     55            InspectorTest.evaluateInPage("removeAttribute('name')");
     56            InspectorTest.addSniffer(WebInspector.StylesSidebarPane.prototype, "_rebuildUpdate", callback);
    5557        }
    5658    ]);

  • trunk/LayoutTests/inspector/styles/styles-update-from-js-expected.txt

    r88337 r88950  
    4646
    4747
     48Inline style invalidated 3 times
    4849

  • trunk/LayoutTests/inspector/styles/styles-update-from-js.html

    r88347 r88950  
    2424{
    2525    var sniffCount = 0;
     26    var inlineStyleInvalidationCount = 0;
    2627
    2728    InspectorTest.selectNodeWithId("container");
    2829    InspectorTest.addSniffer(WebInspector.StylesSidebarPane.prototype, "_rebuildUpdate", snifferCallback, true);
     30    InspectorTest.addSniffer(WebInspector.DOMDispatcher.prototype, "inlineStyleInvalidated", inlineStyleInvalidatedSniffer, true);
     31
     32    function inlineStyleInvalidatedSniffer()
     33    {
     34        inlineStyleInvalidationCount += 1;
     35    }
    2936
    3037    function snifferCallback()
    … …  
    4956                    InspectorTest.addResult("Modified parsed attributes");
    5057                    dumpAttributeAndStyles();
     58                    InspectorTest.addResult("Inline style invalidated " + inlineStyleInvalidationCount + " times");
    5159                    InspectorTest.completeTest();
    5260                    break;

  • trunk/Source/WebCore/ChangeLog

    r88948 r88950  
     12011-06-15  Alexander Pavlov  <apavlov@chromium.org>
     2
     3        Reviewed by Pavel Feldman.
     4
     5        Web Inspector: Serious performance regression during continuous focused element style updates
     6        https://bugs.webkit.org/show_bug.cgi?id=61038
     7
     8        Inline style invalidation events are coalesced in the backend and sent over the wire on timer.
     9
     10        * inspector/Inspector.json:
     11        * inspector/InspectorDOMAgent.cpp:
     12        (WebCore::RevalidateStyleAttributeTask::onTimer):
     13        (WebCore::InspectorDOMAgent::getAttributes):
     14        (WebCore::InspectorDOMAgent::didModifyDOMAttr):
     15        (WebCore::InspectorDOMAgent::styleAttributeInvalidated):
     16        * inspector/InspectorDOMAgent.h:
     17        * inspector/InspectorStyleSheet.cpp:
     18        (WebCore::InspectorStyleSheetForInlineStyle::didModifyElementAttribute):
     19        (WebCore::InspectorStyleSheetForInlineStyle::text):
     20        (WebCore::InspectorStyleSheetForInlineStyle::setStyleText):
     21        (WebCore::InspectorStyleSheetForInlineStyle::ensureParsedDataReady):
     22        (WebCore::InspectorStyleSheetForInlineStyle::getStyleAttributeRanges):
     23        * inspector/InspectorStyleSheet.h:
     24        * inspector/front-end/DOMAgent.js:
     25        (WebInspector.DOMAgent):
     26        (WebInspector.DOMAgent.prototype._attributesUpdated):
     27        (WebInspector.DOMAgent.prototype._loadNodeAttributesSoon):
     28        (WebInspector.DOMAgent.prototype._loadNodeAttributes):
     29        (WebInspector.DOMDispatcher.prototype.attributesUpdated):
     30        (WebInspector.DOMDispatcher.prototype.inlineStyleInvalidated):
     31
    1322011-06-15  Jer Noble  <jer.noble@apple.com>
    233

  • trunk/Source/WebCore/inspector/Inspector.json

    r88943 r88950  
    783783                ],
    784784                "description": "DOM interaction is implemented in terms of mirror objects that represent the actual DOM nodes. DOMNode is a base node mirror type."
     785            },
     786            {
     787                "id": "Attributes",
     788                "type": "object",
     789                "properties": [
     790                    { "name": "id", "type": "integer", "description": "Element id to get attributes for." },
     791                    { "name": "attributes", "type": "array", "items": { "type": "string" }, "description": "An interleaved array of node attribute names and values." }
     792                ],
     793                "description": "A holder structure for all attributes of a single node."
    785794            }
    786795        ],
    … …  
    974983                ],
    975984                "description": "Resolves JavaScript node object for given node id."
     985            },
     986            {
     987                "name": "getAttributes",
     988                "parameters": [
     989                    { "name": "nodeIds", "type": "array", "items": { "type": "integer" }, "description": "Ids of the nodes to retrieve attibutes for." }
     990                ],
     991                "returns": [
     992                    { "name": "attributes", "type": "array", "items": { "type": "Attributes" }, "description": "Attribute holders for the requested nodes." }
     993                ],
     994                "description": "Returns attributes for the specified nodes."
    976995            }
    977996        ],
    … …  
    9921011                "name": "attributesUpdated",
    9931012                "parameters": [
    994                     { "name": "id", "type": "integer", "description": "Id of the node that has changed." },
    995                     { "name": "attributes", "type": "array", "items": { "$ref": "DOMAttribute"}, "description": "New attributes value." }
     1013                    { "name": "nodeId", "type": "integer", "description": "Id of the node that has changed." }
    9961014                ],
    9971015                "description": "Fired when <code>Element</code>'s attributes are updated."
     1016            },
     1017            {
     1018                "name": "inlineStyleInvalidated",
     1019                "parameters": [
     1020                    { "name": "nodeIds", "type": "array", "items": { "type": "integer" }, "description": "Ids of the nodes for which the inline styles have been invalidated." }
     1021                ],
     1022                "description": "Fired when <code>Element</code>'s inline style is modified via a CSS property modification."
    9981023            },
    9991024            {

  • trunk/Source/WebCore/inspector/InspectorDOMAgent.cpp

    r88331 r88950  
    250250{
    251251    // The timer is stopped on m_domAgent destruction, so this method will never be called after m_domAgent has been destroyed.
     252    Vector<Element*> elements;
    252253    for (HashSet<RefPtr<Element> >::iterator it = m_elements.begin(), end = m_elements.end(); it != end; ++it)
    253         m_domAgent->didModifyDOMAttr(it->get());
     254        elements.append(it->get());
     255    m_domAgent->styleAttributeInvalidated(elements);
    254256
    255257    m_elements.clear();
    … …  
    10211023    }
    10221024    *result = resolveNode(node);
     1025}
     1026
     1027void InspectorDOMAgent::getAttributes(ErrorString*, const RefPtr<InspectorArray>& nodeIds, RefPtr<InspectorArray>* result)
     1028{
     1029    for (unsigned i = 0, size = nodeIds->length(); i < size; ++i) {
     1030        RefPtr<InspectorValue> nodeIdValue = nodeIds->get(i);
     1031        int nodeId;
     1032        if (!nodeIdValue->asNumber(&nodeId))
     1033            continue;
     1034        Node* node = nodeForId(nodeId);
     1035        if (node && node->isElementNode()) {
     1036            RefPtr<InspectorObject> entry = InspectorObject::create();
     1037            entry->setNumber("id", nodeId);
     1038            entry->setArray("attributes", buildArrayForElementAttributes(static_cast<Element*>(node)));
     1039            (*result)->pushObject(entry.release());
     1040        }
     1041    }
    10231042}
    10241043
    … …  
    13031322void InspectorDOMAgent::didModifyDOMAttr(Element* element)
    13041323{
    1305     int id = m_documentNodeToIdMap.get(element);
     1324    int id = boundNodeId(element);
    13061325    // If node is not mapped yet -> ignore the event.
    13071326    if (!id)
    … …  
    13111330        m_domListener->didModifyDOMAttr(element);
    13121331
    1313     m_frontend->attributesUpdated(id, buildArrayForElementAttributes(element));
     1332    m_frontend->attributesUpdated(id);
     1333}
     1334
     1335void InspectorDOMAgent::styleAttributeInvalidated(const Vector<Element*>& elements)
     1336{
     1337    RefPtr<InspectorArray> nodeIds = InspectorArray::create();
     1338    for (unsigned i = 0, size = elements.size(); i < size; ++i) {
     1339        Element* element = elements.at(i);
     1340        int id = boundNodeId(element);
     1341        // If node is not mapped yet -> ignore the event.
     1342        if (!id)
     1343            continue;
     1344
     1345        if (m_domListener)
     1346            m_domListener->didModifyDOMAttr(element);
     1347        nodeIds->pushNumber(id);
     1348    }
     1349    m_frontend->inlineStyleInvalidated(nodeIds.release());
    13141350}
    13151351

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

    r88331 r88950  
    127127    void cancelSearch(ErrorString*);
    128128    void resolveNode(ErrorString*, int nodeId, RefPtr<InspectorObject>* result);
     129    void getAttributes(ErrorString*, const RefPtr<InspectorArray>& nodeIds, RefPtr<InspectorArray>* result);
    129130    void setInspectModeEnabled(ErrorString*, bool enabled);
    130131    void pushNodeToFrontend(ErrorString*, const String& objectId, int* nodeId);
    … …  
    147148    void didRemoveDOMNode(Node*);
    148149    void didModifyDOMAttr(Element*);
     150    void styleAttributeInvalidated(const Vector<Element*>& elements);
    149151    void characterDataModified(CharacterData*);
    150152    void didInvalidateStyleAttr(Node*);

  • trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp

    r85603 r88950  
    11851185void InspectorStyleSheetForInlineStyle::didModifyElementAttribute()
    11861186{
    1187     String newStyleText = elementStyleText();
    1188     bool shouldDropSourceData = false;
    1189     if (m_element->isStyledElement() && m_element->style() != m_inspectorStyle->cssStyle()) {
     1187    m_isStyleTextValid = false;
     1188    if (m_element->isStyledElement() && m_element->style() != m_inspectorStyle->cssStyle())
    11901189        m_inspectorStyle = InspectorStyle::create(InspectorCSSId(id(), 0), inlineStyle(), this);
    1191         shouldDropSourceData = true;
    1192     }
    1193     if (newStyleText != m_styleText) {
    1194         m_styleText = newStyleText;
    1195         shouldDropSourceData = true;
    1196     }
    1197     if (shouldDropSourceData)
    1198         m_ruleSourceData.clear();
     1190    m_ruleSourceData.clear();
    11991191}
    12001192
    12011193bool InspectorStyleSheetForInlineStyle::text(String* result) const
    12021194{
     1195    if (!m_isStyleTextValid) {
     1196        m_styleText = elementStyleText();
     1197        m_isStyleTextValid = true;
     1198    }
    12031199    *result = m_styleText;
    12041200    return true;
    … …  
    12111207    m_element->setAttribute("style", text, ec);
    12121208    m_styleText = text;
     1209    m_isStyleTextValid = true;
    12131210    m_ruleSourceData.clear();
    12141211    return !ec;
    … …  
    12271224        m_ruleSourceData.clear();
    12281225        m_styleText = currentStyleText;
     1226        m_isStyleTextValid = true;
    12291227    }
    12301228
    … …  
    12581256}
    12591257
    1260 bool InspectorStyleSheetForInlineStyle::getStyleAttributeRanges(RefPtr<CSSStyleSourceData>* result)
     1258bool InspectorStyleSheetForInlineStyle::getStyleAttributeRanges(RefPtr<CSSStyleSourceData>* result) const
    12611259{
    12621260    if (!m_element->isStyledElement())

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

    r84148 r88950  
    248248    CSSStyleDeclaration* inlineStyle() const;
    249249    const String& elementStyleText() const;
    250     bool getStyleAttributeRanges(RefPtr<CSSStyleSourceData>* result);
     250    bool getStyleAttributeRanges(RefPtr<CSSStyleSourceData>* result) const;
    251251
    252252    RefPtr<Element> m_element;
    … …  
    254254    RefPtr<InspectorStyle> m_inspectorStyle;
    255255
    256     // Contains "style" attribute value and should always be up-to-date.
    257     String m_styleText;
     256    // Contains "style" attribute value.
     257    mutable String m_styleText;
     258    mutable bool m_isStyleTextValid;
    258259};
    259260

  • trunk/Source/WebCore/inspector/front-end/DOMAgent.js

    r88331 r88950  
    326326    this._idToDOMNode = null;
    327327    this._document = null;
     328    this._attributeLoadNodeIds = {};
    328329    InspectorBackend.registerDomainDispatcher("DOM", new WebInspector.DOMDispatcher(this));
    329330}
    … …  
    414415    },
    415416
    416     _attributesUpdated: function(nodeId, attrsArray)
    417     {
    418         var node = this._idToDOMNode[nodeId];
    419         node._setAttributesPayload(attrsArray);
    420         this.dispatchEventToListeners(WebInspector.DOMAgent.Events.AttrModified, node);
     417    _attributesUpdated: function(nodeIds)
     418    {
     419        this._loadNodeAttributesSoon(nodeIds);
     420    },
     421
     422    _loadNodeAttributesSoon: function(nodeIds)
     423    {
     424        for (var i = 0; i < nodeIds.length; ++i)
     425            this._attributeLoadNodeIds[nodeIds[i]] = true;
     426        if ("_loadNodeAttributesTimeout" in this)
     427            return;
     428        this._loadNodeAttributesTimeout = setTimeout(this._loadNodeAttributes.bind(this), 0);
     429    },
     430
     431    _loadNodeAttributes: function()
     432    {
     433        function callback(nodeAttributes)
     434        {
     435            if (!nodeAttributes)
     436                return;
     437            for (var i = 0; i < nodeAttributes.length; ++i) {
     438                var entry = nodeAttributes[i];
     439                var node = this._idToDOMNode[entry.id];
     440                node._setAttributesPayload(entry.attributes);
     441                this.dispatchEventToListeners(WebInspector.DOMAgent.Events.AttrModified, node);
     442            }
     443        }
     444
     445        delete this._loadNodeAttributesTimeout;
     446
     447        var nodeIds = [];
     448        for (var nodeId in this._attributeLoadNodeIds)
     449            nodeIds.push(Number(nodeId));
     450        DOMAgent.getAttributes(nodeIds, this._wrapClientCallback(callback.bind(this)));
     451        this._attributeLoadNodeIds = {};
    421452    },
    422453
    … …  
    543574    },
    544575
    545     attributesUpdated: function(nodeId, attrsArray)
    546     {
    547         this._domAgent._attributesUpdated(nodeId, attrsArray);
     576    attributesUpdated: function(nodeId)
     577    {
     578        this._domAgent._attributesUpdated([nodeId]);
     579    },
     580
     581    inlineStyleInvalidated: function(nodeIds)
     582    {
     583        this._domAgent._attributesUpdated(nodeIds);
    548584    },
    549585
Note: See TracChangeset for help on using the changeset viewer.