Bug 173437

Summary: Web Inspector: Add small delay before showing the progress spinner when loading resources
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, inspector-bugzilla-changes, joepeck, nvasilyev
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
joepeck: review+, joepeck: commit-queue-
Patch
joepeck: review+, joepeck: commit-queue-
Patch none

Description Devin Rousso 2017-06-15 14:20:19 PDT
For small and quickly loading files, it is annoying to see the IndeterminateProgressSpinner briefly appear before being removed almost immediately.
Comment 1 Devin Rousso 2017-06-15 15:04:28 PDT
Created attachment 313014 [details]
Patch
Comment 2 Joseph Pecoraro 2017-06-15 15:37:25 PDT
Comment on attachment 313014 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:13
> +        Delay the creation of the spinner for 100ms.  If the content is available before then, just
> +        clear the timeout and the spinner will never be created/shown.

I know I suggested 100ms, but lets try to arrive at this with more data.

What is the average amount of time it takes for the frontend to load an image when inspecting (1) locally and (2) remotely.

Testing with your Mac Retina laptop should good. I can get you an iOS device to test with.

Once we have that average / bucketed histogram we can provide justification for this currently magic value.
Comment 3 Devin Rousso 2017-06-15 16:59:00 PDT
Created attachment 313027 [details]
Patch
Comment 4 Nikita Vasilyev 2017-06-19 12:01:00 PDT
Comment on attachment 313027 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/ResourceContentView.js:50
> +        console.time("Resource");

This shouldn't be in the patch.
Comment 5 Joseph Pecoraro 2017-06-30 13:57:58 PDT
Comment on attachment 313027 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/ResourceContentView.js:40
> +            // Append a spinner while waiting for contentAvailable. The subclasses are responsible

Nit: "The subclasses are" => "Subclasses are"

You can also say in the comment "... by calling removeLoadingIndicator".

> Source/WebInspectorUI/UserInterface/Views/ResourceContentView.js:46
> +        }, 50);

Lets make this 100 to match the ChangeLog.
Comment 6 Devin Rousso 2017-06-30 14:15:22 PDT
Created attachment 314288 [details]
Patch
Comment 7 WebKit Commit Bot 2017-06-30 14:57:13 PDT
Comment on attachment 314288 [details]
Patch

Clearing flags on attachment: 314288

Committed r219017: <http://trac.webkit.org/changeset/219017>
Comment 8 WebKit Commit Bot 2017-06-30 14:57:14 PDT
All reviewed patches have been landed.  Closing bug.