WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
64601
Web Inspector: implement import/export for timeline data.
https://bugs.webkit.org/show_bug.cgi?id=64601
Summary
Web Inspector: implement import/export for timeline data.
Ilya Tikhonovsky
Reported
2011-07-15 09:12:38 PDT
%subj%
Attachments
[patch] initial version
(18.16 KB, patch)
2011-07-15 09:22 PDT
,
Ilya Tikhonovsky
yurys
: review-
Details
Formatted Diff
Diff
[patch] second version
(21.81 KB, patch)
2011-07-15 11:16 PDT
,
Ilya Tikhonovsky
no flags
Details
Formatted Diff
Diff
[patch] third version
(11.33 KB, patch)
2011-07-18 00:38 PDT
,
Ilya Tikhonovsky
no flags
Details
Formatted Diff
Diff
[patch] third version with binary changes.
(21.80 KB, patch)
2011-07-18 01:41 PDT
,
Ilya Tikhonovsky
pfeldman
: review-
Details
Formatted Diff
Diff
screenshot
(4.21 KB, image/png)
2011-07-18 02:18 PDT
,
Ilya Tikhonovsky
no flags
Details
[patch] fourth version with context menu
(22.56 KB, patch)
2011-07-18 10:21 PDT
,
Ilya Tikhonovsky
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
[patch] fourth version with fixed test
(22.78 KB, patch)
2011-07-18 23:46 PDT
,
Ilya Tikhonovsky
pfeldman
: review-
Details
Formatted Diff
Diff
[patch] fifth version
(11.03 KB, patch)
2011-07-19 01:32 PDT
,
Ilya Tikhonovsky
no flags
Details
Formatted Diff
Diff
[patch] fifth version with extracted model code.
(12.47 KB, patch)
2011-07-19 03:01 PDT
,
Ilya Tikhonovsky
pfeldman
: review-
Details
Formatted Diff
Diff
[patch] next version
(13.50 KB, patch)
2011-07-19 05:46 PDT
,
Ilya Tikhonovsky
yurys
: review+
Details
Formatted Diff
Diff
Show Obsolete
(9)
View All
Add attachment
proposed patch, testcase, etc.
Ilya Tikhonovsky
Comment 1
2011-07-15 09:22:21 PDT
Created
attachment 100992
[details]
[patch] initial version
Yury Semikhatsky
Comment 2
2011-07-15 09:36:44 PDT
Comment on
attachment 100992
[details]
[patch] initial version 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.
Ilya Tikhonovsky
Comment 3
2011-07-15 11:16:54 PDT
Created
attachment 101008
[details]
[patch] second version
Yury Semikhatsky
Comment 4
2011-07-15 11:24:16 PDT
Comment on
attachment 101008
[details]
[patch] second version View in context:
https://bugs.webkit.org/attachment.cgi?id=101008&action=review
> LayoutTests/inspector/timeline/timeline-load.html:16 > + InspectorTest.addSniffer(WebInspector.TimelinePanel.prototype, "_createFileSelector", loaded);
_createFileSelector could be renamed to _loadFromFileComplete for better readability.
Ilya Tikhonovsky
Comment 5
2011-07-18 00:38:35 PDT
Created
attachment 101135
[details]
[patch] third version
Ilya Tikhonovsky
Comment 6
2011-07-18 01:41:52 PDT
Created
attachment 101137
[details]
[patch] third version with binary changes.
Pavel Feldman
Comment 7
2011-07-18 01:56:40 PDT
Comment on
attachment 101137
[details]
[patch] third version with binary changes. View in context:
https://bugs.webkit.org/attachment.cgi?id=101137&action=review
> Source/WebCore/inspector/front-end/inspector.css:3876 > + -webkit-mask-position: -64px -48px;
Could you please attach the image?
Ilya Tikhonovsky
Comment 8
2011-07-18 02:18:49 PDT
Created
attachment 101140
[details]
screenshot
Pavel Feldman
Comment 9
2011-07-18 02:19:51 PDT
I'd say we should have "Export" and "Import" in context menu + support drag'n'drop both ways instead.
Ilya Tikhonovsky
Comment 10
2011-07-18 10:21:38 PDT
Created
attachment 101168
[details]
[patch] fourth version with context menu
WebKit Review Bot
Comment 11
2011-07-18 10:44:30 PDT
Comment on
attachment 101168
[details]
[patch] fourth version with context menu
Attachment 101168
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/9110471
New failing tests: inspector/timeline/timeline-load.html
Ilya Tikhonovsky
Comment 12
2011-07-18 23:46:50 PDT
Created
attachment 101276
[details]
[patch] fourth version with fixed test
Pavel Feldman
Comment 13
2011-07-19 00:00:02 PDT
Comment on
attachment 101276
[details]
[patch] fourth version with fixed test View in context:
https://bugs.webkit.org/attachment.cgi?id=101276&action=review
> Source/WebCore/inspector/InspectorFrontendHost.cpp:95 > + UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture);
yay
> Source/WebCore/inspector/front-end/TimelinePanel.js:248 > + for (var i = 0; i < 20 && index < data.length; ++i, ++index)
Why do you schedule continuation?
> Source/WebCore/inspector/front-end/TimelinePanel.js:257 > + _importComplete: function()
Why is this needed? If for tests, please name accordingly and put comment inside.
> Source/WebCore/inspector/front-end/TimelinePanel.js:263 > + function onLoad(e) {
{ on the next line
> Source/WebCore/inspector/front-end/TimelinePanel.js:283 > + var date = String.sprintf("%4d-%2d-%2d-%2d-%2d", now.getFullYear(), now.getMonth() + 1, now.getDate(), now.getHours(), now.getMinutes());
This does not look like any of the standard notations. Why not to use a "short" or tfc2445? (20110719T1245)?
> Source/WebCore/inspector/front-end/TimelinePanel.js:390 > + this._rawRecords.push(record);
Can we store records in the TimelineAgentModel instead?
> Source/WebCore/inspector/front-end/inspector.css:3875 > +.save-as-status-bar-item .glyph {
Should this be rolled back?
Ilya Tikhonovsky
Comment 14
2011-07-19 01:32:25 PDT
Created
attachment 101287
[details]
[patch] fifth version
> > > Source/WebCore/inspector/front-end/TimelinePanel.js:248 > > + for (var i = 0; i < 20 && index < data.length; ++i, ++index) > > Why do you schedule continuation?
otherwise Inspector is unresponsive until insertion is completed. With this solution the user see the progress of insertion.
> > > Source/WebCore/inspector/front-end/TimelinePanel.js:257 > > + _importComplete: function() > > Why is this needed? If for tests, please name accordingly and put comment inside.
removed.
> > > Source/WebCore/inspector/front-end/TimelinePanel.js:263 > > + function onLoad(e) { > > { on the next line
fixed.
> > > Source/WebCore/inspector/front-end/TimelinePanel.js:283 > > + var date = String.sprintf("%4d-%2d-%2d-%2d-%2d", now.getFullYear(), now.getMonth() + 1, now.getDate(), now.getHours(), now.getMinutes()); > > This does not look like any of the standard notations. Why not to use a "short" or tfc2445? (20110719T1245)?
fixed.
> > > Source/WebCore/inspector/front-end/TimelinePanel.js:390 > > + this._rawRecords.push(record); > > Can we store records in the TimelineAgentModel instead?
There is no such thing as TimelineAgentModel and I see no reason to introduce it.
> > > Source/WebCore/inspector/front-end/inspector.css:3875 > > +.save-as-status-bar-item .glyph { > > Should this be rolled back?
reverted.
Ilya Tikhonovsky
Comment 15
2011-07-19 03:01:43 PDT
Created
attachment 101294
[details]
[patch] fifth version with extracted model code.
Pavel Feldman
Comment 16
2011-07-19 04:15:18 PDT
Comment on
attachment 101294
[details]
[patch] fifth version with extracted model code. View in context:
https://bugs.webkit.org/attachment.cgi?id=101294&action=review
> LayoutTests/inspector/timeline/timeline-load.html:9 > + if (window.layoutTestController)
remove?
> LayoutTests/inspector/timeline/timeline-load.html:40 > + if (JSON.stringify(data) == JSON.stringify(saveData))
InspectorTest.addResult(JSON.stringify(saveData));
> Source/WebCore/inspector/front-end/TimelinePanel.js:224 > + _createFileSelector: function()
Can this be done lazily?
> Source/WebCore/inspector/front-end/TimelinePanel.js:240 > + contextMenu.appendItem(WebInspector.UIString("&Export Timeline data..."), this._exportToFile.bind(this));
Use ellipsis character
> 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.
> 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)
Ilya Tikhonovsky
Comment 17
2011-07-19 05:46:11 PDT
Created
attachment 101306
[details]
[patch] next version (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
Yury Semikhatsky
Comment 18
2011-07-19 06:38:14 PDT
Comment on
attachment 101306
[details]
[patch] next version View in context:
https://bugs.webkit.org/attachment.cgi?id=101306&action=review
> Source/WebCore/inspector/front-end/TimelinePanel.js:1261 > + WebInspector.log(WebInspector.UIString('Timeline.importFromFile: An error occurred reading the file "%s"', file.name));
reading -> while reading
Ilya Tikhonovsky
Comment 19
2011-07-19 06:45:21 PDT
Committed
r91252
: <
http://trac.webkit.org/changeset/91252
>
Ilya Tikhonovsky
Comment 20
2011-08-03 08:28:14 PDT
***
Bug 46276
has been marked as a duplicate of this bug. ***
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug