Bug 37267

Summary: Web Inspector: Render Load, DOM Content and MarkTimeline event dividers on Timeline panel.
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, joepeck, loislo, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[IMAGE] Screenshot while running with patch.
none
[PATCH] Proposed change.
yurys: review+
[IMAGE] Screenshot while running with patch. (Updated). none

Description Pavel Feldman 2010-04-08 07:20:44 PDT
Patch to follow.
Comment 1 Pavel Feldman 2010-04-08 07:22:19 PDT
Created attachment 52860 [details]
[IMAGE] Screenshot while running with patch.
Comment 2 Pavel Feldman 2010-04-08 07:32:49 PDT
Created attachment 52861 [details]
[PATCH] Proposed change.
Comment 3 Yury Semikhatsky 2010-04-08 07:39:40 PDT
Comment on attachment 52861 [details]
[PATCH] Proposed change.

>      if (m_mainResource) {
>          m_mainResource->markDOMContentEventTime();
> +        if (m_timelineAgent)
> +            m_timelineAgent->didMarkDOMContentEvent();
Be consistent in naming methods. It should be markDOMContentEventTime, no "did" prefix when there is no corresponding request.


>      if (m_mainResource) {
>          m_mainResource->markLoadEventTime();
> +        if (m_timelineAgent)
> +            m_timelineAgent->didMarkLoadEvent();
Ditto.
Comment 4 Pavel Feldman 2010-04-08 08:22:42 PDT
Created attachment 52869 [details]
[IMAGE] Screenshot while running with patch. (Updated).
Comment 5 Pavel Feldman 2010-04-08 08:23:38 PDT
(In reply to comment #3)
> (From update of attachment 52861 [details])
> >      if (m_mainResource) {
> >          m_mainResource->markDOMContentEventTime();
> > +        if (m_timelineAgent)
> > +            m_timelineAgent->didMarkDOMContentEvent();
> Be consistent in naming methods. It should be markDOMContentEventTime, no "did"
> prefix when there is no corresponding request.
> 

Am happy to do in a separate change (there is a bunch of items like this).

> 
> >      if (m_mainResource) {
> >          m_mainResource->markLoadEventTime();
> > +        if (m_timelineAgent)
> > +            m_timelineAgent->didMarkLoadEvent();
> Ditto.
Comment 6 Pavel Feldman 2010-04-08 08:27:44 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/inspector/timeline-enum-stability-expected.txt
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorController.h
	M	WebCore/inspector/InspectorTimelineAgent.cpp
	M	WebCore/inspector/InspectorTimelineAgent.h
	M	WebCore/inspector/front-end/ResourcesPanel.js
	M	WebCore/inspector/front-end/TimelineAgent.js
	M	WebCore/inspector/front-end/TimelineGrid.js
	M	WebCore/inspector/front-end/TimelineOverviewPane.js
	M	WebCore/inspector/front-end/TimelinePanel.js
	M	WebCore/inspector/front-end/inspector.css
Committed r57280
Comment 7 Timothy Hatcher 2010-04-08 08:29:20 PDT
I wonder if we should have the same look in Resources?
Comment 8 Brian Weinstein 2010-04-08 09:25:34 PDT
Are there tooltips to show what these lines mean? The resources panel had tooltips to say that these were for DOM Content Loaded and the Load event.
Comment 9 Pavel Feldman 2010-04-08 10:54:55 PDT
(In reply to comment #7)
> I wonder if we should have the same look in Resources?

You mean short dividers? They were cluttering timeline a lot, but timeline is more dense than resources. So I don't have strong opinion.

(In reply to comment #8)
> Are there tooltips to show what these lines mean? The resources panel had
> tooltips to say that these were for DOM Content Loaded and the Load event.

Yes, same styles / code patterns are used across timeline and resources.