Attachment 361856 Details for Bug 170631 – Patch

[patch] Patch

bug-170631-20190212154846.patch (text/plain), 12.46 KB, created by Devin Rousso on 2019-02-12 15:48:46 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2019-02-12 15:48:46 PST

Size: 12.46 KB

patch

obsolete

>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 6689a2b17cf6ceae32f54fa56eae3944a692e3e1..c6d951b4788432309f5ad8fa675b19f80635916d 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-02-12  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 NOBODY (OOPS!).
>+
>+        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-12  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Web Inspector: Timeline.prototype.recordsInTimeRange uses a property most records do not have
>diff --git a/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js b/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
>index b673168064d3ce4d7dda2263132ead19ee6b8533..5d2a309000985e68427257643795e5417d416d10 100644
>--- a/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
>+++ b/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
>@@ -829,6 +829,7 @@ localizedStrings["Scroll Into View"] = "Scroll Into View";
> localizedStrings["Search"] = "Search";
> localizedStrings["Search Again"] = "Search Again";
> localizedStrings["Search Resource Content"] = "Search Resource Content";
>+localizedStrings["Searching %s"] = "Searching %s";
> localizedStrings["Secure"] = "Secure";
> localizedStrings["Security"] = "Security";
> localizedStrings["Security Issue"] = "Security Issue";
>diff --git a/Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js b/Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js
>index bce2c71a57c57ec40516c9d1a064d6adb099fe73..030975d1efe774e46d86db4058de46c9f5aa9591 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js
>@@ -93,12 +93,33 @@ WI.SearchSidebarPanel = class SearchSidebarPanel extends WI.NavigationSidebarPan
>         if (!searchQuery.length)
>             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"));
>+        };
>+
>         // FIXME: Provide UI to toggle regex and case sensitive searches.
>         var isCaseSensitive = false;
>         var isRegex = false;
> 
>-        var updateEmptyContentPlaceholderTimeout = null;
>-
>         function createTreeElementForMatchObject(matchObject, parentTreeElement)
>         {
>             let matchTreeElement = new WI.SearchResultTreeElement(matchObject);
>@@ -110,23 +131,6 @@ WI.SearchSidebarPanel = class SearchSidebarPanel extends WI.NavigationSidebarPan
>                 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"));
>-        }
>-
>         function forEachMatch(searchQuery, lineContent, callback)
>         {
>             var lineMatch;
>@@ -135,18 +139,9 @@ WI.SearchSidebarPanel = class SearchSidebarPanel extends WI.NavigationSidebarPan
>                 callback(lineMatch, searchRegex.lastIndex);
>         }
> 
>-        function resourcesCallback(error, result)
>-        {
>-            updateEmptyContentPlaceholderSoon.call(this);
>-
>-            if (error)
>-                return;
>-
>-            function resourceCallback(frameId, url, error, resourceMatches)
>-            {
>-                updateEmptyContentPlaceholderSoon.call(this);
>-
>-                if (error || !resourceMatches || !resourceMatches.length)
>+        let resourcesCallback = ({result}) => {
>+            let resourceCallback = (frameId, url, resourceMatches) => {
>+                if (!resourceMatches || !resourceMatches.length)
>                     return;
> 
>                 var frame = WI.networkManager.frameForIdentifier(frameId);
>@@ -166,14 +161,11 @@ WI.SearchSidebarPanel = class SearchSidebarPanel extends WI.NavigationSidebarPan
>                         createTreeElementForMatchObject.call(this, matchObject, resourceTreeElement);
>                     });
>                 }
>-
>-                updateEmptyContentPlaceholder.call(this);
>-            }
>+            };
> 
>             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;
> 
>@@ -186,7 +178,7 @@ WI.SearchSidebarPanel = class SearchSidebarPanel extends WI.NavigationSidebarPan
>                 preventDuplicates.add(key);
> 
>                 // 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));
>             }
> 
>             let promises = [
>@@ -194,63 +186,45 @@ WI.SearchSidebarPanel = class SearchSidebarPanel extends WI.NavigationSidebarPan
>                 WI.Target.awaitEvent(WI.Target.Event.ResourceAdded)
>             ];
>             Promise.race(promises).then(this._contentChanged.bind(this));
>-        }
>-
>-        function searchScripts(scriptsToSearch)
>-        {
>-            updateEmptyContentPlaceholderSoon.call(this);
>+        };
> 
>+        let searchScripts = (scriptsToSearch) => {
>             if (!scriptsToSearch.length)
>                 return;
> 
>-            function scriptCallback(script, error, scriptMatches)
>-            {
>-                updateEmptyContentPlaceholderSoon.call(this);
>-
>-                if (error || !scriptMatches || !scriptMatches.length)
>+            let scriptCallback = (script, {result}) => {
>+                if (!result || !result.length)
>                     return;
> 
>                 var scriptTreeElement = this._searchTreeElementForScript(script);
> 
>-                for (var i = 0; i < scriptMatches.length; ++i) {
>-                    var match = scriptMatches[i];
>+                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);
>                     });
>                 }
>-
>-                updateEmptyContentPlaceholder.call(this);
>-            }
>+            };
> 
>             for (let script of scriptsToSearch)
>-                script.target.DebuggerAgent.searchInContent(script.id, searchQuery, isCaseSensitive, isRegex, scriptCallback.bind(this, script));
>-        }
>+                countPromise(script.target.DebuggerAgent.searchInContent(script.id, searchQuery, isCaseSensitive, isRegex), scriptCallback.bind(this, script));
>+        };
> 
>-        function domCallback(error, searchId, resultsCount)
>-        {
>-            updateEmptyContentPlaceholderSoon.call(this);
>-
>-            if (error || !resultsCount)
>+        let domCallback = ({searchId, resultCount}) => {
>+            if (!resultCount)
>                 return;
> 
>             console.assert(searchId);
> 
>             this._domSearchIdentifier = searchId;
> 
>-            function domSearchResults(error, nodeIds)
>-            {
>-                updateEmptyContentPlaceholderSoon.call(this);
>-
>-                if (error)
>+            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;
> 
>-                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;
> 
>@@ -279,35 +253,31 @@ WI.SearchSidebarPanel = class SearchSidebarPanel extends WI.NavigationSidebarPan
>                         var matchObject = new WI.DOMSearchMatchObject(resource, domNode, domNodeTitle, domNodeTitle, new WI.TextRange(0, 0, 0, domNodeTitle.length));
>                         createTreeElementForMatchObject.call(this, matchObject, resourceTreeElement);
>                     }
>-
>-                    updateEmptyContentPlaceholder.call(this);
>                 }
>-            }
>+            };
> 
>-            DOMAgent.getSearchResults(searchId, 0, resultsCount, domSearchResults.bind(this));
>-        }
>+            countPromise(DOMAgent.getSearchResults(searchId, 0, resultCount), domSearchResults);
>+        };
> 
>         if (window.DOMAgent)
>             WI.domManager.ensureDocument();
> 
>         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);
> 
>         if (window.DOMAgent) {
>             if (this._domSearchIdentifier) {
>-                DOMAgent.discardSearchResults(this._domSearchIdentifier);
>+                countPromise(DOMAgent.discardSearchResults(this._domSearchIdentifier));
>                 this._domSearchIdentifier = undefined;
>             }
> 
>-            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);
>     }
> 
>     // Private

Actions: View | Formatted Diff | Diff

Attachments on bug 170631: 361856 | 362967 | 362968