<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[219017] trunk/Source/WebInspectorUI</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/219017">219017</a></dd>
<dt>Author</dt> <dd>drousso@apple.com</dd>
<dt>Date</dt> <dd>2017-06-30 14:57:09 -0700 (Fri, 30 Jun 2017)</dd>
</dl>

<h3>Log Message</h3>
<pre>Web Inspector: Add small delay before showing the progress spinner when loading resources
https://bugs.webkit.org/show_bug.cgi?id=173437

Reviewed by Joseph Pecoraro.

* UserInterface/Views/ResourceContentView.js:
(WebInspector.ResourceContentView):
(WebInspector.ResourceContentView.prototype.removeLoadingIndicator): Added.
(WebInspector.ResourceContentView.prototype._contentError):
(WebInspector.ResourceContentView.prototype._hasContent):
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.

We measured an average of 35ms to load and display images with slow cases being around 55ms.
100ms was chosen for the timeout to give some room to allow for abnormally slow loading
while not being too long as to be outright noticable.

* UserInterface/Views/FontResourceContentView.js:
(WebInspector.FontResourceContentView.prototype.contentAvailable):
* UserInterface/Views/ImageResourceContentView.js:
(WebInspector.ImageResourceContentView.prototype.contentAvailable):
* UserInterface/Views/TextResourceContentView.js:
(WebInspector.TextResourceContentView.prototype._contentWillPopulate):
Calls the new protected function removeLoadingIndicator to ensure that the spinner (and any
other element) is removed.

This is necessary because TextResourceContentView effectively has two phases of loading its
content: getting the content and formatting it for display.  The first follows the same path
as the other ResourceContentView subclasses, the second waits for the ContentWillPopulate
event on SourceCodeTextEditor before it actually adds the content as a subview.  In this
case, the spinner should only be removed right before the content is actually added, not
once it's ready.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebInspectorUIChangeLog">trunk/Source/WebInspectorUI/ChangeLog</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsFontResourceContentViewjs">trunk/Source/WebInspectorUI/UserInterface/Views/FontResourceContentView.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsImageResourceContentViewjs">trunk/Source/WebInspectorUI/UserInterface/Views/ImageResourceContentView.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsResourceContentViewjs">trunk/Source/WebInspectorUI/UserInterface/Views/ResourceContentView.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsTextResourceContentViewjs">trunk/Source/WebInspectorUI/UserInterface/Views/TextResourceContentView.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebInspectorUIChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/ChangeLog (219016 => 219017)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/ChangeLog    2017-06-30 21:52:53 UTC (rev 219016)
+++ trunk/Source/WebInspectorUI/ChangeLog       2017-06-30 21:57:09 UTC (rev 219017)
</span><span class="lines">@@ -1,3 +1,38 @@
</span><ins>+2017-06-30  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Add small delay before showing the progress spinner when loading resources
+        https://bugs.webkit.org/show_bug.cgi?id=173437
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Views/ResourceContentView.js:
+        (WebInspector.ResourceContentView):
+        (WebInspector.ResourceContentView.prototype.removeLoadingIndicator): Added.
+        (WebInspector.ResourceContentView.prototype._contentError):
+        (WebInspector.ResourceContentView.prototype._hasContent):
+        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.
+
+        We measured an average of 35ms to load and display images with slow cases being around 55ms.
+        100ms was chosen for the timeout to give some room to allow for abnormally slow loading
+        while not being too long as to be outright noticable.
+
+        * UserInterface/Views/FontResourceContentView.js:
+        (WebInspector.FontResourceContentView.prototype.contentAvailable):
+        * UserInterface/Views/ImageResourceContentView.js:
+        (WebInspector.ImageResourceContentView.prototype.contentAvailable):
+        * UserInterface/Views/TextResourceContentView.js:
+        (WebInspector.TextResourceContentView.prototype._contentWillPopulate):
+        Calls the new protected function removeLoadingIndicator to ensure that the spinner (and any
+        other element) is removed.
+
+        This is necessary because TextResourceContentView effectively has two phases of loading its
+        content: getting the content and formatting it for display.  The first follows the same path
+        as the other ResourceContentView subclasses, the second waits for the ContentWillPopulate
+        event on SourceCodeTextEditor before it actually adds the content as a subview.  In this
+        case, the spinner should only be removed right before the content is actually added, not
+        once it's ready.
+
</ins><span class="cx"> 2017-06-30  Commit Queue  <commit-queue@webkit.org>
</span><span class="cx"> 
</span><span class="cx">         Unreviewed, rolling out r218983.
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsFontResourceContentViewjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/FontResourceContentView.js (219016 => 219017)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Views/FontResourceContentView.js       2017-06-30 21:52:53 UTC (rev 219016)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/FontResourceContentView.js  2017-06-30 21:57:09 UTC (rev 219017)
</span><span class="lines">@@ -72,7 +72,7 @@
</span><span class="cx">         if (this._styleElement && this._styleElement.parentNode)
</span><span class="cx">             this._styleElement.parentNode.removeChild(this._styleElement);
</span><span class="cx"> 
</span><del>-        this.element.removeChildren();
</del><ins>+        this.removeLoadingIndicator();
</ins><span class="cx"> 
</span><span class="cx">         this._styleElement = document.createElement("style");
</span><span class="cx">         this._styleElement.textContent = "@font-face { font-family: \"" + uniqueFontName + "\"; src: url(" + this._fontObjectURL + ")" + format + "; }";
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsImageResourceContentViewjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ImageResourceContentView.js (219016 => 219017)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Views/ImageResourceContentView.js      2017-06-30 21:52:53 UTC (rev 219016)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ImageResourceContentView.js 2017-06-30 21:57:09 UTC (rev 219017)
</span><span class="lines">@@ -53,7 +53,7 @@
</span><span class="cx">             return;
</span><span class="cx">         }
</span><span class="cx"> 
</span><del>-        this.element.removeChildren();
</del><ins>+        this.removeLoadingIndicator();
</ins><span class="cx"> 
</span><span class="cx">         this._imageElement = document.createElement("img");
</span><span class="cx">         this._imageElement.addEventListener("load", function() { URL.revokeObjectURL(objectURL); });
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsResourceContentViewjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ResourceContentView.js (219016 => 219017)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Views/ResourceContentView.js   2017-06-30 21:52:53 UTC (rev 219016)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ResourceContentView.js      2017-06-30 21:57:09 UTC (rev 219017)
</span><span class="lines">@@ -36,11 +36,15 @@
</span><span class="cx"> 
</span><span class="cx">         this.element.classList.add(styleClassName, "resource");
</span><span class="cx"> 
</span><del>-        // Append a spinner while waiting for contentAvailable. The subclasses are responsible for removing
-        // the spinner before showing the resource content.
-        var spinner = new WebInspector.IndeterminateProgressSpinner;
-        this.element.appendChild(spinner.element);
</del><ins>+        this._spinnerTimeout = setTimeout(() => {
+            // Append a spinner while waiting for contentAvailable. Subclasses are responsible for
+            // removing the spinner before showing the resource content by calling removeLoadingIndicator.
+            let spinner = new WebInspector.IndeterminateProgressSpinner;
+            this.element.appendChild(spinner.element);
</ins><span class="cx"> 
</span><ins>+            this._spinnerTimeout = undefined;
+        }, 100);
+
</ins><span class="cx">         this.element.addEventListener("click", this._mouseWasClicked.bind(this), false);
</span><span class="cx"> 
</span><span class="cx">         // Request content last so the spinner will always be removed in case the content is immediately available.
</span><span class="lines">@@ -97,6 +101,18 @@
</span><span class="cx">             WebInspector.issueManager.removeEventListener(null, null, this);
</span><span class="cx">     }
</span><span class="cx"> 
</span><ins>+    // Protected
+
+    removeLoadingIndicator()
+    {
+        if (this._spinnerTimeout) {
+            clearTimeout(this._spinnerTimeout);
+            this._spinnerTimeout = undefined;
+        }
+
+        this.element.removeChildren();
+    }
+
</ins><span class="cx">     // Private
</span><span class="cx"> 
</span><span class="cx">     _contentAvailable(parameters)
</span><span class="lines">@@ -117,7 +133,8 @@
</span><span class="cx">         if (this._hasContent())
</span><span class="cx">             return;
</span><span class="cx"> 
</span><del>-        this.element.removeChildren();
</del><ins>+        this.removeLoadingIndicator();
+
</ins><span class="cx">         this.element.appendChild(WebInspector.createMessageTextView(error, true));
</span><span class="cx"> 
</span><span class="cx">         this.dispatchEventToListeners(WebInspector.ResourceContentView.Event.ContentError);
</span><span class="lines">@@ -125,7 +142,7 @@
</span><span class="cx"> 
</span><span class="cx">     _hasContent()
</span><span class="cx">     {
</span><del>-        return !this.element.querySelector(".indeterminate-progress-spinner");
</del><ins>+        return this.element.hasChildNodes() && !this.element.querySelector(".indeterminate-progress-spinner");
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     _issueWasAdded(event)
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsTextResourceContentViewjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/TextResourceContentView.js (219016 => 219017)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Views/TextResourceContentView.js       2017-06-30 21:52:53 UTC (rev 219016)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/TextResourceContentView.js  2017-06-30 21:57:09 UTC (rev 219017)
</span><span class="lines">@@ -191,7 +191,8 @@
</span><span class="cx">         if (this._textEditor.parentView === this)
</span><span class="cx">             return;
</span><span class="cx"> 
</span><del>-        this.element.removeChildren();
</del><ins>+        this.removeLoadingIndicator();
+
</ins><span class="cx">         this.addSubview(this._textEditor);
</span><span class="cx">     }
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>