[Webkit-unassigned] [Bug 188270] Web Inspector: Global search sometimes returns duplicate results for a resource

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188270

Brian Burg <bburg at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bburg at apple.com
 Attachment #346432|review?                     |review+
              Flags|                            |

--- Comment #9 from Brian Burg <bburg at apple.com> ---
Comment on attachment 346432
  --> https://bugs.webkit.org/attachment.cgi?id=346432
[PATCH] Proposed Fix

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

r=me

> Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js:175
> +            for (let i = 0; i < result.length; ++i) {

Alternatively, use reduce()

> Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js:180
> +                // FIXME: Backend sometimes searches files twice.

Nit: add link to backend bug

> Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js:182
> +                let key = searchResult.frameId + ":" + searchResult.url;

Nit: could use destructuring of searchResult above and template literal. Or not, doesn't matter.

> Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js:187
>                  // COMPATIBILITY (iOS 9): Page.searchInResources did not have the optional requestId parameter.

Not a blocker, but... it seems problematic that the View should be doing this sort of backend-specific deduplication logic at all. It may make sense to introduce a SearchManager or some other model/controller object to make requests and keep track of cached search results apart from the sidebar view class. That would also make this testable.

If there is any reason to wait on the searchInResource backend command to finish, we could iterate preventDuplicates and issue all the commands at once and use Promise.all to wait the results.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180803/99bf21be/attachment.html>