Changeset 241110 in webkit


Ignore:
Timestamp:
Feb 6, 2019 4:31:56 PM (5 years ago)
Author:
Devin Rousso
Message:

Web Inspector: DOM: don't send the entire function string with each event listener
​https://bugs.webkit.org/show_bug.cgi?id=194293
<rdar://problem/47822809>

Reviewed by Joseph Pecoraro.

Source/JavaScriptCore:

  • inspector/protocol/DOM.json:
  • runtime/JSFunction.h:

Export calculatedDisplayName.

Source/WebCore:

Test: inspector/dom/getEventListenersForNode.html

  • inspector/agents/InspectorDOMAgent.cpp:

(WebCore::InspectorDOMAgent::buildObjectForEventListener):

Source/WebInspectorUI:

  • UserInterface/Views/EventListenerSectionGroup.js:

(WI.EventListenerSectionGroup.prototype._functionTextOrLink):

LayoutTests:

  • inspector/dom/getEventListenersForNode.html:
  • inspector/dom/getEventListenersForNode-expected.txt:
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r241105 r241110  
     12019-02-06  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: DOM: don't send the entire function string with each event listener
     4        https://bugs.webkit.org/show_bug.cgi?id=194293
     5        <rdar://problem/47822809>
     6
     7        Reviewed by Joseph Pecoraro.
     8
     9        * inspector/dom/getEventListenersForNode.html:
     10        * inspector/dom/getEventListenersForNode-expected.txt:
     11
    1122019-02-06  Andy Estes  <aestes@apple.com>
    213

  • trunk/LayoutTests/inspector/dom/getEventListenersForNode-expected.txt

    r213873 r241110  
    44== Running test suite: DOM.getEventListenersForNode
    55-- Running test case: DOM.getEventListenersForNode.Basic
    6 Event: alpha
     6Event: A
     7Node: body
     8Capture: true
     9Attribute: false
     10Handler Name: bodyA
     11PASS: The Event Listener has a source location.
     12
     13Event: B
    714Node: body
    815Capture: true
    916Attribute: false
    1017PASS: The Event Listener has a source location.
    11 PASS: The Event Listener has a function.
    1218
    13 Event: beta
     19Event: E
    1420Node: div#x
    1521Capture: false
    1622Attribute: false
    17 Once: true
     23Handler Name: ObjectEventHandler
    1824PASS: The Event Listener has a source location.
    19 PASS: The Event Listener has a function.
    2025
    21 Event: alpha
     26Event: D
     27Node: div#x
     28Capture: false
     29Attribute: false
     30Handler Name: handleEvent
     31PASS: The Event Listener has a source location.
     32
     33Event: C
    2234Node: div#x
    2335Capture: false
    2436Attribute: false
    2537PASS: The Event Listener has a source location.
    26 PASS: The Event Listener has a function.
    2738
    28 Event: alpha
     39Event: B
     40Node: div#x
     41Capture: false
     42Attribute: false
     43Handler Name: xB
     44Once: true
     45PASS: The Event Listener has a source location.
     46
     47Event: A
     48Node: div#x
     49Capture: false
     50Attribute: false
     51Handler Name: xA
     52PASS: The Event Listener has a source location.
     53
     54Event: click
     55Node: div#x
     56Capture: false
     57Attribute: true
     58Handler Name: onclick
     59PASS: The Event Listener has a source location.
     60
     61Event: B
    2962Node: #document
    3063Capture: false
    … …  
    3265Passive: true
    3366PASS: The Event Listener has a source location.
    34 PASS: The Event Listener has a function.
     67
     68Event: A
     69Node: #document
     70Capture: false
     71Attribute: false
     72Handler Name: documentA
     73Passive: true
     74PASS: The Event Listener has a source location.
    3575
    3676

  • trunk/LayoutTests/inspector/dom/getEventListenersForNode.html

    r236766 r241110  
    2828                    InspectorTest.log(`Attribute: ${eventListener.isAttribute}`);
    2929
     30                    if (eventListener.handlerName)
     31                        InspectorTest.log(`Handler Name: ${eventListener.handlerName}`);
    3032                    if (eventListener.passive)
    3133                        InspectorTest.log(`Passive: ${eventListener.passive}`);
    … …  
    3436
    3537                    InspectorTest.expectThat(eventListener.location, "The Event Listener has a source location.");
    36                     InspectorTest.expectThat(eventListener.handlerBody, "The Event Listener has a function.");
    3738
    3839                    InspectorTest.log("");
    … …  
    6364    <p>Testing DOMAgent.getEventListenersForNode.</p>
    6465
    65     <div id="x"></div>
     66    <div id="x" onclick="(function xClick(event) { })()"></div>
    6667    <script>
     68        class ObjectEventHandler {
     69            handleEvent(event) { }
     70        }
     71
    6772        let element = document.getElementById("x");
    68         element.addEventListener("alpha", (event) => {});
    69         element.addEventListener("beta", (event) => {}, {once: true});
     73        element.addEventListener("A", function xA(event) { });
     74        element.addEventListener("B", function xB(event) { }, {once: true});
     75        element.addEventListener("C", (event) => { });
     76        element.addEventListener("D", { handleEvent(event) { } });
     77        element.addEventListener("E", new ObjectEventHandler);
    7078
    71         document.body.addEventListener("alpha", (event) => {}, true);
     79        document.body.addEventListener("A", function bodyA(event) { }, true);
     80        document.body.addEventListener("B", (event) => {}, true);
    7281
    73         document.addEventListener("alpha", (event) => {}, {passive: true});
     82        document.addEventListener("A", function documentA(event) { }, {passive: true});
     83        document.addEventListener("B", (event) => {}, {passive: true});
    7484    </script>
    7585</body>

  • trunk/Source/JavaScriptCore/ChangeLog

    r241104 r241110  
     12019-02-06  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: DOM: don't send the entire function string with each event listener
     4        https://bugs.webkit.org/show_bug.cgi?id=194293
     5        <rdar://problem/47822809>
     6
     7        Reviewed by Joseph Pecoraro.
     8
     9        * inspector/protocol/DOM.json:
     10
     11        * runtime/JSFunction.h:
     12        Export `calculatedDisplayName`.
     13
    1142019-02-06  Yusuke Suzuki  <ysuzuki@apple.com>
    215

  • trunk/Source/JavaScriptCore/inspector/protocol/DOM.json

    r239183 r241110  
    8080                { "name": "isAttribute", "type": "boolean", "description": "<code>EventListener</code>'s isAttribute." },
    8181                { "name": "nodeId", "$ref": "NodeId", "description": "Target <code>DOMNode</code> id." },
    82                 { "name": "handlerBody", "type": "string", "description": "Event handler function body." },
    8382                { "name": "location", "$ref": "Debugger.Location", "optional": true, "description": "Handler code location." },
    84                 { "name": "sourceName", "type": "string", "optional": true, "description": "Source script URL." },
    85                 { "name": "handler", "$ref": "Runtime.RemoteObject", "optional": true, "description": "Event handler function value." },
     83                { "name": "handlerName", "type": "string", "optional": true, "description": "Event handler function name." },
     84                { "name": "handlerObject", "$ref": "Runtime.RemoteObject", "optional": true, "description": "Event handler function value." },
    8685                { "name": "passive", "type": "boolean", "optional": true, "description": "<code>EventListener</code>'s passive." },
    8786                { "name": "once", "type": "boolean", "optional": true, "description": "<code>EventListener</code>'s once." },

  • trunk/Source/JavaScriptCore/runtime/JSFunction.h

    r240965 r241110  
    8989    JS_EXPORT_PRIVATE String name(VM&);
    9090    JS_EXPORT_PRIVATE String displayName(VM&);
    91     const String calculatedDisplayName(VM&);
     91    JS_EXPORT_PRIVATE const String calculatedDisplayName(VM&);
    9292
    9393    ExecutableBase* executable() const { return m_executable.get(); }

  • trunk/Source/WebCore/ChangeLog

    r241105 r241110  
     12019-02-06  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: DOM: don't send the entire function string with each event listener
     4        https://bugs.webkit.org/show_bug.cgi?id=194293
     5        <rdar://problem/47822809>
     6
     7        Reviewed by Joseph Pecoraro.
     8
     9        Test: inspector/dom/getEventListenersForNode.html
     10
     11        * inspector/agents/InspectorDOMAgent.cpp:
     12        (WebCore::InspectorDOMAgent::buildObjectForEventListener):
     13
    1142019-02-06  Andy Estes  <aestes@apple.com>
    215

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

    r239183 r241110  
    16641664    Ref<EventListener> eventListener = registeredEventListener.callback();
    16651665
    1666     JSC::ExecState* state = nullptr;
    1667     JSC::JSObject* handler = nullptr;
    1668     String body;
     1666    JSC::ExecState* exec = nullptr;
     1667    JSC::JSObject* handlerObject = nullptr;
     1668    JSC::JSFunction* handlerFunction = nullptr;
     1669    String handlerName;
    16691670    int lineNumber = 0;
    16701671    int columnNumber = 0;
    16711672    String scriptID;
    1672     String sourceName;
    16731673    if (is<JSEventListener>(eventListener.get())) {
    16741674        auto& scriptListener = downcast<JSEventListener>(eventListener.get());
     1675
    16751676        JSC::JSLockHolder lock(scriptListener.isolatedWorld().vm());
    1676         state = execStateFromNode(scriptListener.isolatedWorld(), &node->document());
    1677         handler = scriptListener.jsFunction(node->document());
    1678         if (handler && state) {
    1679             body = handler->toString(state)->value(state);
    1680             if (auto function = JSC::jsDynamicCast<JSC::JSFunction*>(state->vm(), handler)) {
    1681                 if (!function->isHostOrBuiltinFunction()) {
    1682                     if (auto executable = function->jsExecutable()) {
    1683                         lineNumber = executable->firstLine() - 1;
    1684                         columnNumber = executable->startColumn() - 1;
    1685                         scriptID = executable->sourceID() == JSC::SourceProvider::nullID ? emptyString() : String::number(executable->sourceID());
    1686                         sourceName = executable->sourceURL();
    1687                     }
     1677
     1678        exec = execStateFromNode(scriptListener.isolatedWorld(), &node->document());
     1679        handlerObject = scriptListener.jsFunction(node->document());
     1680        if (handlerObject && exec) {
     1681            handlerFunction = JSC::jsDynamicCast<JSC::JSFunction*>(exec->vm(), handlerObject);
     1682
     1683            if (!handlerFunction) {
     1684                auto scope = DECLARE_CATCH_SCOPE(exec->vm());
     1685
     1686                // If the handler is not actually a function, see if it implements the EventListener interface and use that.
     1687                auto handleEventValue = handlerObject->get(exec, JSC::Identifier::fromString(exec, "handleEvent"));
     1688
     1689                if (UNLIKELY(scope.exception()))
     1690                    scope.clearException();
     1691
     1692                if (handleEventValue)
     1693                    handlerFunction = JSC::jsDynamicCast<JSC::JSFunction*>(exec->vm(), handleEventValue);
     1694            }
     1695
     1696            if (handlerFunction && !handlerFunction->isHostOrBuiltinFunction()) {
     1697                // If the listener implements the EventListener interface, use the class name instead of
     1698                // "handleEvent", unless it is a plain object.
     1699                if (handlerFunction != handlerObject)
     1700                    handlerName = JSC::JSObject::calculatedClassName(handlerObject);
     1701                if (handlerName.isEmpty() || handlerName == "Object"_s)
     1702                    handlerName = handlerFunction->calculatedDisplayName(exec->vm());
     1703
     1704                if (auto executable = handlerFunction->jsExecutable()) {
     1705                    lineNumber = executable->firstLine() - 1;
     1706                    columnNumber = executable->startColumn() - 1;
     1707                    scriptID = executable->sourceID() == JSC::SourceProvider::nullID ? emptyString() : String::number(executable->sourceID());
    16881708                }
    16891709            }
    … …  
    16971717        .setIsAttribute(eventListener->isAttribute())
    16981718        .setNodeId(pushNodePathToFrontend(node))
    1699         .setHandlerBody(body)
    17001719        .release();
    1701     if (objectGroupId && handler && state) {
    1702         InjectedScript injectedScript = m_injectedScriptManager.injectedScriptFor(state);
     1720    if (objectGroupId && handlerObject && exec) {
     1721        InjectedScript injectedScript = m_injectedScriptManager.injectedScriptFor(exec);
    17031722        if (!injectedScript.hasNoValue())
    1704             value->setHandler(injectedScript.wrapObject(handler, *objectGroupId));
     1723            value->setHandlerObject(injectedScript.wrapObject(handlerObject, *objectGroupId));
    17051724    }
    17061725    if (!scriptID.isNull()) {
    … …  
    17111730        location->setColumnNumber(columnNumber);
    17121731        value->setLocation(WTFMove(location));
    1713         if (!sourceName.isEmpty())
    1714             value->setSourceName(sourceName);
    1715     }
     1732    }
     1733    if (!handlerName.isEmpty())
     1734        value->setHandlerName(handlerName);
    17161735    if (registeredEventListener.isPassive())
    17171736        value->setPassive(true);

  • trunk/Source/WebInspectorUI/ChangeLog

    r241039 r241110  
     12019-02-06  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: DOM: don't send the entire function string with each event listener
     4        https://bugs.webkit.org/show_bug.cgi?id=194293
     5        <rdar://problem/47822809>
     6
     7        Reviewed by Joseph Pecoraro.
     8
     9        * UserInterface/Views/EventListenerSectionGroup.js:
     10        (WI.EventListenerSectionGroup.prototype._functionTextOrLink):
     11
    1122019-02-06  Joseph Pecoraro  <pecoraro@apple.com>
    213

  • trunk/Source/WebInspectorUI/UserInterface/Views/EventListenerSectionGroup.js

    r236766 r241110  
    8181    _functionTextOrLink()
    8282    {
    83         var match = this._eventListener.handlerBody.match(/function ([^\(]+?)\(/);
    84         if (match) {
    85             var anonymous = false;
    86             var functionName = match[1];
    87         } else {
    88             var anonymous = true;
    89             var functionName = WI.UIString("(anonymous function)");
     83        let anonymous = false;
     84        let functionName = this._eventListener.handlerName;
     85
     86        // COMPATIBILITY (iOS 12.2): DOM.EventListener.handlerBody was replaced by DOM.EventListener.handlerName.
     87        if (!functionName && this._eventListener.handlerBody) {
     88            let match = this._eventListener.handlerBody.match(/function ([^\(]+?)\(/);
     89            if (match)
     90                functionName = match[1];
     91        }
     92
     93        if (!functionName) {
     94            anonymous = true;
     95            functionName = WI.UIString("(anonymous function)");
    9096        }
    9197
Note: See TracChangeset for help on using the changeset viewer.