Changeset 242080 in webkit

Timestamp:

Feb 26, 2019 12:14:57 AM (5 years ago)

Author:

Devin Rousso

Message:

Web Inspector: navigation sidebar says "No Search Results" when a slow search is in progress
​https://bugs.webkit.org/show_bug.cgi?id=170631
<rdar://problem/29473874>

Reviewed by Joseph Pecoraro.

Keep a count of all the backend commands (increment when firing, decrement when a result is
sent back to the frontend). Once the count comes back to 0, attempt to show the "No Results"
placeholder, since we will have finished searching at that point. Since commands can be called
as a result of other commands, using Promise.all isn't possible.

• UserInterface/Views/SearchSidebarPanel.js:

(WI.SearchSidebarPanel.prototype.performSearch):
(WI.SearchSidebarPanel.prototype.performSearch.updateEmptyContentPlaceholderSoon): Deleted.
(WI.SearchSidebarPanel.prototype.performSearch.updateEmptyContentPlaceholder): Deleted.
Drive-by: replace bind calls with arrow functions, and use for-of loops.

• Localizations/en.lproj/localizedStrings.js:

Location:

trunk/Source/WebInspectorUI

Files:

• ChangeLog (modified) (1 diff)
• Localizations/en.lproj/localizedStrings.js (modified) (1 diff)
• UserInterface/Views/SearchSidebarPanel.js (modified) (8 diffs)

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2019-02-26  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: navigation sidebar says "No Search Results" when a slow search is in progress
+        https://bugs.webkit.org/show_bug.cgi?id=170631
+        <rdar://problem/29473874>
+
+        Reviewed by Joseph Pecoraro.
+
+        Keep a count of all the backend commands (increment when firing, decrement when a result is
+        sent back to the frontend). Once the count comes back to `0`, attempt to show the "No Results"
+        placeholder, since we will have finished searching at that point. Since commands can be called
+        as a result of other commands, using `Promise.all` isn't possible.
+
+        * UserInterface/Views/SearchSidebarPanel.js:
+        (WI.SearchSidebarPanel.prototype.performSearch):
+        (WI.SearchSidebarPanel.prototype.performSearch.updateEmptyContentPlaceholderSoon): Deleted.
+        (WI.SearchSidebarPanel.prototype.performSearch.updateEmptyContentPlaceholder): Deleted.
+        Drive-by: replace `bind` calls with arrow functions, and use for-of loops.
+
+        * Localizations/en.lproj/localizedStrings.js:
+
 2019-02-26  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js

@@ -850 +850 @@
 /* Settings tab label for search related settings */
 localizedStrings["Search: @ Settings"] = "Search:";
+localizedStrings["Searching %s"] = "Searching %s";
 localizedStrings["Secure"] = "Secure";
 localizedStrings["Security"] = "Security";

trunk/Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js

@@ -101 +101 @@
             return;
 
+        let promiseCount = 0;
+        let countPromise = async (promise, callback) => {
+            ++promiseCount;
+            if (promiseCount === 1) {
+                let searchingPlaceholder = WI.createMessageTextView("");
+                String.format(WI.UIString("Searching %s"), [(new WI.IndeterminateProgressSpinner).element], String.standardFormatters, searchingPlaceholder, (a, b) => {
+                    a.append(b);
+                    return a;
+                });
+                this.updateEmptyContentPlaceholder(searchingPlaceholder);
+            }
+
+            let value = await promise;
+
+            if (callback)
+                callback(value);
+
+            --promiseCount;
+            console.assert(promiseCount >= 0);
+            if (promiseCount === 0)
+                this.updateEmptyContentPlaceholder(WI.UIString("No Search Results"));
+        };
+
         let isCaseSensitive = !!this._searchInputSettings.caseSensitive.value;
         let isRegex = !!this._searchInputSettings.regularExpression.value;
-
-        var updateEmptyContentPlaceholderTimeout = null;
 
         function createTreeElementForMatchObject(matchObject, parentTreeElement)
@@ -115 +136 @@
             if (!this.contentTreeOutline.selectedTreeElement)
                 matchTreeElement.revealAndSelect(false, true);
-        }
-
-        function updateEmptyContentPlaceholderSoon()
-        {
-            if (updateEmptyContentPlaceholderTimeout)
-                return;
-            updateEmptyContentPlaceholderTimeout = setTimeout(updateEmptyContentPlaceholder.bind(this), 100);
-        }
-
-        function updateEmptyContentPlaceholder()
-        {
-            if (updateEmptyContentPlaceholderTimeout) {
-                clearTimeout(updateEmptyContentPlaceholderTimeout);
-                updateEmptyContentPlaceholderTimeout = null;
-            }
-
-            this.updateEmptyContentPlaceholder(WI.UIString("No Search Results"));
         }
 
@@ -145 +149 @@
         }
 
-        function resourcesCallback(error, result)
-        {
-            updateEmptyContentPlaceholderSoon.call(this);
-
-            if (error)
-                return;
-
-            function resourceCallback(frameId, url, error, resourceMatches)
-            {
-                updateEmptyContentPlaceholderSoon.call(this);
-
-                if (error || !resourceMatches || !resourceMatches.length)
-                    return;
-
-                var frame = WI.networkManager.frameForIdentifier(frameId);
-                if (!frame)
-                    return;
-
-                var resource = frame.url === url ? frame.mainResource : frame.resourceForURL(url);
-                if (!resource)
-                    return;
-
-                var resourceTreeElement = this._searchTreeElementForResource(resource);
-
-                for (var i = 0; i < resourceMatches.length; ++i) {
-                    var match = resourceMatches[i];
-                    forEachMatch(searchQuery, match.lineContent, (lineMatch, lastIndex) => {
-                        var matchObject = new WI.SourceCodeSearchMatchObject(resource, match.lineContent, searchQuery, new WI.TextRange(match.lineNumber, lineMatch.index, match.lineNumber, lastIndex));
-                        createTreeElementForMatchObject.call(this, matchObject, resourceTreeElement);
-                    });
-                }
-
-                if (!resourceTreeElement.children.length)
-                    this.contentTreeOutline.removeChild(resourceTreeElement);
-
-                updateEmptyContentPlaceholder.call(this);
+        let resourceCallback = (frameId, url, {result}) => {
+            if (!result || !result.length)
+                return;
+
+            var frame = WI.networkManager.frameForIdentifier(frameId);
+            if (!frame)
+                return;
+
+            var resource = frame.url === url ? frame.mainResource : frame.resourceForURL(url);
+            if (!resource)
+                return;
+
+            var resourceTreeElement = this._searchTreeElementForResource(resource);
+
+            for (var i = 0; i < result.length; ++i) {
+                var match = result[i];
+                forEachMatch(searchQuery, match.lineContent, (lineMatch, lastIndex) => {
+                    var matchObject = new WI.SourceCodeSearchMatchObject(resource, match.lineContent, searchQuery, new WI.TextRange(match.lineNumber, lineMatch.index, match.lineNumber, lastIndex));
+                    createTreeElementForMatchObject.call(this, matchObject, resourceTreeElement);
+                });
             }
 
+            if (!resourceTreeElement.children.length)
+                this.contentTreeOutline.removeChild(resourceTreeElement);
+        };
+
+        let resourcesCallback = ({result}) => {
             let preventDuplicates = new Set;
 
-            for (let i = 0; i < result.length; ++i) {
-                let searchResult = result[i];
+            for (let searchResult of result) {
                 if (!searchResult.url || !searchResult.frameId)
                     continue;
@@ -199 +191 @@
 
                 // COMPATIBILITY (iOS 9): Page.searchInResources did not have the optional requestId parameter.
-                PageAgent.searchInResource(searchResult.frameId, searchResult.url, searchQuery, isCaseSensitive, isRegex, searchResult.requestId, resourceCallback.bind(this, searchResult.frameId, searchResult.url));
+                countPromise(PageAgent.searchInResource(searchResult.frameId, searchResult.url, searchQuery, isCaseSensitive, isRegex, searchResult.requestId), resourceCallback.bind(this, searchResult.frameId, searchResult.url));
             }
 
@@ -207 +199 @@
             ];
             Promise.race(promises).then(this._contentChanged.bind(this));
-        }
-
-        function searchScripts(scriptsToSearch)
-        {
-            updateEmptyContentPlaceholderSoon.call(this);
-
+        };
+
+        let scriptCallback = (script, {result}) => {
+            if (!result || !result.length)
+                return;
+
+            var scriptTreeElement = this._searchTreeElementForScript(script);
+
+            for (let match of result) {
+                forEachMatch(searchQuery, match.lineContent, (lineMatch, lastIndex) => {
+                    var matchObject = new WI.SourceCodeSearchMatchObject(script, match.lineContent, searchQuery, new WI.TextRange(match.lineNumber, lineMatch.index, match.lineNumber, lastIndex));
+                    createTreeElementForMatchObject.call(this, matchObject, scriptTreeElement);
+                });
+            }
+
+            if (!scriptTreeElement.children.length)
+                this.contentTreeOutline.removeChild(scriptTreeElement);
+        };
+
+        let searchScripts = (scriptsToSearch) => {
             if (!scriptsToSearch.length)
                 return;
 
-            function scriptCallback(script, error, scriptMatches)
-            {
-                updateEmptyContentPlaceholderSoon.call(this);
-
-                if (error || !scriptMatches || !scriptMatches.length)
+            for (let script of scriptsToSearch)
+                countPromise(script.target.DebuggerAgent.searchInContent(script.id, searchQuery, isCaseSensitive, isRegex), scriptCallback.bind(this, script));
+        };
+
+        let domCallback = ({searchId, resultCount}) => {
+            if (!resultCount)
+                return;
+
+            console.assert(searchId);
+
+            this._domSearchIdentifier = searchId;
+
+            let domSearchResults = ({nodeIds}) => {
+                // If someone started a new search, then return early and stop showing seach results from the old query.
+                if (this._domSearchIdentifier !== searchId)
                     return;
 
-                var scriptTreeElement = this._searchTreeElementForScript(script);
-
-                for (var i = 0; i < scriptMatches.length; ++i) {
-                    var match = scriptMatches[i];
-                    forEachMatch(searchQuery, match.lineContent, (lineMatch, lastIndex) => {
-                        var matchObject = new WI.SourceCodeSearchMatchObject(script, match.lineContent, searchQuery, new WI.TextRange(match.lineNumber, lineMatch.index, match.lineNumber, lastIndex));
-                        createTreeElementForMatchObject.call(this, matchObject, scriptTreeElement);
-                    });
-                }
-
-                if (!scriptTreeElement.children.length)
-                    this.contentTreeOutline.removeChild(scriptTreeElement);
-
-                updateEmptyContentPlaceholder.call(this);
-            }
-
-            for (let script of scriptsToSearch)
-                script.target.DebuggerAgent.searchInContent(script.id, searchQuery, isCaseSensitive, isRegex, scriptCallback.bind(this, script));
-        }
-
-        function domCallback(error, searchId, resultsCount)
-        {
-            updateEmptyContentPlaceholderSoon.call(this);
-
-            if (error || !resultsCount)
-                return;
-
-            console.assert(searchId);
-
-            this._domSearchIdentifier = searchId;
-
-            function domSearchResults(error, nodeIds)
-            {
-                updateEmptyContentPlaceholderSoon.call(this);
-
-                if (error)
-                    return;
-
-                for (var i = 0; i < nodeIds.length; ++i) {
-                    // If someone started a new search, then return early and stop showing seach results from the old query.
-                    if (this._domSearchIdentifier !== searchId)
-                        return;
-
-                    var domNode = WI.domManager.nodeForId(nodeIds[i]);
+                for (let nodeId of nodeIds) {
+                    let domNode = WI.domManager.nodeForId(nodeId);
                     if (!domNode || !domNode.ownerDocument)
                         continue;
@@ -299 +273 @@
                         this.contentTreeOutline.removeChild(resourceTreeElement);
 
-                    updateEmptyContentPlaceholder.call(this);
                 }
-            }
-
-            DOMAgent.getSearchResults(searchId, 0, resultsCount, domSearchResults.bind(this));
-        }
+            };
+
+            countPromise(DOMAgent.getSearchResults(searchId, 0, resultCount), domSearchResults);
+        };
 
         if (window.DOMAgent)
@@ -310 +283 @@
 
         if (window.PageAgent)
-            PageAgent.searchInResources(searchQuery, isCaseSensitive, isRegex, resourcesCallback.bind(this));
+            countPromise(PageAgent.searchInResources(searchQuery, isCaseSensitive, isRegex), resourcesCallback);
 
         setTimeout(searchScripts.bind(this, WI.debuggerManager.searchableScripts), 0);
@@ -320 +293 @@
             }
 
-            DOMAgent.performSearch(searchQuery, domCallback.bind(this));
+            countPromise(DOMAgent.performSearch(searchQuery), domCallback);
         }
 
         // FIXME: Resource search should work in JSContext inspection.
         // <https://webkit.org/b/131252> Web Inspector: JSContext inspection Resource search does not work
-        if (!window.DOMAgent && !window.PageAgent)
-            updateEmptyContentPlaceholderSoon.call(this);
     }