Bug 84613 - Insert source file and line number for v8 function calls into tracing
Summary: Insert source file and line number for v8 function calls into tracing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrienne Walker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-23 10:45 PDT by Adrienne Walker
Modified: 2012-05-17 11:22 PDT (History)
6 users (show)

See Also:

Attachments
Patch (4.14 KB, patch)
2012-04-23 10:46 PDT, Adrienne Walker 		no flags Details | Formatted Diff | Diff
Patch (3.79 KB, patch)
2012-05-16 16:35 PDT, Adrienne Walker 		haraken: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>