Closed
Bug 1130204
Opened 10 years ago
Closed 10 years ago
Should hide features when not supported by the server in perf++
Categories
(DevTools :: Performance Tools (Profiler/Timeline), defect)
Tracking
(firefox38 fixed)
RESOLVED
FIXED
Firefox 38
Tracking | Status | |
---|---|---|
firefox38 | --- | fixed |
People
(Reporter: jsantell, Assigned: jsantell)
References
Details
Attachments
(1 file, 1 obsolete file)
19.46 KB,
patch
|
jsantell
:
review+
|
Details | Diff | Splinter Review |
When connecting to Fx2.1, for example, we don't have a real TimelineActor, so we should hide the overview. Fx 2.0, 2.1: Hide all overview waterfall, JS Flamegraph, Memory Call Tree, Memory Flamegraph, possibly 'enable framerate/memory' options Fx 2.2: Figure this out in bug 1130202
Assignee | ||
Updated•10 years ago
|
Updated•10 years ago
|
Blocks: enable-perf-tool
Updated•10 years ago
|
Blocks: perf-tool-v2
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•10 years ago
|
||
Some bad compatibility bugs (if timeline is disabled, memory will never be enabled, as the actor cannot detect the methods) -- fixes are in bug 1130202
Depends on: 1130202
Assignee | ||
Comment 2•10 years ago
|
||
Should be the last component for supporting different targets. This'll hide everything but js calltree (including overview) on 2.0/2.1 b2g, and on b2g 2.2, disable the memory tree/flame view https://treeherder.mozilla.org/#/jobs?repo=try&revision=7246dab0d7d7
Attachment #8564329 -
Flags: review?(vporof)
Comment 3•10 years ago
|
||
Comment on attachment 8564329 [details] [diff] [review] 1130204-hide-perf-views-support.patch Review of attachment 8564329 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/performance/test/head.js @@ +265,5 @@ > function mousedown (win, button) { > EventUtils.sendMouseEvent({ type: "mousedown" }, button, win); > } > > +function* startRecording(panel, waitForOverview=true) { Maybe have waitForOverview as a property in an option, to make calls to this function more obvious. startRecording(panel, { waitForOverview: true }); @@ +303,5 @@ > ok(!button.hasAttribute("locked"), > "The record button should not be locked."); > } > > +function* stopRecording(panel, waitForOverview=true) { Ditto ::: browser/devtools/performance/views/details.js @@ +11,5 @@ > /** > * Name to node+object mapping of subviews. > */ > components: { > + "waterfall": { id: "waterfall-view", view: WaterfallView, support: "timeline" }, s/support/requires maybe? Also, I think it'd be best to have this an array, instead of a string. Let's not make the assumption that only one actor is needed, or that another actor is available implicitly based on another one. @@ +16,4 @@ > "js-calltree": { id: "js-calltree-view", view: JsCallTreeView }, > + "js-flamegraph": { id: "js-flamegraph-view", view: JsFlameGraphView, support: "timeline" }, > + "memory-calltree": { id: "memory-calltree-view", view: MemoryCallTreeView, support: "memory", pref: "enable-memory" }, > + "memory-flamegraph": { id: "memory-flamegraph-view", view: MemoryFlameGraphView, support: "memory", pref: "enable-memory" } e.g. here you should have [timeline, memory] @@ +70,5 @@ > let recording = PerformanceController.getCurrentRecording(); > > let isRecorded = recording && !recording.isRecording(); > + let isEnabled = (!pref || PerformanceController.getPref(pref)) && > + (!support || !mocks[support]); Would help readability to have this split out as `isSupported` and `isEnabled`, and use both everywhere.
Attachment #8564329 -
Flags: review?(vporof) → review+
Assignee | ||
Comment 4•10 years ago
|
||
comments all addressed
Attachment #8564329 -
Attachment is obsolete: true
Attachment #8564584 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/4d43899972e7
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4d43899972e7
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 38
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•