[webkit-reviews] review denied: [Bug 30567] add support for exporting HTTP Archive format in Resources panel : [Attachment 57538] patch (added HAREntry.js itself)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 1 07:42:35 PDT 2010


Pavel Feldman <pfeldman at chromium.org> has denied Andrey Kosyakov
<caseq at chromium.org>'s request for review:
Bug 30567: add support for exporting HTTP Archive format in Resources panel
https://bugs.webkit.org/show_bug.cgi?id=30567

Attachment 57538: patch (added HAREntry.js itself)
https://bugs.webkit.org/attachment.cgi?id=57538&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
WebCore/ChangeLog:5
 +	    Added conversion of inspector's resource representation into HAR.
Web Inspector: Added ...


LayoutTests/ChangeLog:18
 +  2010-06-01	Andrey Kosyakov  <caseq at chromium.org>
You don't need two log entries.

LayoutTests/ChangeLog:11
 +	    * http/tests/inspector/resource-har-conversion.html: Added.
You should also land expectations.

LayoutTests/http/tests/inspector/inspector-test.js:47
 +	    if (prop in (nondeterministicProps || {}))
if (nondeterministicProps && prop in nondeterministicProps)

LayoutTests/http/tests/inspector/inspector-test.js:54
 +		output(prefixWithName  + propValue);
extra space

WebCore/inspector/front-end/HAREntry.js:40
 +  {
Place { on the same line as =

WebCore/inspector/front-end/Resource.js:452
 +	    return this._parsedQueryParameters =
this._parseParameters(queryString);
Split this into two lines please.

WebCore/inspector/front-end/Resource.js:464
 +	    return this._parsedFormParameters =
this._parseParameters(this.requestFormData);
ditto

WebCore/inspector/front-end/ResourceView.js:210
 +	    this.queryStringTreeElement.hidden = queryParameters == null;
!queryParameters

LayoutTests/http/tests/inspector/resource-har-conversion.html:10
 +	notifyDone();
You should not notifyDone synchronously in case dumpResources is asynchronous.

LayoutTests/http/tests/inspector/resource-tests.js:11
 +  function dumpResources() {
I doubt this one does not deserve to be generic.


More information about the webkit-reviews mailing list