WebKit Bugzilla
Attachment 361218 Details for
Bug 194293
: Web Inspector: DOM: don't send the entire function string with each event listener
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[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:
Patch
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>
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194293
:
361218
|
361307
|
361327
|
361335