[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 #25 from Peter Wang <peter.wang at torchmobile.com.cn>  2012-06-10 21:17:53 PST ---
(In reply to comment #24)
> (From update of attachment 145731 [details])
> 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?

It's Problem Report of the BlackBerry Browser. Our Browser cannot support "Pretty print" mode of JS debugging because of this bug of webkit.

> > 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?
Because the breakpoints in m_lastHitScriptBreakpoints are not always in same line. 

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

The m_lastHitScriptBreakpoints is used to record recent hit points. By its help we can figure out what breakpoint should be toggled. For example, if there 3 breakpoints in a same line, and the first and second are already hit, then now in this line the third should hit, even the JSC VM only provide the line number, we can figure out accurately which breakpoint should be toggled. 

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

The information in breaksVector is accurate since it come from Frontend. In the webpage of Inspector the script calculates the breakpoint's position and sends message, finally the setBreakpoints is invoked and save it in breaksVector. 
The problem is that in hasBreakpoints we only know which line we are. So we using m_lastHitScriptBreakpoints to calculate which breakpoint in this line should be toggled. And this breakpoint also should be recorded as recent toggled breakpoints.     

> > 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.
You can take m_curerntCallFrame as a copy of callFrame of JSC VM. The Inspector Frontend send the brekpoint information of it to outside. Since JSC VM only provide the line number, the column number is always 0. Here just to set a accurate column number.

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