[Webkit-unassigned] [Bug 59596] Web Inspector: Move Resources Panel search to backend

[bugzilla-daemon at webkit.org]

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


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #91267|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #2 from Pavel Feldman <pfeldman at chromium.org>  2011-04-27 05:34:39 PST ---
(From update of attachment 91267)
View in context: https://bugs.webkit.org/attachment.cgi?id=91267&action=review

> Source/WebCore/inspector/Inspector.json:101
> +                "id": "ResourceSearchResult",

"SearchResult"

> Source/WebCore/inspector/Inspector.json:108
> +                            "properties": [

I'd suggest making "SearchResult" type for this item and return array of these in the protocol method.

> Source/WebCore/inspector/Inspector.json:183
> +                    { "name": "query", "type": "string", "description": "String to search for."  },

query -> "text" or "regex"

> Source/WebCore/inspector/Inspector.json:184
> +                    { "name": "ignoreCase", "type": "boolean", "description": "Specifies whether case should be ignored or not."  }

"optional": true

> Source/WebCore/inspector/Inspector.json:187
> +                    { "name": "searchResult", "$ref": "ResourceSearchResult", "description": "Resource content search result." }

name: "result", "type":"array", "items": { "$ref":"SearchResult" }

> Source/WebCore/inspector/InspectorPageAgent.cpp:478
> +int InspectorPageAgent::countRegularExpressionMatches(const RegularExpression& regex, const String& content)

You could define static functions in WebCore namespace for helper routines.

> Source/WebCore/inspector/InspectorPageAgent.cpp:483
> +    while ((position = regex.match(content, start)) != -1) {

Sounds like you could provide offsets easily too.

> Source/WebCore/inspector/InspectorPageAgent.cpp:501
> +PassRefPtr<InspectorObject> InspectorPageAgent::buildObjectForSearchMatch(Frame* frame, const String& url, int matches)

ditto

> Source/WebCore/inspector/InspectorPageAgent.cpp:513
> +    const CachedResourceLoader::DocumentResourceMap& allResources = frame->document()->cachedResourceLoader()->allCachedResources();

We already have this logic somewhere. Could you reuse it?

> Source/WebCore/inspector/InspectorPageAgent.cpp:520
> +        if (equalIgnoringFragmentIdentifier(kurl, frame->loader()->iconURL()))

Why this check?

> Source/WebCore/inspector/InspectorPageAgent.cpp:530
> +            matches = countMatchesInResourceContent(frame, kurl, regex);

You could shortcut and use cached resource instance to get its content.

> Source/WebCore/inspector/front-end/ResourcesPanel.js:730
> +                var match = searchResult.matchesList[i];

this code is way too messy. you should refactor it.

> Source/WebCore/inspector/front-end/ResourcesPanel.js:748
> +            PageAgent.searchInResources(regexes[i].source, regexes[i].ignoreCase, callback.bind(this));

can there be multiple search queries?

> Source/WebCore/inspector/front-end/ResourcesPanel.js:826
> +    jumpToNextSearchResult: function()

It looks like too much code. I am sure it can be simpler :)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.