[webkit-reviews] review denied: [Bug 69025] Web Inspector: Make search-in-resource test simpler. : [Attachment 109069] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 28 22:24:37 PDT 2011


Pavel Feldman <pfeldman at chromium.org> has denied Vsevolod Vlasov
<vsevik at chromium.org>'s request for review:
Bug 69025: Web Inspector: Make search-in-resource test simpler.
https://bugs.webkit.org/show_bug.cgi?id=69025

Attachment 109069: Patch
https://bugs.webkit.org/attachment.cgi?id=109069&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=109069&action=review


> LayoutTests/http/tests/inspector/search/search-test.js:41
> +	  
setTimeout(InspectorTest._runAfterResourcesAreCreated.bind(InspectorTest,
resourceURLs, callback), 0);

setTimeout is a source of flake. You should sniff for calls you expect instead.


> Source/WebCore/inspector/InspectorPageAgent.cpp:489
> +    *object = InspectorArray::create();

It is not an object. Rename this to "array" or "results"?

> Source/WebCore/inspector/front-end/NetworkManager.js:70
> +    searchInResource: function(resource, query, callback)

This change blurs the distinction between Requests vs Resources. We should not
talk to PageAgent from within network manager if at all possible.


More information about the webkit-reviews mailing list