Bug 105851 - Web Inspector: show raster tasks on Timeline
Summary: Web Inspector: show raster tasks on Timeline
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: Andrey Kosyakov
URL:
Keywords:
Depends on: 105796
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-29 02:09 PST by Andrey Kosyakov
Modified: 2013-02-27 00:39 PST (History)
9 users (show)

See Also:


Attachments
Patch (10.06 KB, patch)
2012-12-29 02:12 PST, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
Patch (12.63 KB, patch)
2012-12-29 07:43 PST, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
Patch (13.86 KB, patch)
2013-01-14 09:11 PST, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
Patch (17.57 KB, patch)
2013-02-01 04:51 PST, Andrey Kosyakov
pfeldman: review+
Details | Formatted Diff | Diff
Patch (17.40 KB, patch)
2013-02-06 09:26 PST, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
EWS run (26.63 KB, patch)
2013-02-26 08:49 PST, Andrey Kosyakov
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
EWS run (26.59 KB, patch)
2013-02-26 09:16 PST, Andrey Kosyakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Kosyakov 2012-12-29 02:09:27 PST
Add processing of trace events emitted during paint and deferred paint of a layer and display deferred paint (and potential nested events, such as image decode/resize) on the timeline.
Comment 1 Andrey Kosyakov 2012-12-29 02:12:40 PST
Created attachment 180926 [details]
Patch
Comment 2 Pavel Feldman 2012-12-29 04:42:34 PST
Comment on attachment 180926 [details]
Patch

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

> Source/WebCore/inspector/InspectorTimelineAgent.cpp:281
> +        m_traceEventProcessor->willPaint();

It would be great if there was unidirectional dependency in the instrumentation (i.e. processor -> timeline).
Comment 3 Andrey Kosyakov 2012-12-29 07:43:35 PST
Created attachment 180938 [details]
Patch
Comment 4 Pavel Feldman 2013-01-14 06:48:05 PST
Comment on attachment 180938 [details]
Patch

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

r- for introducing page pointer and not using constants. Otherwise looks cryptic and good.

> Source/WebCore/inspector/InspectorInstrumentation.cpp:508
> +    TRACE_EVENT_INSTANT1("webkit", "Instrumentation::Paint", "pageId", frame ? reinterpret_cast<unsigned long long>(frame->page()) : 0);

These need constants. Is there anything we could reuse?

> Source/WebCore/inspector/TimelineTraceEventProcessor.cpp:179
> +TimelineTraceEventProcessor::TimelineTraceEventProcessor(InspectorTimelineAgent* timelineAgent, InspectorClient *client, Page* page)

timeline agent has page agent already.

> Source/WebCore/inspector/TimelineTraceEventProcessor.cpp:182
> +    , m_pageId(reinterpret_cast<unsigned long long>(page))

page can be recreated.

> Source/WebCore/inspector/TimelineTraceEventProcessor.cpp:191
> +    m_handlersByType.set("Instrumentation::Paint", EventTypeEntry(&TimelineTraceEventProcessor::onPaint));

Lets have constants for these in InspectorInstrumentation.
Comment 5 Andrey Kosyakov 2013-01-14 09:11:38 PST
Created attachment 182589 [details]
Patch
Comment 6 Andrey Kosyakov 2013-01-14 09:14:48 PST
(In reply to comment #4)
> 
> These need constants. Is there anything we could reuse?

Added constants to InspectorInstrumentation.{h,cpp}. I don't think we should be re-using existing trace events, as ours are different in that their names are meaningful to other modules.

> > Source/WebCore/inspector/TimelineTraceEventProcessor.cpp:179
> > +TimelineTraceEventProcessor::TimelineTraceEventProcessor(InspectorTimelineAgent* timelineAgent, InspectorClient *client, Page* page)
> 
> timeline agent has page agent already.

Exposed InspectorTimelineAgent::page()

> 
> > Source/WebCore/inspector/TimelineTraceEventProcessor.cpp:182
> > +    , m_pageId(reinterpret_cast<unsigned long long>(page))
> 
> page can be recreated.

Now within the lifetime of TimelineTraceEventProcessor -- we'll kill it upon stopping the record.

> > Source/WebCore/inspector/TimelineTraceEventProcessor.cpp:191
> > +    m_handlersByType.set("Instrumentation::Paint", EventTypeEntry(&TimelineTraceEventProcessor::onPaint));
> 
> Lets have constants for these in InspectorInstrumentation.

done!
Comment 7 Andrey Kosyakov 2013-02-01 04:51:23 PST
Created attachment 186012 [details]
Patch
Comment 8 Andrey Kosyakov 2013-02-06 09:26:45 PST
Created attachment 186871 [details]
Patch
Comment 9 Andrey Kosyakov 2013-02-26 05:14:40 PST
Committed r144044: <http://trac.webkit.org/changeset/144044>
Comment 10 Andrey Kosyakov 2013-02-26 08:49:17 PST
Reopening to attach new patch.
Comment 11 Andrey Kosyakov 2013-02-26 08:49:21 PST
Created attachment 190297 [details]
EWS run
Comment 12 Early Warning System Bot 2013-02-26 09:03:30 PST
Comment on attachment 190297 [details]
EWS run

Attachment 190297 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/16768384
Comment 13 Early Warning System Bot 2013-02-26 09:08:26 PST
Comment on attachment 190297 [details]
EWS run

Attachment 190297 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/16781163
Comment 14 Andrey Kosyakov 2013-02-26 09:16:23 PST
Created attachment 190300 [details]
EWS run
Comment 15 Andrey Kosyakov 2013-02-27 00:39:15 PST
Committed r144154: <http://trac.webkit.org/changeset/144154>