[Webkit-unassigned] [Bug 37502] Web Inspector: Removes public callLocation API from ScriptCallStack and replaces with ability to get top 10 stack frames

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from jaimeyap at google.com  2010-04-14 07:19:03 PST ---
(In reply to comment #3)
> > Index: WebCore/ChangeLog
> > +
> > +String* InspectorTimelineAgent::getCurrentStackTrace()
> > +{
> > +    if (m_currentStackTrace.isEmpty()) {       
> > +        m_hasStackTrace = ScriptCallStack::stackTrace(&m_currentStackTrace);
> > +        // So that we don't attempt to regrab a strack trace when there is none to grab.
> > +        if (!m_hasStackTrace)
> > +            m_currentStackTrace = "NotEmpty";
> > +    }
> > +    if (m_hasStackTrace)
> > +        return &m_currentStackTrace;
> > +    else
> > +        return 0;
> > +}
> > +
> > +void InspectorTimelineAgent::invalidateStackTrace() {
> > +    m_currentStackTrace = "";
> > +    m_hasStackTrace = false;
> > +}
> 
> We can have a bit complex situation as example with two listeners (an event
> with two nested callFunction events will be generated).
> If the first one generate an install timer event then for the second
> willCallFunction event we assign the call stack of install timer event.

Do the willCallFunctions nest within eachother? Or will they be peers? If they
are peers then it will be sufficient to invalidate the stack when popping
willCallFunctions. 

That being said though, I think I had a rather large flaw in my reasoning about
re-entering javascript. The approach that I thought would work was to
invalidate:
- When pushing willCallFunction or EvalScript.
- When popping willCallFunction or EvalScript.
- When popping the entire record stack, completing a top level event.

But the issue is that control can return to javascript anytime within either of
these two events. I am starting to think that caching the stack trace is in
fact incorrect in the common case. It only "helps" in the cases where layout
and style recalculation run side-by-side before returning control to JS, and in
the case where nodes nest before returning control to JS. The case I intended
to optimize, which is the common case where we have many peer nodes that are
children of some JavaScript execution, would in fact be incorrect to no re-grab
stack traces each time.

I will experiment with reducing the number of stack frames we stringify to see
what overhead we still see. I think I will take out the cache in my subsequent
patch.

> 
> I think it might be interesting to cache call stack in the parent event record
> but I'm not sure that we will have no problem.
> 
> 
> > @@ -811,8 +812,11 @@ WebInspector.TimelinePanel.FormattedReco
> >      } else if (record.type === recordTypes.TimerFire) {
> >          var timerInstalledRecord = timerRecords[record.data.timerId];
> >          if (timerInstalledRecord) {
> > -            this.callSiteScriptName = timerInstalledRecord.callerScriptName;
> > -            this.callSiteScriptLine = timerInstalledRecord.callerScriptLine;
> > +            var stackTrace = timerInstalledRecord.stackTrace;
> > +            if (stackTrace && stackTrace[0]) {
> > +                this.callSiteScriptName = stackTrace[0].scriptName;
> > +                this.callSiteScriptLine = stackTrace[0].scriptLine + 1;                
> > +            }
> 
> 
> We can't use +1 for scriptLine here because JSC numbers are starting from 1. It
> should be done in V8 specific code.
> 

Yes ofcourse :).

> 
> 
> > @@ -918,8 +922,10 @@ WebInspector.TimelinePanel.FormattedReco
> >                  recordContentTable.appendChild(this._createRow(WebInspector.UIString("Details"), this.details));
> >          }
> >  
> > -        if (this.type !== recordTypes.GCEvent && this.callerScriptName) {
> > -            var link = WebInspector.linkifyResourceAsNode(this.callerScriptName, "scripts", this.callerScriptLine);
> > +        if (this.type !== recordTypes.GCEvent && this.stackTrace) {
> > +            var callerScriptName = this.stackTrace[0] ? this.stackTrace[0].scriptName : "";
> > +            var callerScriptLine = this.stackTrace[0] ? this.stackTrace[0].scriptLine + 1 : 0;
> 
> 
> Ditto.

Will fix.

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