Jordan Santell [:jsantell] [@jsantell] Assignee
	Description • 10 years ago

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

	Jordan Santell [:jsantell] [@jsantell] 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

	Jordan Santell [:jsantell] [@jsantell] 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

	Victor Porof [:vporof][:vp]
	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.

	Jordan Santell [:jsantell] [@jsantell] Assignee
	Comment 4 • 10 years ago

comments all addressed

	Carsten Book [:Tomcat]
	Comment 5 • 10 years ago

https://hg.mozilla.org/integration/fx-team/rev/4d43899972e7

	Carsten Book [:Tomcat]
	Comment 6 • 10 years ago

https://hg.mozilla.org/integration/fx-team/rev/4d43899972e7

	Carsten Book [:Tomcat]
	Comment 7 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/4d43899972e7