Bug 84613

Summary: Insert source file and line number for v8 function calls into tracing
Product: WebKit Reporter: Adrienne Walker <enne>
Component: New BugsAssignee: Adrienne Walker <enne>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, enne, haraken, japhet, nduca, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch haraken: review+

Description Adrienne Walker 2012-04-23 10:45:47 PDT
Insert source file and line number for v8 function calls into tracing
Comment 1 Adrienne Walker 2012-04-23 10:46:39 PDT
Created attachment 138381 [details]
Patch
Comment 2 Adrienne Walker 2012-05-15 16:56:30 PDT
Do any of y'all know who the right person to review this would be?
Comment 3 Kentaro Hara 2012-05-15 17:13:36 PDT
Comment on attachment 138381 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138381&action=review

> Source/WebCore/ChangeLog:7
> +

Please explain the rational for the change.

> Source/WebCore/bindings/v8/V8Proxy.cpp:435
> +    Vector<UChar> combine;
> +    append(combine, resourceName);
> +    combine.append(':');
> +    appendNumber(combine, lineNumber);
> +
> +    String final;
> +    final = String::adopt(combine);
> +    return final;

Shall we use StringBuilder?

> Source/WebCore/platform/chromium/TraceEvent.h:677
> +    explicit TraceStringWithCopy(const unsigned char* str) : m_str(reinterpret_cast<const char*>(str)) { }

What is this change for?
Comment 4 Adrienne Walker 2012-05-16 16:35:49 PDT
Created attachment 142369 [details]
Patch
Comment 5 Adrienne Walker 2012-05-16 16:36:06 PDT
(In reply to comment #3)
> (From update of attachment 138381 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138381&action=review
> 
> > Source/WebCore/ChangeLog:7
> > +
> 
> Please explain the rational for the change.

Done.

> > Source/WebCore/bindings/v8/V8Proxy.cpp:435
> > +    Vector<UChar> combine;
> > +    append(combine, resourceName);
> > +    combine.append(':');
> > +    appendNumber(combine, lineNumber);
> > +
> > +    String final;
> > +    final = String::adopt(combine);
> > +    return final;
> 
> Shall we use StringBuilder?

Done.

> > Source/WebCore/platform/chromium/TraceEvent.h:677
> > +    explicit TraceStringWithCopy(const unsigned char* str) : m_str(reinterpret_cast<const char*>(str)) { }
> 
> What is this change for?

It looks like this is no longer needed.  Removed.
Comment 6 Kentaro Hara 2012-05-16 17:44:11 PDT
Comment on attachment 142369 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142369&action=review

Looks OK

> Source/WebCore/bindings/v8/V8Proxy.cpp:412
> +    builder.append(String::String::number(lineNumber));

String::String::number() => String::number()
Comment 7 Adrienne Walker 2012-05-17 11:22:10 PDT
Committed r117466: <http://trac.webkit.org/changeset/117466>