[webkit-reviews] review denied: [Bug 40119] Web Inspector [JSC]: Instrument JS function calls from bindings for Timeline Panel. : [Attachment 123200] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Eli Fidler <efidler at rim.com>'s
request for review:
Bug 40119: Web Inspector [JSC]: Instrument JS function calls from bindings for
Timeline Panel.
https://bugs.webkit.org/show_bug.cgi?id=40119

Attachment 123200: Patch
https://bugs.webkit.org/attachment.cgi?id=123200&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
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?