| Summary: | Web Inspector: recordsInTimeRange sometimes does not get the expected record when includeRecordBeforeStart | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||
| Component: | Web Inspector | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
Created attachment 363616 [details]
[PATCH] Proposed Fix
Comment on attachment 363616 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=363616&action=review r=me > Source/WebInspectorUI/UserInterface/Models/Timeline.js:116 > + lowerIndex--; You could make this into a do-while :) > Source/WebInspectorUI/UserInterface/Models/Timeline.js:117 > + while (this._records[lowerIndex] !== recordBefore.parent) Is it guaranteed that we'd eventually reach the parent record, or should we also add `&& this._records[lowerIndex].type === recordBefore.parent.type` so it's a bit more restrictive? Comment on attachment 363616 [details] [PATCH] Proposed Fix Attachment 363616 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11375599 New failing tests: fast/scrolling/ios/hit-testing-iframe-002.html fast/images/imagemap-polygon-focus-ring.html fast/scrolling/ios/hit-testing-iframe-003.html editing/selection/thai-word-at-document-end.html fast/scrolling/ios/mixing-user-and-programmatic-scroll-006.html Created attachment 363629 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 363616 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=363616&action=review >> Source/WebInspectorUI/UserInterface/Models/Timeline.js:116 >> + lowerIndex--; > > You could make this into a do-while :) True, I thought this was clearer. >> Source/WebInspectorUI/UserInterface/Models/Timeline.js:117 >> + while (this._records[lowerIndex] !== recordBefore.parent) > > Is it guaranteed that we'd eventually reach the parent record, or should we also add `&& this._records[lowerIndex].type === recordBefore.parent.type` so it's a bit more restrictive? We already did this check up above: `recordBefore.parent && recordBefore.parent.type === recordBefore.type`. Once they are the same type, then yes it should be guaranteed that the parent is in the list (it should have started earlier then the child). Comment on attachment 363616 [details] [PATCH] Proposed Fix Clearing flags on attachment: 363616 Committed r242520: <https://trac.webkit.org/changeset/242520> All reviewed patches have been landed. Closing bug. |
recordsInTimeRange sometimes does not get the expected record when includeRecordBeforeStart For example, when the before record is a composite of a bunch of paints: - Composite: - Paint: ... - Paint: ... - Paint: ... - Paint: ... Currently we get the inner Paint, but that isn't as useful as the Composite, which may be much longer in duration. Steps to Reproduce: 1. Record CPU Timeline on maps.google.com zooming in/out 2. Select part of the large composite and view the CPU Timeline graph => Main Thread indicator doesn't see paints when it should