Bug 40119 – Web Inspector [JSC]: Instrument JS function calls from bindings for Timeline Panel.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

timelineAgent->willCallFunction(resourceName, lineNumber); should be called each time some native event handler passes control to the JavaScript function.

[Timothy Hatcher]

This is something V8 has, that we should consider for JSC to match Chrome.

[Eli Fidler]

Created attachment 123200 [details]
Patch

[Pavel Feldman]

Comment on attachment 123200 [details]
Patch

Web Inspector part looks good. Please have someone from the JSC team look at it (JSCallbackData changes and instrumentedCall source / line fetching).

[Eli Fidler]

CC ggaren and olliej for feedback

[Geoffrey Garen]

Comment on attachment 123200 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=123200&action=review

> Source/WebCore/bindings/js/JSCallbackData.cpp:73
> +        Page* page = static_cast<JSDOMWindow*>(globalObject())->impl()->frame()->page();
> +        ASSERT(page);

What guarantees that frame() and page() will not be NULL here? I think you need to NULL check both.

> Source/WebCore/bindings/js/JSMainThreadExecState.h:63
> +            String resourceName("undefined");

Creating the string "undefined" should be conditional -- otherwise, it's a malloc per function call.

> Source/WebCore/bindings/js/JSMainThreadExecState.h:76
> +        InspectorInstrumentation::didCallFunction(cookie);

This code will call didCallFunction even if it didn't call willCallFunction. Is that right?

[Eli Fidler]

Created attachment 123392 [details]
Patch

[Eli Fidler]

addressed ggaren's first two comments.

The didCallFunction even if not willCallFunction paradigm is the same as the V8 implementation. It should work fine.

[Geoffrey Garen]

Comment on attachment 123392 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 123392 [details]
Patch

Clearing flags on attachment: 123392

Committed r106030: <http://trac.webkit.org/changeset/106030>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.