[Webkit-unassigned] [Bug 53003] Web Inspector: [JSC] implement setting breakpoints by line:column

[bugzilla-daemon at webkit.org]

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





--- Comment #24 from Pavel Feldman <pfeldman at chromium.org>  2012-06-09 07:17:37 PST ---
(From update of attachment 145731)
View in context: https://bugs.webkit.org/attachment.cgi?id=145731&action=review

> Source/WebCore/ChangeLog:6
> +        RIM PR https://bugzilla.qnx.com/bugzilla/show_bug.cgi?id=152507 depends on this bug.

What is RIM PR?

> Source/WebCore/bindings/js/ScriptDebugServer.cpp:99
> +    std::sort(breaksVector.begin(), breaksVector.end(), breakpointOrderSortFunction);

If breaksVector are breakpoints in the line, why comparing the line numbers in the comparator?

>> Source/WebCore/bindings/js/ScriptDebugServer.cpp:160
>> +    unsigned hitBreaksCount = m_lastHitScriptBreakpoints.size();
> 
> I don't understand the purpose of m_lastHitScriptBreakpoints - isn't it always empty at this point?

I also can't say I'm following the purpose of the m_lastHitScriptBreakpoints.

> Source/WebCore/bindings/js/ScriptDebugServer.cpp:171
> +            m_lastHitScriptBreakpoints.append(breaksVector.at(i));

So at first, you will go here and consider that you should stop on breakpoint no matter whether its position matches the breakpoint column? This looks wrong.
If you set a breakpoint in the formatted script (which originally was a single javascript line), it will map to say line 1, column 42. Then every statement regardless of the offset will be finding this breakpoint and stopping on it.

> Source/WebCore/bindings/js/ScriptDebugServer.cpp:445
> +        m_currentCallFrame->updatePosition(hitPosition);

This sounds like a hack. You should get someone from the JSC team to bless it.

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