To provide information to the inspector, JS builtins should have some sort of instrumentation system. In this patch, we'll introduce a new private namespace object (like Reflect/Math) @InspectorInstrumentation into the builtin JS. It gives the instrumentation ability to the builtin JS code in JSC. Here's my design. In @fulfillPromise in Operation.Promise.js, function fulfillPromise(...) { ... @InspectorInstrumentation.promiseFulfilled(promise); ... } And in InspectorInstrumentationObject.js (builtin JS code), function promiseFulfilled(promise) { if (!this.isEnabled) return; this.promiseFulfilledImpl(promise); // call C++ code / other inspector code }
Created attachment 258834 [details] Patch Proof of Concept
Created attachment 258835 [details] Patch Proof of Concept, with fix
Comment on attachment 258835 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258835&action=review Looking pretty awesome so far! > Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:52 > + promiseRejected inspectorInstrumentationObjectPromiseRejected DontEnum|Function 3 Do these need to be in the table, even if they are defined inside InspectorImplementationObject.js? > Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:65 > + putDirectWithoutTransition(vm, Identifier::fromString(&vm, "isEnabled"), jsBoolean(false)); Is this InspectorInstrumentation object is instantiated once per context, per VM, or per page? Either way is fine, just need to know what objects we would need to find and set isEnabled for. We usually control whether inspector is enabled or not on a per-Page basis.
Comment on attachment 258835 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258835&action=review Later, I'll update the build fixed patch. >> Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:52 >> + promiseRejected inspectorInstrumentationObjectPromiseRejected DontEnum|Function 3 > > Do these need to be in the table, even if they are defined inside InspectorImplementationObject.js? Currently yes. InspectorImplementationObject.js just define the code. Picking up the function and set it as the method is the role of this table. For example, Operations.Promise.js has the several functions which are not used as the methods of some objects (like rejectPromise). If C++ or JS code becomes so typical, generating code by writing the new script may be nice. >> Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:65 >> + putDirectWithoutTransition(vm, Identifier::fromString(&vm, "isEnabled"), jsBoolean(false)); > > Is this InspectorInstrumentation object is instantiated once per context, per VM, or per page? Either way is fine, just need to know what objects we would need to find and set isEnabled for. We usually control whether inspector is enabled or not on a per-Page basis. Currently, InspectorInstrumentation object is instantiated once per JSGlobalObject (so, window object in WebCore). So, it is per page I think. And if there's the iframe that has the different global object, it has the different InspectorInstrumentation object.
Created attachment 258839 [details] Patch Proof of Concept, with fix, part 2
Comment on attachment 258835 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258835&action=review > Source/JavaScriptCore/runtime/JSGlobalObject.cpp:510 > + GlobalPropertyInfo(vm.propertyNames->builtinNames().InspectorInstrumentationPrivateName(), InspectorInstrumentationObject::create(vm, this, InspectorInstrumentationObject::createStructure(vm, this, m_objectPrototype.get())), DontEnum | DontDelete | ReadOnly), Here, we create the InspectorImplementation object and inject it into the JSGlobalObject's property (so it becomes the global variable with the private symbol name.)
Created attachment 258867 [details] Patch
Created attachment 258885 [details] Patch
Comment on attachment 258885 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258885&action=review > Source/JavaScriptCore/ChangeLog:8 > + This patch adds new private global object, @InspectorInstruments. typo.
Created attachment 258896 [details] Patch
Comment on attachment 258885 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258885&action=review >> Source/JavaScriptCore/ChangeLog:8 >> + This patch adds new private global object, @InspectorInstruments. > > typo. Thank you. Fixed.
Inspector parts look good, would like a JSC reviewer to peek as well.
Comment on attachment 258896 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258896&action=review r=me > Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:49 > + debug inspectorInstrumentationObjectDebug DontEnum|Function 1 Can we just have log, without debug? They seem to be the same. > Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:50 > + dataLogImpl inspectorInstrumentationObjectDataLogImpl DontEnum|Function 1 Let's call this "log".
Comment on attachment 258896 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258896&action=review Thanks! >> Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:49 >> + debug inspectorInstrumentationObjectDebug DontEnum|Function 1 > > Can we just have log, without debug? They seem to be the same. Make sense. I'll drop this function. >> Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:50 >> + dataLogImpl inspectorInstrumentationObjectDataLogImpl DontEnum|Function 1 > > Let's call this "log". And rename it to the "log" :)
Committed r188497: <http://trac.webkit.org/changeset/188497>