Bug 101334 - Web Inspector: Timeline: 'undefined' javascript filenames.
Summary: Web Inspector: Timeline: 'undefined' javascript filenames.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: eustas.bug
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-06 04:30 PST by eustas.bug
Modified: 2012-11-14 03:23 PST (History)
12 users (show)

See Also:


Attachments
Patch (3.36 KB, patch)
2012-11-13 23:02 PST, eustas.bug
no flags Details | Formatted Diff | Diff
Patch (3.36 KB, patch)
2012-11-14 00:49 PST, eustas.bug
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description eustas.bug 2012-11-06 04:30:33 PST
Steps to reproduce:
1. Record a trace while navigating in google code search (eg. http://code.google.com/p/chromium/source/search?q=file%3Aeventhandler.cpp)
2. Try to drill into why some events are taking a long time.

Result:
Source locations look like 'undefined:NNNN' and so clicking on them doesn't work.

Original report:
http://code.google.com/p/chromium/issues/detail?id=159413
Comment 1 eustas.bug 2012-11-07 07:54:32 PST
V8 patch:
https://codereview.appspot.com/6811090/
Comment 2 eustas.bug 2012-11-13 23:00:09 PST
Fixed in V8.
Adding test.
Comment 3 eustas.bug 2012-11-13 23:02:01 PST
Created attachment 174082 [details]
Patch
Comment 4 Pavel Feldman 2012-11-13 23:32:24 PST
Comment on attachment 174082 [details]
Patch

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

> LayoutTests/inspector/timeline/timeline-timer-fired-from-eval-call-site.html:24
> +        InspectorTest.evaluateInPage("performActions()");

You should use named function start here as you have finish below.

> LayoutTests/inspector/timeline/timeline-timer-fired-from-eval-call-site.html:30
> +        function formatter(record) {

{ on the next line

> LayoutTests/inspector/timeline/timeline-timer-fired-from-eval-call-site.html:42
> +    setTimeout(performActions, 3000);

Never use setTimeout in tests. Do this from within startTimeline callback
Comment 5 Pavel Feldman 2012-11-14 00:22:48 PST
Comment on attachment 174082 [details]
Patch

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

> LayoutTests/inspector/timeline/timeline-timer-fired-from-eval-call-site.html:41
> +if (!window.testRunner)

Sorry, I take it back - this is debugging!
Comment 6 eustas.bug 2012-11-14 00:39:56 PST
Comment on attachment 174082 [details]
Patch

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

>> LayoutTests/inspector/timeline/timeline-timer-fired-from-eval-call-site.html:24
>> +        InspectorTest.evaluateInPage("performActions()");
> 
> You should use named function start here as you have finish below.

Done

>> LayoutTests/inspector/timeline/timeline-timer-fired-from-eval-call-site.html:30
>> +        function formatter(record) {
> 
> { on the next line

Fixed
Comment 7 eustas.bug 2012-11-14 00:49:27 PST
Created attachment 174096 [details]
Patch
Comment 8 WebKit Review Bot 2012-11-14 01:29:50 PST
Comment on attachment 174096 [details]
Patch

Clearing flags on attachment: 174096

Committed r134573: <http://trac.webkit.org/changeset/134573>
Comment 9 WebKit Review Bot 2012-11-14 01:29:54 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Csaba Osztrogonác 2012-11-14 03:08:22 PST
The new test fails on Mac and Qt:

--- /Volumes/Data/slave/lion-release-tests-wk1/build/layout-test-results/inspector/timeline/timeline-timer-fired-from-eval-call-site-expected.txt
+++ /Volumes/Data/slave/lion-release-tests-wk1/build/layout-test-results/inspector/timeline/timeline-timer-fired-from-eval-call-site-actual.txt
@@ -1,5 +1,5 @@
 Tests the Timeline API instrumentation of a TimerFired events inside evaluated scripts.
 
-TimerFire fromEval.js:2
-TimerFire fromEval.js:1
+TimerFire :2
+TimerFire :1
Comment 11 Pavel Feldman 2012-11-14 03:12:47 PST
We need to land Qt baselines as default and the existing ones should go to under platform/chromium.
Comment 12 Pavel Feldman 2012-11-14 03:23:10 PST
Landed r134589 with that.