WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
161314
Web Inspector: Add resource timing model with timing information
https://bugs.webkit.org/show_bug.cgi?id=161314
Summary
Web Inspector: Add resource timing model with timing information
Johan K. Jensen
Reported
2016-08-29 09:46:34 PDT
Add a resource timing model to keep track of timing information used by e.g. timing details/waterfall in the network tab.
Attachments
Patch
(21.91 KB, patch)
2016-08-29 10:41 PDT
,
Johan K. Jensen
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews102 for mac-yosemite
(834.28 KB, application/zip)
2016-08-29 11:32 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2
(875.03 KB, application/zip)
2016-08-29 11:35 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews116 for mac-yosemite
(1.46 MB, application/zip)
2016-08-29 11:47 PDT
,
Build Bot
no flags
Details
Patch for bots
(22.04 KB, patch)
2016-08-29 13:11 PDT
,
Johan K. Jensen
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-yosemite
(849.73 KB, application/zip)
2016-08-29 13:42 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2
(835.54 KB, application/zip)
2016-08-29 14:03 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews117 for mac-yosemite
(1.47 MB, application/zip)
2016-08-29 14:08 PDT
,
Build Bot
no flags
Details
Patch
(22.29 KB, patch)
2016-08-29 15:03 PDT
,
Johan K. Jensen
no flags
Details
Formatted Diff
Diff
Patch
(22.26 KB, patch)
2016-08-30 16:39 PDT
,
Johan K. Jensen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(9)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2016-08-29 09:47:09 PDT
<
rdar://problem/28056977
>
Johan K. Jensen
Comment 2
2016-08-29 10:41:46 PDT
Created
attachment 287280
[details]
Patch
Build Bot
Comment 3
2016-08-29 11:32:13 PDT
Comment on
attachment 287280
[details]
Patch
Attachment 287280
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/1966762
New failing tests: http/tests/inspector/network/resource-timing.html
Build Bot
Comment 4
2016-08-29 11:32:22 PDT
Created
attachment 287289
[details]
Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Joseph Pecoraro
Comment 5
2016-08-29 11:33:39 PDT
Comment on
attachment 287280
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=287280&action=review
r=me, though please double check with earlier iOS devices to make sure things still work there.
> Source/WebInspectorUI/ChangeLog:31 > + Update timing object with info on from response.
Typo: "on from response"
> Source/WebInspectorUI/UserInterface/Main.html:355 > + <script src="Models/ResourceTimingData.js"></script>
Style: Alphabetical order. Even though "Resource" uses "ResourceTimingData" the order is fine because neither are actually used until after all scripts have loaded.
> Source/WebInspectorUI/UserInterface/Models/Resource.js:475 > this._responseHeadersSize = String(this._statusCode).length + this._statusText.length + 12; // Extra length is for "HTTP/1.1 ", " ", and "\r\n".
Gosh, just noticed this wouldn't work for HTTP/2. Nothing you need to change.
> Source/WebInspectorUI/UserInterface/Models/Resource.js:579 > + this._timingData._responseEnd = elapsedTime || NaN;
Setting a private member like this is sketchy. Normally we would create some public method on ResourceTimingData and call here it. Something like: markResponseEndTime(responseEnd)
> Source/WebInspectorUI/UserInterface/Models/ResourceTimelineRecord.js:55 > - return this._resource.requestSentTimestamp; > + return this._resource.timingData.startTime;
Same thing here.
> Source/WebInspectorUI/UserInterface/Models/ResourceTimingData.js:68 > + // Static
Style: Static methods, like this, normally go above the Public block.
> LayoutTests/http/tests/inspector/network/resource-timing.html:11 > +function resolve(url) { > + let a = document.createElement('a'); > + a.href = url; > + return a.href; > +}
This function doesn't appear to be used. Drop it?
> LayoutTests/http/tests/inspector/network/resource-timing.html:13 > +function createRequest() > +{
Nit: My style in tests would be to put this brace on the same line as the function name because it is outside of the test() function. But I may be the only one that uses this style.
> LayoutTests/http/tests/inspector/network/resource-timing.html:21 > + let suite = InspectorTest.createAsyncSuite("ResourceTiming");
Lets name this after the model object "ResourceTimingData".
Build Bot
Comment 6
2016-08-29 11:34:50 PDT
Comment on
attachment 287280
[details]
Patch
Attachment 287280
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/1966767
New failing tests: http/tests/inspector/network/resource-timing.html
Build Bot
Comment 7
2016-08-29 11:35:00 PDT
Created
attachment 287290
[details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Joseph Pecoraro
Comment 8
2016-08-29 11:42:26 PDT
Hmm, the test failed on the Mac Yosemite bots: --- /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/http/tests/inspector/network/resource-timing-expected.txt +++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/http/tests/inspector/network/resource-timing-actual.txt @@ -12,7 +12,7 @@ PASS: domainLookupStart and domainLookupEnd should both be NaN or a number. PASS: connectStart and connectEnd should both be NaN or a number. PASS: requestStart should come after startTime. -PASS: A secure connection should be reused or secureConnectionStart should come after connectStart. +FAIL: A secure connection should be reused or secureConnectionStart should come after connectStart. PASS: responseStart should come after requestStart. PASS: responseEnd should not be available yet. PASS: responseEnd should come after responseStart. That is weird.
Build Bot
Comment 9
2016-08-29 11:47:18 PDT
Comment on
attachment 287280
[details]
Patch
Attachment 287280
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/1966764
New failing tests: http/tests/inspector/network/resource-timing.html
Build Bot
Comment 10
2016-08-29 11:47:23 PDT
Created
attachment 287296
[details]
Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Johan K. Jensen
Comment 11
2016-08-29 13:11:16 PDT
Created
attachment 287309
[details]
Patch for bots
Build Bot
Comment 12
2016-08-29 13:41:57 PDT
Comment on
attachment 287309
[details]
Patch for bots
Attachment 287309
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/1967417
New failing tests: http/tests/inspector/network/resource-timing.html
Build Bot
Comment 13
2016-08-29 13:42:03 PDT
Created
attachment 287316
[details]
Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 14
2016-08-29 14:03:45 PDT
Comment on
attachment 287309
[details]
Patch for bots
Attachment 287309
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/1967488
New failing tests: http/tests/inspector/network/resource-timing.html
Build Bot
Comment 15
2016-08-29 14:03:51 PDT
Created
attachment 287318
[details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 16
2016-08-29 14:08:26 PDT
Comment on
attachment 287309
[details]
Patch for bots
Attachment 287309
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/1967487
New failing tests: http/tests/inspector/network/resource-timing.html
Build Bot
Comment 17
2016-08-29 14:08:31 PDT
Created
attachment 287321
[details]
Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Johan K. Jensen
Comment 18
2016-08-29 15:03:26 PDT
Created
attachment 287334
[details]
Patch
Joseph Pecoraro
Comment 19
2016-08-30 12:31:34 PDT
Comment on
attachment 287334
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=287334&action=review
> Source/WebInspectorUI/UserInterface/Models/ResourceTimingData.js:85 > + if (isNaN(data.connectStart) && !isNaN(data.secureConnectionStart)) > + data.connectStart = data.domainLookupEnd ? data.domainLookupEnd : data.startTime;
Maybe this should just be data.connectStart = data.secureConnectionStart? And we don't get to distinguish between connectStart/secureConnectionStart. I don't like the idea of using one of the previous timestamps. Would that cause problems?
Johan K. Jensen
Comment 20
2016-08-30 16:39:31 PDT
Created
attachment 287450
[details]
Patch
Joseph Pecoraro
Comment 21
2016-08-30 17:08:02 PDT
Comment on
attachment 287450
[details]
Patch r=me!
WebKit Commit Bot
Comment 22
2016-08-30 17:29:19 PDT
Comment on
attachment 287450
[details]
Patch Clearing flags on attachment: 287450 Committed
r205211
: <
http://trac.webkit.org/changeset/205211
>
WebKit Commit Bot
Comment 23
2016-08-30 17:29:26 PDT
All reviewed patches have been landed. Closing 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