[webkit-reviews] review denied: [Bug 64601] Web Inspector: implement import/export for timeline data. : [Attachment 100992] [patch] initial version

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 64601: Web Inspector: implement import/export for timeline data.
https://bugs.webkit.org/show_bug.cgi?id=64601

Attachment 100992: [patch] initial version
https://bugs.webkit.org/attachment.cgi?id=100992&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=100992&action=review


> Source/WebCore/inspector/front-end/TimelinePanel.js:211
> +	   this.saveAsButton = new
WebInspector.StatusBarButton(WebInspector.UIString("Save as..."),
"save-as-status-bar-item");

saveAsButton should be private

> Source/WebCore/inspector/front-end/TimelinePanel.js:214
> +	   this.loadButton = new
WebInspector.StatusBarButton(WebInspector.UIString("Load from..."),
"load-from-status-bar-item");

Ditto.

> Source/WebCore/inspector/front-end/TimelinePanel.js:247
> +		   nextOffset = data.indexOf("\n", offset);

This code assumes that input data is not a simple JSON array but an array with
additional line breaks which looks odd.

> Source/WebCore/inspector/front-end/TimelinePanel.js:289
> +	   InspectorFrontendHost.saveAs("TimelineRawData-" + date + ".json",
records.join("\n"));

Output format should be covered with a test, r- for this.