RESOLVED FIXED 37267
Web Inspector: Render Load, DOM Content and MarkTimeline event dividers on Timeline panel.
https://bugs.webkit.org/show_bug.cgi?id=37267
Summary Web Inspector: Render Load, DOM Content and MarkTimeline event dividers on Ti...
Pavel Feldman
Reported 2010-04-08 07:20:44 PDT
Patch to follow.
Attachments
[IMAGE] Screenshot while running with patch. (133.44 KB, image/png)
2010-04-08 07:22 PDT, Pavel Feldman
no flags
[PATCH] Proposed change. (18.30 KB, patch)
2010-04-08 07:32 PDT, Pavel Feldman
yurys: review+
[IMAGE] Screenshot while running with patch. (Updated). (140.34 KB, image/png)
2010-04-08 08:22 PDT, Pavel Feldman
no flags
Pavel Feldman
Comment 1 2010-04-08 07:22:19 PDT
Created attachment 52860 [details] [IMAGE] Screenshot while running with patch.
Pavel Feldman
Comment 2 2010-04-08 07:32:49 PDT
Created attachment 52861 [details] [PATCH] Proposed change.
Yury Semikhatsky
Comment 3 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.
Pavel Feldman
Comment 4 2010-04-08 08:22:42 PDT
Created attachment 52869 [details] [IMAGE] Screenshot while running with patch. (Updated).
Pavel Feldman
Comment 5 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.
Pavel Feldman
Comment 6 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
Timothy Hatcher
Comment 7 2010-04-08 08:29:20 PDT
I wonder if we should have the same look in Resources?
Brian Weinstein
Comment 8 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.
Pavel Feldman
Comment 9 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.
Note You need to log in before you can comment on or make changes to this bug.