[webkit-reviews] review requested: [Bug 64601] Web Inspector: implement import/export for timeline data. : [Attachment 101306] [patch] next version

[bugzilla-daemon at webkit.org]

Ilya Tikhonovsky <loislo at chromium.org> has asked  for review:
Bug 64601: Web Inspector: implement import/export for timeline data.
https://bugs.webkit.org/show_bug.cgi?id=64601

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

------- Additional Comments from Ilya Tikhonovsky <loislo at chromium.org>
(In reply to comment #16)
> (From update of attachment 101294 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=101294&action=review
> 
> > LayoutTests/inspector/timeline/timeline-load.html:9
> > +	 if (window.layoutTestController)
> 
> remove?

done

> 
> > LayoutTests/inspector/timeline/timeline-load.html:40
> > +	     if (JSON.stringify(data) == JSON.stringify(saveData))
> 
> InspectorTest.addResult(JSON.stringify(saveData));

The test doesn't verify the data but tests that import/export things work.

> 
> > Source/WebCore/inspector/front-end/TimelinePanel.js:224
> > +	 _createFileSelector: function()
> 
> Can this be done lazily?

Nope. It doesn't work if I create input element just on the fly.
I can't use setTimeout because timer call has no UserGesture flag enabled.

> 
> > Source/WebCore/inspector/front-end/TimelinePanel.js:240
> > +	     contextMenu.appendItem(WebInspector.UIString("&Export Timeline
data..."), this._exportToFile.bind(this));
> 
> Use ellipsis character

done.

> > Source/WebCore/inspector/front-end/TimelinePanel.js:1253
> > +		    
WebInspector.error(WebInspector.UIString('Timeline.importFromFile: File "%s"
not found.', file.name));
> 
> There is no WebInspector.error.

done

> 
> > Source/WebCore/inspector/front-end/TimelinePanel.js:1274
> > +	     var date = String.sprintf("%4d%2d%2dT%2d%2d", now.getFullYear(),
now.getMonth() + 1, now.getDate(), now.getHours(), now.getMinutes());
> 
> You can move this to utilities.js (Date.prototype.toRFC2445)

done. toRFC3339