WebKit Bugzilla
Attachment 361327 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-20190206141211.patch (text/plain), 16.54 KB, created by
Devin Rousso
on 2019-02-06 14:12:12 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Devin Rousso
Created:
2019-02-06 14:12:12 PST
Size:
16.54 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(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>
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