[webkit-reviews] review denied: [Bug 36839] Web Inspector: Adds the ability to get the function symbol name when looking up call location. : [Attachment 52645] Updated patchset according to Pavel's suggestions

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied jaimeyap at google.com's request
for review:
Bug 36839: Web Inspector: Adds the ability to get the function symbol name when
looking up call location.
https://bugs.webkit.org/show_bug.cgi?id=36839

Attachment 52645: Updated patchset according to Pavel's suggestions
https://bugs.webkit.org/attachment.cgi?id=52645&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
> +    // Compile JavaScript function for retrieving the source line, the
source
> +    // name and the symbol name for the top JavaScript stack frame.
> +    DEFINE_STATIC_LOCAL(const char*, lastCallFrame,
> +	   ("function lastCallFrame(exec_state) {"
> +	   "  var frame = exec_state.frame(0);"
Please check that the stack is not empty before accessing first frame. TOT
WebKit already contains exec_state.frameCount() check, it seem that you copy
needs update.

> +    
> +    // Function for retrieving the source name, line number and function
name for the top
> +    // JavaScript stack frame.
> +    //
> +    // It will return true if the caller information was successfully
retrieved and written
> +    // into the function parameters, otherwise the function will return
false. It may
> +    // fail due to a stck overflow in the underlying JavaScript
implentation, handling
> +    // of such exception is up to the caller.
> +    static bool lastCallFrame(String& sourceName, int& lineNumber, String&
funcName);
>  
Consider naming it topCallFrame.


Please add frameCount check, otherwise r+.