Bug 110819

Summary: Web Inspector: Save/load timeline should preserve DOMContentLoaded and Load event markers
Product: WebKit Reporter: Tony Gentilcore <tonyg>
Component: Web Inspector (Deprecated)Assignee: Eugene Klyuchnikov <eustas>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, buildbot, dglazkov, keishi, loislo, pfeldman, pmuellr, rniwa, vsevik, web-inspector-bugs, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
pfeldman: review+
Patch none

Tony Gentilcore
Reported 2013-02-25 16:12:35 PST
1. Record a timeline that includes a page load. Notice the presence of DOMContentLoaded and Load event markers. 2. Right click and "Save Timeline data..." 3. Close inspector, open a new inspector, right click on timeline and "Load Timeline data..." 4. Notice DOMContentLoaded and Load event markers are gone. I expect DOMContentLoaded and Load event markers should survive saving and loading the timeline.
Attachments
Patch (4.95 KB, patch)
2013-02-26 09:44 PST, Eugene Klyuchnikov
no flags
Patch (6.08 KB, patch)
2013-02-26 21:38 PST, Eugene Klyuchnikov
no flags
Patch (6.22 KB, patch)
2013-02-26 22:39 PST, Eugene Klyuchnikov
pfeldman: review+
Patch (6.22 KB, patch)
2013-03-01 04:02 PST, Eugene Klyuchnikov
no flags
Eugene Klyuchnikov
Comment 1 2013-02-26 09:44:24 PST
Build Bot
Comment 2 2013-02-26 12:31:34 PST
Comment on attachment 190315 [details] Patch Attachment 190315 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/16781302 New failing tests: inspector/timeline/timeline-load-event.html inspector/timeline/timeline-dom-content-loaded-event.html
WebKit Review Bot
Comment 3 2013-02-26 13:39:05 PST
Comment on attachment 190315 [details] Patch Attachment 190315 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/16782211 New failing tests: inspector/timeline/timeline-load-event.html inspector/timeline/timeline-dom-content-loaded-event.html
Build Bot
Comment 4 2013-02-26 16:39:03 PST
Comment on attachment 190315 [details] Patch Attachment 190315 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/16769558 New failing tests: inspector/timeline/timeline-dom-content-loaded-event.html inspector/timeline/timeline-load-event.html
Eugene Klyuchnikov
Comment 5 2013-02-26 21:38:40 PST
Ilya Tikhonovsky
Comment 6 2013-02-26 21:45:38 PST
Comment on attachment 190438 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190438&action=review > Source/WebCore/ChangeLog:3 > + Save/load timeline should preserve DOMContentLoaded and Load event markers Please provide Web Inspector prefix. > Source/WebCore/ChangeLog:8 > + Store "isMainFrame" flag in MarkLoad and MarkDOMConatent records data. It is not clear from the description what was the root of problem and how it was fixed.
Eugene Klyuchnikov
Comment 7 2013-02-26 22:35:09 PST
Analysis: TimelinePresentationModel creates event dividers for MarkLoad and MarkDOMContent only if they are fired for main frame. It uses resource resourceTreeModel to check if given frame id is main frame id. This is not good because at time check is performed, main frame could have changed (for example when we load saved timeline data). With this patch check is performed at time record is created and result is saved as a part of record.
Eugene Klyuchnikov
Comment 8 2013-02-26 22:38:44 PST
Comment on attachment 190438 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190438&action=review >> Source/WebCore/ChangeLog:3 >> + Save/load timeline should preserve DOMContentLoaded and Load event markers > > Please provide Web Inspector prefix. Fixed. >> Source/WebCore/ChangeLog:8 >> + Store "isMainFrame" flag in MarkLoad and MarkDOMConatent records data. > > It is not clear from the description what was the root of problem and how it was fixed. Fixed.
Eugene Klyuchnikov
Comment 9 2013-02-26 22:39:41 PST
Pavel Feldman
Comment 10 2013-03-01 01:16:18 PST
Comment on attachment 190447 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190447&action=review > Source/WebCore/inspector/TimelineRecordFactory.cpp:190 > + data->setBoolean("mainFrame", mainFrame); isMainFrame
Eugene Klyuchnikov
Comment 11 2013-03-01 04:02:57 PST
Eugene Klyuchnikov
Comment 12 2013-03-01 04:04:10 PST
Comment on attachment 190447 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190447&action=review >> Source/WebCore/inspector/TimelineRecordFactory.cpp:190 >> + data->setBoolean("mainFrame", mainFrame); > > isMainFrame Done
Eugene Klyuchnikov
Comment 13 2013-03-01 04:06:41 PST
Note You need to log in before you can comment on or make changes to this bug.