Bug 195317 – Web Inspector: recordsInTimeRange sometimes does not get the expected record when includeRecordBeforeStart

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

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

[Joseph Pecoraro]

Created attachment 363616 [details]
[PATCH] Proposed Fix

[Devin Rousso]

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?

[EWS Watchlist]

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

[EWS Watchlist]

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

[Joseph Pecoraro]

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).

[WebKit Commit Bot]

Comment on attachment 363616 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 363616

Committed r242520: <https://trac.webkit.org/changeset/242520>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48620478>