Bug 158218 – Web Inspector: scrolled Snapshot list is reset to top and drawn blank after switching back from Snapshot Comparison view

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

[Blaze Burg]

STEPS TO REPRODUCE:

1. Capture enough snapshots so that the Snapshot List table has scroll overflow.
2. Scroll down to the bottom.
3. Select two snapshots with "Compare Snapshot" button
4. Re-select "Snapshot List" path component in the secondary navigation bar.

EXPECTED:

Scroll position within the data grid is restored

ACTUAL:

Scroll position is reset, and data grid appears empty until scrolling forces relayout/repaint

NOTES:

It looks like the data grid scroll position is reset to top, but the styles of the rows are not re-synced to that position.
I think it's fine to reset the scroll position since we don't integrate secondary view changes into our back-forward list. But, the view shouldn't be blank!

[Radar WebKit Bug Importer]

<rdar://problem/26545000>

[Blaze Burg]

This also seems to happen when switching between Allocations timeline and other timelines if the Snapshot List content view has enough snapshots to cause scroll overflow.

[Timothy Hatcher]

We need to make sure scrollableElements is implemented for these views. This is fallout from my change to cache more things in DataGrid. The DataGrid assumes the scroll position does not change between hide and show to avoid expensive forced layouts.

I thought I got all the Timeline views, but I might have missed these views.

[Nikita Vasilyev]

Created attachment 281659 [details]
[Video] Can't reproduce

I don't see any blank DataGrids. Let me know if I'm missing something.

[Joseph Pecoraro]

I was able to reproduce pretty quickly in r202232. Two issues, one that caused missing records (drawn blank) and one just basic not saving the scroll position:

Steps to Reproduce:
1. Inspect this page
2. Enable the JavaScript Allocations Timeline
3. Start recording
4. Create lots of snapshots (~50+)
5. Scroll all the way down the snapshot list
6. Switch Tab away and switch tabs back
  => List is suddenly scrolled to top
7. Scroll all the way down the snapshot list
8. Compare the last two snapshots
9. Go back to snapshot list
  => List is suddenly scrolled to top, missing records

[Joseph Pecoraro]

Created attachment 281670 [details]
[IMAGE] Issue - Blank Section

[Nikita Vasilyev]

(In reply to comment #5)
> I was able to reproduce pretty quickly in r202232. Two issues, one that
> caused missing records (drawn blank) and one just basic not saving the
> scroll position:
> 
> Steps to Reproduce:
> 1. Inspect this page
> 2. Enable the JavaScript Allocations Timeline
> 3. Start recording
> 4. Create lots of snapshots (~50+)
> 5. Scroll all the way down the snapshot list
> 6. Switch Tab away and switch tabs back
>   => List is suddenly scrolled to top
> 7. Scroll all the way down the snapshot list
> 8. Compare the last two snapshots
> 9. Go back to snapshot list
>   => List is suddenly scrolled to top, missing records

I just reproduced the bug on this page. I'm not sure what was different before.

[Nikita Vasilyev]

Created attachment 281679 [details]
WIP

(In reply to comment #3)
> We need to make sure scrollableElements is implemented for these views. This
> is fallout from my change to cache more things in DataGrid. The DataGrid
> assumes the scroll position does not change between hide and show to avoid
> expensive forced layouts.
> 
> I thought I got all the Timeline views, but I might have missed these views.

I assume you were talking about this change:
https://bugs.webkit.org/show_bug.cgi?id=157709

I added scrollableElements to WebInspector.HeapAllocationsTimelineView
but it didn't fix the bug.

[Timothy Hatcher]

I am unable to reproduce this. I do see the scroll position reset to the top, but the view is not blank.

I'll look into this some more.

[Timothy Hatcher]

I found the issue. Showing the snapshot lists does view swapping, not a full show/hide of a ContentView. So scrollableElements is not used in all cases.

[Timothy Hatcher]

Created attachment 283045 [details]
Patch

[Blaze Burg]

Comment on attachment 283045 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=283045&action=review

r=me

> Source/WebInspectorUI/ChangeLog:15
> +        This was duplicated in the class, removed one.

Lol

[WebKit Commit Bot]

Comment on attachment 283045 [details]
Patch

Clearing flags on attachment: 283045

Committed r202932: <http://trac.webkit.org/changeset/202932>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.