Attachment 361218 Details for Bug 194293 – Patch

[patch] Patch

bug-194293-20190205135909.patch (text/plain), 13.93 KB, created by Devin Rousso on 2019-02-05 13:59:10 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2019-02-05 13:59:10 PST

Size: 13.93 KB

patch

obsolete

>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 17fc4efc0e2b12455432f4f88187635cf22e4b81..d6084cd3266659df58ecdd1c971e88eacd88d8f0 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-02-05  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 NOBODY (OOPS!).
>+
>+        * inspector/protocol/DOM.json:
>+
>+        * runtime/JSFunction.h:
>+        Export `calculatedDisplayName`.
>+
> 2019-02-04  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         Unreviewed, add missing exception checks after r240637
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index dc74493baf6e8e464b5ec2dae7b0f514cf2747a6..4a15600e86e130599a077cbca989e8403a9e5af7 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-02-05  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 NOBODY (OOPS!).
>+
>+        Test: inspector/dom/getEventListenersForNode.html
>+
>+        * inspector/agents/InspectorDOMAgent.cpp:
>+        (WebCore::InspectorDOMAgent::buildObjectForEventListener):
>+
> 2019-02-04  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Shrink size of VM by lazily allocating IsoSubspaces for non-common types
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 630bec4dfb6c1eba5909bffb509c133e6ba5b701..99269425f1ef51ef7246c8d910357816c2780c90 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-02-05  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 NOBODY (OOPS!).
>+
>+        * UserInterface/Views/EventListenerSectionGroup.js:
>+        (WI.EventListenerSectionGroup.prototype._functionTextOrLink):
>+
> 2019-02-04  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: Resources: missing resource data for document on reload
>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..23ca8b5b8842d6a95598540d0668663d0477947f 100644
>--- a/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
>+++ b/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
>@@ -1664,26 +1664,24 @@ Ref<Inspector::Protocol::DOM::EventListener> InspectorDOMAgent::buildObjectForEv
>     Ref<EventListener> eventListener = registeredEventListener.callback();
> 
>     JSC::ExecState* state = nullptr;
>-    JSC::JSObject* handler = nullptr;
>-    String body;
>+    JSC::JSObject* handlerObject = 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)) {
>+        handlerObject = scriptListener.jsFunction(node->document());
>+        if (handlerObject && state) {
>+            if (auto function = JSC::jsDynamicCast<JSC::JSFunction*>(state->vm(), handlerObject)) {
>+                handlerName = function->calculatedDisplayName(state->vm());
>                 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();
>                     }
>                 }
>             }
>@@ -1696,12 +1694,11 @@ Ref<Inspector::Protocol::DOM::EventListener> InspectorDOMAgent::buildObjectForEv
>         .setUseCapture(registeredEventListener.useCapture())
>         .setIsAttribute(eventListener->isAttribute())
>         .setNodeId(pushNodePathToFrontend(node))
>-        .setHandlerBody(body)
>         .release();
>-    if (objectGroupId && handler && state) {
>+    if (objectGroupId && handlerObject && state) {
>         InjectedScript injectedScript = m_injectedScriptManager.injectedScriptFor(state);
>         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 +1707,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..aa7a446189fb5b2d3a4b6149d25941797346b580 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/EventListenerSectionGroup.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/EventListenerSectionGroup.js
>@@ -80,13 +80,20 @@ 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 dd06b004fc322add7c828253254c3cfb5124dfb8..950c0bb55a2f31fa11fc0e2c531c4e02463b9ba2 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-02-05  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 NOBODY (OOPS!).
>+
>+        * inspector/dom/getEventListenersForNode.html:
>+        * inspector/dom/getEventListenersForNode-expected.txt:
>+
> 2019-02-04  Jonathan Bedard  <jbedard@apple.com>
> 
>         [iPad] Initial test gardening (Part 1)
>diff --git a/LayoutTests/inspector/dom/getEventListenersForNode-expected.txt b/LayoutTests/inspector/dom/getEventListenersForNode-expected.txt
>index 26b12256268a2f76fb64f471849fab1ab7f2c469..4c008f4970486c7e53f33697860731417d19812a 100644
>--- a/LayoutTests/inspector/dom/getEventListenersForNode-expected.txt
>+++ b/LayoutTests/inspector/dom/getEventListenersForNode-expected.txt
>@@ -7,30 +7,49 @@ Event: alpha
> Node: body
> Capture: true
> Attribute: false
>+Handler Name: bodyAlpha
>+PASS: The Event Listener has a source location.
>+
>+Event: beta
>+Node: body
>+Capture: true
>+Attribute: false
>+PASS: The Event Listener has a source location.
>+
>+Event: gamma
>+Node: div#x
>+Capture: false
>+Attribute: false
> PASS: The Event Listener has a source location.
>-PASS: The Event Listener has a function.
> 
> Event: beta
> Node: div#x
> Capture: false
> Attribute: false
>+Handler Name: xBeta
> Once: true
> PASS: The Event Listener has a source location.
>-PASS: The Event Listener has a function.
> 
> Event: alpha
> Node: div#x
> Capture: false
> Attribute: false
>+Handler Name: xAlpha
> PASS: The Event Listener has a source location.
>-PASS: The Event Listener has a function.
> 
>-Event: alpha
>+Event: beta
> Node: #document
> Capture: false
> Attribute: false
> Passive: true
> PASS: The Event Listener has a source location.
>-PASS: The Event Listener has a function.
>+
>+Event: alpha
>+Node: #document
>+Capture: false
>+Attribute: false
>+Handler Name: documentAlpha
>+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..54cd270764737e85896ee6b9efd398dae985390c 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("");
>                 }
>@@ -65,12 +66,15 @@ function test() {
>     <div id="x"></div>
>     <script>
>         let element = document.getElementById("x");
>-        element.addEventListener("alpha", (event) => {});
>-        element.addEventListener("beta", (event) => {}, {once: true});
>+        element.addEventListener("alpha", function xAlpha(event) { });
>+        element.addEventListener("beta", function xBeta(event) { }, {once: true});
>+        element.addEventListener("gamma", (event) => { });
> 
>-        document.body.addEventListener("alpha", (event) => {}, true);
>+        document.body.addEventListener("alpha", function bodyAlpha(event) { }, true);
>+        document.body.addEventListener("beta", (event) => {}, true);
> 
>-        document.addEventListener("alpha", (event) => {}, {passive: true});
>+        document.addEventListener("alpha", function documentAlpha(event) { }, {passive: true});
>+        document.addEventListener("beta", (event) => {}, {passive: true});
>     </script>
> </body>
> </html>

Actions: View | Formatted Diff | Diff

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