[webkit-reviews] review denied: [Bug 89244] Web Inspector: Add progress events to the timeline to keep track of how much the main frame has loaded : [Attachment 148797] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied hanma at rim.com's request for
review:
Bug 89244: Web Inspector: Add progress events to the timeline to keep track of
how much the main frame has loaded
https://bugs.webkit.org/show_bug.cgi?id=89244

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

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


> Source/WebCore/ChangeLog:3
> +	   Added content length, data length, and percentage loaded to

Please prepent Web Inspector:

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:188
> +	   this._progress = {};

These names should be more specific to the network.

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:396
> +	       presentationModel._progress[record.data["requestId"]] +=
parseInt(record.data["length"]);

I don't think progress makes sense in the default timeline view.

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:523
> +		       contentHelper._appendTextRow(WebInspector.UIString("Data
Length"), WebInspector.UIString("%d Bytes", this.data["length"]));

Please add this line to the English.lproj/localizedStrings.js.

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:524
> +		   if (contentLength) {

Please remove this branch, I don't think percentage loaded makes sense in
timeline. Could you explain your scenario in greater detail? We don't focus on
network requests in Timeline panel. We have a Network panel for that.

> Source/WebCore/loader/ResourceLoader.cpp:443
> +    InspectorInstrumentationCookie cookie =
InspectorInstrumentation::willReceiveResourceData(m_frame.get(), identifier(),
length);

You might want encodedDataLength here (the one that is actually transmitted
over the network, not the decompressed chunk).