[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





--- Comment #5 from Peter Rybin <prybin at chromium.org>  2012-05-22 19:47:04 PST ---
(In reply to comment #2)
> (From update of attachment 143084 [details])
> 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

function-details is extended. Will be a problem with JSC builds? Do we do any kind of conditioning for V8/JSC?

> > 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.

You said we have polymorphic part and V8-specific part. Should we keep it clear? Anyway, the method is redone now.

> > 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.

I made a bug and added FIXME. I haven't investigated JSC yet on this.

> > Source/WebCore/inspector/InjectedScriptHost.cpp:186
> > +ScriptDebugServer& InjectedScriptHost::getScriptDebugServer()
> We don't use get prefixes in WebKit, simply scriptDebugServer()

Done

> > Source/WebCore/inspector/InjectedScriptSource.js:215
> > +                // FIXME: fix group id
> Group id should be passed as a parameter of the protocol command.
The comment was obsolete. Sorry about this. I inherit group id from the function id (similar to how getProperties works I believe).

> > 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.

Old approach restored.

> > Source/WebCore/inspector/InjectedScriptSource.js:559
> > +    return function(scope_type_code, scope_object, group_id) {
> style: use camelCase for variable names
Done

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