[Webkit-unassigned] [Bug 86861] Web Inspector: Expose function (closure) scopes in remote protocol

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=86861


Yury Semikhatsky <yurys at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #143084|review?                     |review-
               Flag|                            |




--- Comment #2 from Yury Semikhatsky <yurys at chromium.org>  2012-05-21 23:00:33 PST ---
(From update of attachment 143084)
View in context: https://bugs.webkit.org/attachment.cgi?id=143084&action=review

We will need a test for this change. See LayoutTests/inspector/debugger/function-details.html

> Source/WebCore/bindings/v8/ScriptDebugServer.h:117
> +// V8-specific interface section.

Just move the method to the public section above. The comment should be more detailed, otherwise it is confusing as there are other v8-specific methods already. I'd just drop the comment.

> Source/WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:188
> +    InjectedScriptHost* host = V8InjectedScriptHost::toNative(args.Holder());

What would it take to support this for JSC as well? If you can't implement it for JSC, please file a bug and put FIXME with the bug # in the corresponding JS bindings.

> Source/WebCore/inspector/InjectedScriptHost.cpp:186
> +ScriptDebugServer& InjectedScriptHost::getScriptDebugServer()

We don't use get prefixes in WebKit, simply scriptDebugServer()

> Source/WebCore/inspector/InjectedScriptSource.js:215
> +                // FIXME: fix group id

Group id should be passed as a parameter of the protocol command.

> Source/WebCore/inspector/InjectedScriptSource.js:545
> +InjectedScript.CallFrameProxy._createScopeJson = (function() {

No need to use extra closure here. If you are concerned about the performance you may move the types map to InjectedScript.CallFrameProxy.ScopeTypeNames.

> Source/WebCore/inspector/InjectedScriptSource.js:559
> +    return function(scope_type_code, scope_object, group_id) {

style: use camelCase for variable names

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.