WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
127899
Web Inspector: Add the model objects for the new profile data
https://bugs.webkit.org/show_bug.cgi?id=127899
Summary
Web Inspector: Add the model objects for the new profile data
Timothy Hatcher
Reported
2014-01-30 02:30:50 PST
Add proper model objects for the profile data.
Attachments
Patch
(24.56 KB, patch)
2014-01-30 02:33 PST
,
Timothy Hatcher
joepeck
: review+
timothy
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2014-01-30 02:31:11 PST
<
rdar://problem/15944957
>
Timothy Hatcher
Comment 2
2014-01-30 02:33:48 PST
Created
attachment 222649
[details]
Patch
Joseph Pecoraro
Comment 3
2014-01-30 11:05:50 PST
Comment on
attachment 222649
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=222649&action=review
r=me
> Source/WebInspectorUI/UserInterface/ProfileNode.js:148 > + rangeStartTime = !isNaN(rangeStartTime) ? rangeStartTime : 0; > + rangeEndTime = !isNaN(rangeEndTime) ? rangeEndTime : Infinity;
This reads strange to me. I'd prefer a positive condition "isNaN(foo) ? 0 : foo". Or the longer but more straightforward: if (isNaN(foo)) foo = ..;
> Source/WebInspectorUI/UserInterface/ProfileNode.js:207 > + cookie[WebInspector.ProfileNode.SourceCodeURLCookieKey] = this._sourceCodeLocation ? this._sourceCodeLocation.sourceCode.url ? this._sourceCodeLocation.sourceCode.url.hash : null : null;
Ew! Replace: a ? a.b ? a.b.c : null : null; With: a && a.b ? a.b.c : null
> Source/WebInspectorUI/UserInterface/ProfileNodeCall.js:30 > + console.assert(startTime);
So, startTime cannot be 0? Should this be a type check, or is 0 not allowed? console.assert(typeof startTime === "number");
> Source/WebInspectorUI/UserInterface/TimelineManager.js:375 > + // Iterate over the node tree using a stack. Doing this recursively can easily cause a stack overflow. > + // We traverse the profile in post-order and convert the payloads in place until we get back to the root.
This is so wild. I feel like there must be a better way. But I can't find any logic issues with it.
> Source/WebInspectorUI/UserInterface/TimelineManager.js:396 > + return new WebInspector.Profile(rootNodes, payload.idleTime);
Assert "idleTime" in payload near the function entry?
Timothy Hatcher
Comment 4
2014-01-30 17:31:41 PST
https://trac.webkit.org/r163141
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