Attachment 361335 Details for Bug 194293 – Patch

[patch] Patch

194293.patch (text/plain), 16.55 KB, created by Devin Rousso on 2019-02-06 15:10:08 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2019-02-06 15:10:08 PST

Size: 16.55 KB

patch

obsolete

>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index fb8389d38d980e86d6792f3b8103ca902ad6f5e2..2a2107005ce6ceaf8eace697aa80e16be077231a 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-02-06  Devin Rousso  <drousso@apple.com>
>+
>+        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.
>+
>+        * inspector/protocol/DOM.json:
>+
>+        * runtime/JSFunction.h:
>+        Export `calculatedDisplayName`.
>+
> 2019-02-05  Mark Lam  <mark.lam@apple.com>
> 
>         Fix DFG's doesGC() for a few more nodes.
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 3896620c7c3a8f2bc319f3726e2be1208929629e..a018c5b1bff32249763fb06921b0ad33663ef730 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-02-06  Devin Rousso  <drousso@apple.com>
>+
>+        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.
>+
>+        Test: inspector/dom/getEventListenersForNode.html
>+
>+        * inspector/agents/InspectorDOMAgent.cpp:
>+        (WebCore::InspectorDOMAgent::buildObjectForEventListener):
>+
> 2019-02-05  Alex Christensen  <achristensen@webkit.org>
> 
>         Stop using blobRegistry in NetworkProcess
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 1c4338aceafe53966bebc23b0f04cc455b52fe32..86505a4e6b03c806433780c5af0d26676469d05b 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-02-06  Devin Rousso  <drousso@apple.com>
>+
>+        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.
>+
>+        * UserInterface/Views/EventListenerSectionGroup.js:
>+        (WI.EventListenerSectionGroup.prototype._functionTextOrLink):
>+
> 2019-02-05  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: Lots of time spent updating related resources in ResourceDetailsSidebar when loading a page with lots of resources
>diff --git a/Source/JavaScriptCore/inspector/protocol/DOM.json b/Source/JavaScriptCore/inspector/protocol/DOM.json
>index 781609d1de96b06f45cafb6e341647b563392283..013c2bb2e31b40699827707edaa237f16c8e3ea6 100644
>--- a/Source/JavaScriptCore/inspector/protocol/DOM.json
>+++ b/Source/JavaScriptCore/inspector/protocol/DOM.json
>@@ -79,10 +79,9 @@
>                 { "name": "useCapture", "type": "boolean", "description": "<code>EventListener</code>'s useCapture." },
>                 { "name": "isAttribute", "type": "boolean", "description": "<code>EventListener</code>'s isAttribute." },
>                 { "name": "nodeId", "$ref": "NodeId", "description": "Target <code>DOMNode</code> id." },
>-                { "name": "handlerBody", "type": "string", "description": "Event handler function body." },
>                 { "name": "location", "$ref": "Debugger.Location", "optional": true, "description": "Handler code location." },
>-                { "name": "sourceName", "type": "string", "optional": true, "description": "Source script URL." },
>-                { "name": "handler", "$ref": "Runtime.RemoteObject", "optional": true, "description": "Event handler function value." },
>+                { "name": "handlerName", "type": "string", "optional": true, "description": "Event handler function name." },
>+                { "name": "handlerObject", "$ref": "Runtime.RemoteObject", "optional": true, "description": "Event handler function value." },
>                 { "name": "passive", "type": "boolean", "optional": true, "description": "<code>EventListener</code>'s passive." },
>                 { "name": "once", "type": "boolean", "optional": true, "description": "<code>EventListener</code>'s once." },
>                 { "name": "disabled", "type": "boolean", "optional": true },
>diff --git a/Source/JavaScriptCore/runtime/JSFunction.h b/Source/JavaScriptCore/runtime/JSFunction.h
>index 3bf067658c3af38e759784e06a7b4dde43823f7e..242c8d6bcb4bddf0a92d0e67aee44bd23aed3862 100644
>--- a/Source/JavaScriptCore/runtime/JSFunction.h
>+++ b/Source/JavaScriptCore/runtime/JSFunction.h
>@@ -88,7 +88,7 @@ public:
> 
>     JS_EXPORT_PRIVATE String name(VM&);
>     JS_EXPORT_PRIVATE String displayName(VM&);
>-    const String calculatedDisplayName(VM&);
>+    JS_EXPORT_PRIVATE const String calculatedDisplayName(VM&);
> 
>     ExecutableBase* executable() const { return m_executable.get(); }
> 
>diff --git a/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp b/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
>index 8797661f234181f6a414bcb93bcb187662525a79..394bd5fe1921c31938f4b102510c90cc147ff852 100644
>--- a/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
>+++ b/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
>@@ -1663,28 +1663,48 @@ Ref<Inspector::Protocol::DOM::EventListener> InspectorDOMAgent::buildObjectForEv
> {
>     Ref<EventListener> eventListener = registeredEventListener.callback();
> 
>-    JSC::ExecState* state = nullptr;
>-    JSC::JSObject* handler = nullptr;
>-    String body;
>+    JSC::ExecState* exec = nullptr;
>+    JSC::JSObject* handlerObject = nullptr;
>+    JSC::JSFunction* handlerFunction = nullptr;
>+    String handlerName;
>     int lineNumber = 0;
>     int columnNumber = 0;
>     String scriptID;
>-    String sourceName;
>     if (is<JSEventListener>(eventListener.get())) {
>         auto& scriptListener = downcast<JSEventListener>(eventListener.get());
>+
>         JSC::JSLockHolder lock(scriptListener.isolatedWorld().vm());
>-        state = execStateFromNode(scriptListener.isolatedWorld(), &node->document());
>-        handler = scriptListener.jsFunction(node->document());
>-        if (handler && state) {
>-            body = handler->toString(state)->value(state);
>-            if (auto function = JSC::jsDynamicCast<JSC::JSFunction*>(state->vm(), handler)) {
>-                if (!function->isHostOrBuiltinFunction()) {
>-                    if (auto executable = function->jsExecutable()) {
>-                        lineNumber = executable->firstLine() - 1;
>-                        columnNumber = executable->startColumn() - 1;
>-                        scriptID = executable->sourceID() == JSC::SourceProvider::nullID ? emptyString() : String::number(executable->sourceID());
>-                        sourceName = executable->sourceURL();
>-                    }
>+
>+        exec = execStateFromNode(scriptListener.isolatedWorld(), &node->document());
>+        handlerObject = scriptListener.jsFunction(node->document());
>+        if (handlerObject && exec) {
>+            handlerFunction = JSC::jsDynamicCast<JSC::JSFunction*>(exec->vm(), handlerObject);
>+
>+            if (!handlerFunction) {
>+                auto scope = DECLARE_CATCH_SCOPE(exec->vm());
>+
>+                // If the handler is not actually a function, see if it implements the EventListener interface and use that.
>+                auto handleEventValue = handlerObject->get(exec, JSC::Identifier::fromString(exec, "handleEvent"));
>+
>+                if (UNLIKELY(scope.exception()))
>+                    scope.clearException();
>+
>+                if (handleEventValue)
>+                    handlerFunction = JSC::jsDynamicCast<JSC::JSFunction*>(exec->vm(), handleEventValue);
>+            }
>+
>+            if (handlerFunction && !handlerFunction->isHostOrBuiltinFunction()) {
>+                // If the listener implements the EventListener interface, use the class name instead of
>+                // "handleEvent", unless it is a plain object.
>+                if (handlerFunction != handlerObject)
>+                    handlerName = JSC::JSObject::calculatedClassName(handlerObject);
>+                if (handlerName.isEmpty() || handlerName == "Object"_s)
>+                    handlerName = handlerFunction->calculatedDisplayName(exec->vm());
>+
>+                if (auto executable = handlerFunction->jsExecutable()) {
>+                    lineNumber = executable->firstLine() - 1;
>+                    columnNumber = executable->startColumn() - 1;
>+                    scriptID = executable->sourceID() == JSC::SourceProvider::nullID ? emptyString() : String::number(executable->sourceID());
>                 }
>             }
>         }
>@@ -1696,12 +1716,11 @@ Ref<Inspector::Protocol::DOM::EventListener> InspectorDOMAgent::buildObjectForEv
>         .setUseCapture(registeredEventListener.useCapture())
>         .setIsAttribute(eventListener->isAttribute())
>         .setNodeId(pushNodePathToFrontend(node))
>-        .setHandlerBody(body)
>         .release();
>-    if (objectGroupId && handler && state) {
>-        InjectedScript injectedScript = m_injectedScriptManager.injectedScriptFor(state);
>+    if (objectGroupId && handlerObject && exec) {
>+        InjectedScript injectedScript = m_injectedScriptManager.injectedScriptFor(exec);
>         if (!injectedScript.hasNoValue())
>-            value->setHandler(injectedScript.wrapObject(handler, *objectGroupId));
>+            value->setHandlerObject(injectedScript.wrapObject(handlerObject, *objectGroupId));
>     }
>     if (!scriptID.isNull()) {
>         auto location = Inspector::Protocol::Debugger::Location::create()
>@@ -1710,9 +1729,9 @@ Ref<Inspector::Protocol::DOM::EventListener> InspectorDOMAgent::buildObjectForEv
>             .release();
>         location->setColumnNumber(columnNumber);
>         value->setLocation(WTFMove(location));
>-        if (!sourceName.isEmpty())
>-            value->setSourceName(sourceName);
>     }
>+    if (!handlerName.isEmpty())
>+        value->setHandlerName(handlerName);
>     if (registeredEventListener.isPassive())
>         value->setPassive(true);
>     if (registeredEventListener.isOnce())
>diff --git a/Source/WebInspectorUI/UserInterface/Views/EventListenerSectionGroup.js b/Source/WebInspectorUI/UserInterface/Views/EventListenerSectionGroup.js
>index 181d059dfd4a2b80843a73715453f705f821a86c..894fed10ff5caa055684ea2ff7892e118f8a705e 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/EventListenerSectionGroup.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/EventListenerSectionGroup.js
>@@ -80,13 +80,19 @@ WI.EventListenerSectionGroup = class EventListenerSectionGroup extends WI.Detail
> 
>     _functionTextOrLink()
>     {
>-        var match = this._eventListener.handlerBody.match(/function ([^\(]+?)\(/);
>-        if (match) {
>-            var anonymous = false;
>-            var functionName = match[1];
>-        } else {
>-            var anonymous = true;
>-            var functionName = WI.UIString("(anonymous function)");
>+        let anonymous = false;
>+        let functionName = this._eventListener.handlerName;
>+
>+        // COMPATIBILITY (iOS 12.2): DOM.EventListener.handlerBody was replaced by DOM.EventListener.handlerName.
>+        if (!functionName && this._eventListener.handlerBody) {
>+            let match = this._eventListener.handlerBody.match(/function ([^\(]+?)\(/);
>+            if (match)
>+                functionName = match[1];
>+        }
>+
>+        if (!functionName) {
>+            anonymous = true;
>+            functionName = WI.UIString("(anonymous function)");
>         }
> 
>         if (!this._eventListener.location)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index b51adadd1224b2a8f32a24bf06afce283fa0555e..574b8b308f557f071dc9714959542f92a456bb70 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-02-06  Devin Rousso  <drousso@apple.com>
>+
>+        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.
>+
>+        * inspector/dom/getEventListenersForNode.html:
>+        * inspector/dom/getEventListenersForNode-expected.txt:
>+
> 2019-02-05  Megan Gardner  <megan_gardner@apple.com>
> 
>         [iOS] Layout tests editing/pasteboard/smart-paste-007.html and editing/pasteboard/smart-paste-008.html are failing
>diff --git a/LayoutTests/inspector/dom/getEventListenersForNode-expected.txt b/LayoutTests/inspector/dom/getEventListenersForNode-expected.txt
>index 26b12256268a2f76fb64f471849fab1ab7f2c469..91194eb96cf61bdcaae7bcee507ca4b39e1350e6 100644
>--- a/LayoutTests/inspector/dom/getEventListenersForNode-expected.txt
>+++ b/LayoutTests/inspector/dom/getEventListenersForNode-expected.txt
>@@ -3,34 +3,74 @@ Testing DOMAgent.getEventListenersForNode.
> 
> == Running test suite: DOM.getEventListenersForNode
> -- Running test case: DOM.getEventListenersForNode.Basic
>-Event: alpha
>+Event: A
>+Node: body
>+Capture: true
>+Attribute: false
>+Handler Name: bodyA
>+PASS: The Event Listener has a source location.
>+
>+Event: B
> Node: body
> Capture: true
> Attribute: false
> PASS: The Event Listener has a source location.
>-PASS: The Event Listener has a function.
> 
>-Event: beta
>+Event: E
> Node: div#x
> Capture: false
> Attribute: false
>+Handler Name: ObjectEventHandler
>+PASS: The Event Listener has a source location.
>+
>+Event: D
>+Node: div#x
>+Capture: false
>+Attribute: false
>+Handler Name: handleEvent
>+PASS: The Event Listener has a source location.
>+
>+Event: C
>+Node: div#x
>+Capture: false
>+Attribute: false
>+PASS: The Event Listener has a source location.
>+
>+Event: B
>+Node: div#x
>+Capture: false
>+Attribute: false
>+Handler Name: xB
> Once: true
> PASS: The Event Listener has a source location.
>-PASS: The Event Listener has a function.
> 
>-Event: alpha
>+Event: A
> Node: div#x
> Capture: false
> Attribute: false
>+Handler Name: xA
> PASS: The Event Listener has a source location.
>-PASS: The Event Listener has a function.
> 
>-Event: alpha
>+Event: click
>+Node: div#x
>+Capture: false
>+Attribute: true
>+Handler Name: onclick
>+PASS: The Event Listener has a source location.
>+
>+Event: B
> Node: #document
> Capture: false
> Attribute: false
> Passive: true
> PASS: The Event Listener has a source location.
>-PASS: The Event Listener has a function.
>+
>+Event: A
>+Node: #document
>+Capture: false
>+Attribute: false
>+Handler Name: documentA
>+Passive: true
>+PASS: The Event Listener has a source location.
> 
> 
>diff --git a/LayoutTests/inspector/dom/getEventListenersForNode.html b/LayoutTests/inspector/dom/getEventListenersForNode.html
>index 697e5326fdd63d96c8a88df3eb3c6ef68bf0f8aa..0d3e667ac95eb92b9c31949c10d32680f70637a8 100644
>--- a/LayoutTests/inspector/dom/getEventListenersForNode.html
>+++ b/LayoutTests/inspector/dom/getEventListenersForNode.html
>@@ -27,13 +27,14 @@ function test() {
>                     InspectorTest.log(`Capture: ${eventListener.useCapture}`);
>                     InspectorTest.log(`Attribute: ${eventListener.isAttribute}`);
> 
>+                    if (eventListener.handlerName)
>+                        InspectorTest.log(`Handler Name: ${eventListener.handlerName}`);
>                     if (eventListener.passive)
>                         InspectorTest.log(`Passive: ${eventListener.passive}`);
>                     if (eventListener.once)
>                         InspectorTest.log(`Once: ${eventListener.once}`);
> 
>                     InspectorTest.expectThat(eventListener.location, "The Event Listener has a source location.");
>-                    InspectorTest.expectThat(eventListener.handlerBody, "The Event Listener has a function.");
> 
>                     InspectorTest.log("");
>                 }
>@@ -62,15 +63,24 @@ function test() {
> <body onload="runTest()">
>     <p>Testing DOMAgent.getEventListenersForNode.</p>
> 
>-    <div id="x"></div>
>+    <div id="x" onclick="(function xClick(event) { })()"></div>
>     <script>
>+        class ObjectEventHandler {
>+            handleEvent(event) { }
>+        }
>+
>         let element = document.getElementById("x");
>-        element.addEventListener("alpha", (event) => {});
>-        element.addEventListener("beta", (event) => {}, {once: true});
>+        element.addEventListener("A", function xA(event) { });
>+        element.addEventListener("B", function xB(event) { }, {once: true});
>+        element.addEventListener("C", (event) => { });
>+        element.addEventListener("D", { handleEvent(event) { } });
>+        element.addEventListener("E", new ObjectEventHandler);
> 
>-        document.body.addEventListener("alpha", (event) => {}, true);
>+        document.body.addEventListener("A", function bodyA(event) { }, true);
>+        document.body.addEventListener("B", (event) => {}, true);
> 
>-        document.addEventListener("alpha", (event) => {}, {passive: true});
>+        document.addEventListener("A", function documentA(event) { }, {passive: true});
>+        document.addEventListener("B", (event) => {}, {passive: true});
>     </script>
> </body>
> </html>

Actions: View | Formatted Diff | Diff

Attachments on bug 194293: 361218 | 361307 | 361327 | 361335