Closed
Bug 1164131
Opened 9 years ago
Closed 9 years ago
[regression] CallTree does not display files with port names in URL correctly
Categories
(DevTools :: Performance Tools (Profiler/Timeline), defect, P1)
Tracking
(firefox40 verified, firefox41 fixed)
VERIFIED
FIXED
Firefox 41
People
(Reporter: jsantell, Assigned: jsantell)
References
Details
(Whiteboard: [polish-backlog])
Attachments
(2 files)
68.95 KB,
image/png
|
Details | |
11.40 KB,
patch
|
shu
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
via Bug 1154115
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jsantell
Blocks: perf-tool-papercuts
Status: NEW → ASSIGNED
Whiteboard: [devedition-40]
Assignee | ||
Comment 1•9 years ago
|
||
Not just for imports, but for any local file
Summary: [regression] Importing profiler v2 does not display fileName in calltree → [regression] Local files do not have correct fileName in call tree
Comment 2•9 years ago
|
||
AHAHAHhahahhah T_T
Assignee | ||
Comment 3•9 years ago
|
||
Related to port number. Previously worked in most common cases, fixing to work in all port cases. Adding "port" and "host" (combo of hostName:port) to parseLocation return.
Summary: [regression] Local files do not have correct fileName in call tree → [regression] CallTree does not display files with port names in URL correctly
Updated•9 years ago
|
Priority: -- → P1
Assignee | ||
Comment 4•9 years ago
|
||
Fixed the issue and did infact test with inline scripts on root's that contain port numbers, and we do get a `/` after the port. Added tests for all of these cases, and we now use `host` instead of just `hostName`. https://treeherder.mozilla.org/#/jobs?repo=try&revision=80f14fdff166
Attachment #8604923 -
Flags: review?(shu)
Comment 5•9 years ago
|
||
Comment on attachment 8604923 [details] [diff] [review] 1164131-calltreeports.patch Review of attachment 8604923 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/shared/profiler/frame-utils.js @@ +150,5 @@ > + port = uri.port === -1 ? null : uri.port; > + host = port !== null ? `${hostName}:${port}` : hostName; > + } catch (e) { > + host = hostName; > + } I don't like this, but I'll live with it. The answer I got from Jonas was that our default implementation of nsIURI doesn't throw for .port, but addons can do what they want since they can implement nsIURI. As long as we don't throw 99% of the time, this is fine.
Attachment #8604923 -
Flags: review?(shu) → review+
Assignee | ||
Comment 6•9 years ago
|
||
Hoping that 1160581 will render this moot
Assignee | ||
Comment 8•9 years ago
|
||
remote: https://hg.mozilla.org/integration/fx-team/rev/7f901174dd44 remote: https://hg.mozilla.org/integration/fx-team/rev/32ad078a4b5f remote: https://hg.mozilla.org/integration/fx-team/rev/914dda9f4a1a
Whiteboard: [devedition-40] → [devedition-40][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/7f901174dd44
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Whiteboard: [devedition-40][fixed-in-fx-team] → [devedition-40]
Target Milestone: --- → Firefox 41
Updated•9 years ago
|
Blocks: perf-40-uplifts
Updated•9 years ago
|
Flags: qe-verify+
Comment 10•9 years ago
|
||
Comment on attachment 8604923 [details] [diff] [review] 1164131-calltreeports.patch Approval Request Comment [Feature/regressing bug #]: 1167252, the new performance tool [User impact if declined]: Won't ship the performance tool [Describe test coverage new/current, TreeHerder]: There are try pushes in Bug 1167252 with all patches needing uplift [Risks and why]: Requesting uplift for the accumulated changes in the performance tool since the 40 merge date, so these changes haven't had the full 6 weeks to bake. Risks are generally contained within devtools, specifically within the performance panel. [String/UUID change made/needed]: None
Attachment #8604923 -
Flags: approval-mozilla-aurora?
Comment 11•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/fc6d8e85c5c2
status-firefox40:
--- → fixed
Comment 12•9 years ago
|
||
Note: I had verbal confirmation for these uplifts from Sylvestre even before he's flagged them as a+. See https://bugzilla.mozilla.org/show_bug.cgi?id=1167252#c26
Comment 13•9 years ago
|
||
Comment on attachment 8604923 [details] [diff] [review] 1164131-calltreeports.patch Change approved to skip one train as part of the spring campaign.
Attachment #8604923 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 14•9 years ago
|
||
Verified fixed on Aurora 40.0a2 (2015-06-04), using Ubuntu 14.04 (x64), Windows 7 (x64) and Mac OS X 10.9.5. Call Tree now properly displays the files from URL's containing port names, e.g. http://i.imgur.com/8BMUtaL.png.
Updated•9 years ago
|
Whiteboard: [devedition-40] → [polish-backlog]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•