1265727 - Decouple fronts from actors in performance tool.

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), enhancement, P1)

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

I am getting lots of errors from `styles` and `domnode` actor types (this patch is on top of their decoupling) that are causing failures in the performance tool's mochitests. This is generally preventing me from understanding if I've done everything needed for this bug and there are bugs in the code this depends on, or if my changes still have bugs in them.

`stylesheet` errors:

>     console.error:
>       Failed to load module devtools/shared/gcli/commands/highlight: Error: Unknown type: stylesheet
>     console.error:
>       types.getType@resource://devtools/server/protocol.js:101:9
>     Arg<.initialize@resource://devtools/server/protocol.js:446:17
>     constructor@resource://gre/modules/commonjs/sdk/core/heritage.js:146:23
>     @resource://devtools/shared/specs/styles.js:83:19
>     @resource://devtools/client/fronts/styles.js:12:27
>     @resource://devtools/server/actors/styles.js:13:26
>     @resource://devtools/server/actors/inspector.js:64:46
>     @resource://devtools/shared/gcli/commands/highlight.js:9:1
>     exports.createSystem/system.addItemsByModule/</options.loader@resource://devtools/shared/gcli/source/lib/gcli/system.js:165:20
>     exports.createSystem/loadModule@resource://devtools/shared/gcli/source/lib/gcli/system.js:112:30
>     exports.createSystem/system.load/promises<@resource://devtools/shared/gcli/source/lib/gcli/system.js:210:16
>     exports.createSystem/system.load@resource://devtools/shared/gcli/source/lib/gcli/system.js:208:22
>     exports.getSystem@resource://devtools/shared/gcli/commands/index.js:151:14
>     CommandUtils.createRequisition@resource://devtools/client/shared/developer-toolbar.js:45:12
>     Toolbox.prototype._buildButtons@resource://devtools/client/framework/toolbox.js:1005:12
>     Toolbox.prototype.open/<@resource://devtools/client/framework/toolbox.js:429:28
>     TaskImpl_run@resource://gre/modules/Task.jsm:319:40
>     Handler.prototype.process@resource://gre/modules/Promise-backend.js:937:23
>     this.PromiseWalker.walkerLoop@resource://gre/modules/Promise-backend.js:816:7
>     Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise-backend.js:747:11
>     this.PromiseWalker.schedulePromise@resource://gre/modules/Promise-backend.js:779:7
>     this.PromiseWalker.completePromise@resource://gre/modules/Promise-backend.js:714:7
>     Toolbox.prototype.open/</<@resource://devtools/client/framework/toolbox.js:378:36

`domnode` errors, I think related to the pre-declaration in specs/styles.js:

>     console.error:
>       Failed to load module devtools/shared/gcli/commands/measure: Error: Type 'domnode' already exists.
>     console.error:
>       types.addType@resource://devtools/server/protocol.js:145:11
>     types.addActorType@resource://devtools/server/protocol.js:261:14
>     @resource://devtools/shared/specs/styles.js:16:1
>     @resource://devtools/client/fronts/styles.js:12:27
>     @resource://devtools/server/actors/styles.js:13:26
>     @resource://devtools/server/actors/inspector.js:64:46
>     @resource://devtools/shared/gcli/commands/measure.js:16:1
>     exports.createSystem/system.addItemsByModule/</options.loader@resource://devtools/shared/gcli/source/lib/gcli/system.js:165:20
>     exports.createSystem/loadModule@resource://devtools/shared/gcli/source/lib/gcli/system.js:112:30
>     exports.createSystem/system.load/promises<@resource://devtools/shared/gcli/source/lib/gcli/system.js:210:16
>     exports.createSystem/system.load@resource://devtools/shared/gcli/source/lib/gcli/system.js:208:22
>     exports.getSystem@resource://devtools/shared/gcli/commands/index.js:151:14
>     CommandUtils.createRequisition@resource://devtools/client/shared/developer-toolbar.js:45:12
>     Toolbox.prototype._buildButtons@resource://devtools/client/framework/toolbox.js:1005:12
>     Toolbox.prototype.open/<@resource://devtools/client/framework/toolbox.js:429:28
>     TaskImpl_run@resource://gre/modules/Task.jsm:319:40
>     Handler.prototype.process@resource://gre/modules/Promise-backend.js:937:23
>     this.PromiseWalker.walkerLoop@resource://gre/modules/Promise-backend.js:816:7
>     Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise-backend.js:747:11
>     this.PromiseWalker.schedulePromise@resource://gre/modules/Promise-backend.js:779:7
>     this.PromiseWalker.completePromise@resource://gre/modules/Promise-backend.js:714:7
>     Toolbox.prototype.open/</<@resource://devtools/client/framework/toolbox.js:378:36

More `domnode` errors, also related to the pre-declaration in
specs/styles.js. This one is coming up a lot, and I'm also
getting protocol error packets containing this one. AFAICT, this
is the one preventing the performance tool's mochitests from
continuing/passing.

>     Console message: [JavaScript Error: "error occurred while processing 'protocolDescription: TypeError: actorSpec is undefined
>     Stack: exports.dumpActorSpec@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1507:12
>     exports.dumpProtocolSpec@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1545:25
>     RootActor.prototype.onProtocolDescription@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/root.js:486:12
>     DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1643:15
>     LocalDebuggerTransport.prototype.send/<@chrome://marionette/content/server.js -> resource://devtools/shared/transport/transport.js:569:11
>     exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
>     DevToolsUtils.executeSoon*executeSoon@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/DevToolsUtils.js:33:11
>     LocalDebuggerTransport.prototype.send@chrome://marionette/content/server.js -> resource://devtools/shared/transport/transport.js:562:7
>     _sendRequest@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:875:7
>     _sendOrQueueRequest@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:859:7
>     DebuggerClient.prototype.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:729:5
>     DebuggerClient.requester/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:284:12
>     exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
>     TabTarget.prototype.getActorDescription@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:198:7
>     TabTarget.prototype.actorHasMethod@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:241:12
>     PerformanceController.canCurrentlyRecord<@chrome://devtools/content/performance/performance-controller.js:226:31
>     TaskImpl_run@resource://gre/modules/Task.jsm:319:40
>     TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:400:7
>     TaskImpl_run@resource://gre/modules/Task.jsm:327:13
>     TaskImpl@resource://gre/modules/Task.jsm:280:3
>     createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:254:14
>     PerformanceView.initialize<@chrome://devtools/content/performance/performance-view.js:80:15
>     TaskImpl_run@resource://gre/modules/Task.jsm:319:40
>     TaskImpl@resource://gre/modules/Task.jsm:280:3
>     createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:254:14
>     startupPerformance<@chrome://devtools/content/performance/performance-controller.js:72:9
>     TaskImpl_run@resource://gre/modules/Task.jsm:319:40
>     Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
>     this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
>     Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
>     this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
>     this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
>     Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1284:9
>     DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1226:7
>     generateRequestMethods/</frontProto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1390:14
>     exports.PerformanceFront<.connect<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/fronts/performance.js:50:28
>     TaskImpl_run@resource://gre/modules/Task.jsm:319:40
>     Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
>     this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
>     Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
>     this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
>     Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:454:5
>     TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:395:7
>     TaskImpl_run@resource://gre/modules/Task.jsm:327:13
>     TaskImpl@resource://gre/modules/Task.jsm:280:3
>     createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:254:14
>     Toolbox.prototype.initPerformance<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:2203:11
>     TaskImpl_run@resource://gre/modules/Task.jsm:319:40
>     TaskImpl@resource://gre/modules/Task.jsm:280:3
>     createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:254:14
>     PerformancePanel.prototype.open<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/performance/panel.js:47:23
>     TaskImpl_run@resource://gre/modules/Task.jsm:319:40
>     TaskImpl@resource://gre/modules/Task.jsm:280:3
>     createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:254:14
>     Toolbox.prototype.loadTool/onLoad@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:1316:19

Comment 2

[Marco Mucci [:MarcoM]]

Removed from release - blocked on dependency.

Comment 3

[Eddy Bruel [:ejpbruel]]

Attachment: Decouple PerformanceFront from PerformanceActor.

Comment 4

[Eddy Bruel [:ejpbruel]]

Attachment: Decouple PerformanceRecordingFront from PerformanceRecordingActor.

Comment 5

[Eddy Bruel [:ejpbruel]]

Try push for the patch to decouple PerformanceFront from PerformanceActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a78c37bdeeb7

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/0bc46a85df8c
Decouple PerformanceFront from PerformanceActor;r=fitzgen

Comment 7

[Eddy Bruel [:ejpbruel]]

Try push for the patch to decouple PerformanceRecordingFront from PerformanceRecordingActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e93e44a7167e

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/42bfdef552ff
Decouple PerformanceRecordingFront from PerformanceRecordingActor;r=fitzgen

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0bc46a85df8c
https://hg.mozilla.org/mozilla-central/rev/42bfdef552ff

Comment 10

[Eddy Bruel [:ejpbruel]]

Attachment: Decouple ProfilerFront from ProfilerActor.

Comment 11

[Eddy Bruel [:ejpbruel]]

Attachment: Decouple ProfilerFront from ProfilerActor.

Comment 12

[Eddy Bruel [:ejpbruel]]

Attachment: Decouple FramerateFront from FramerateActor.

Comment 13

[Eddy Bruel [:ejpbruel]]

Attachment: Decouple EventLoopLagFront from EventLoopActor.

N.B. This code seems to be only used by b2g. I don't know how to run the b2g tests locally, so this patch may contain stupid mistakes.

Comment 14

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8765487 [details] [diff] [review]
Decouple EventLoopLagFront from EventLoopActor.

Review of attachment 8765487 [details] [diff] [review]:
-----------------------------------------------------------------

If it is only used for b2g you may want to just remove it completely.

Comment 15

[Eddy Bruel [:ejpbruel]]

Try push for the patch to decouple ProfilerFront from ProfilerActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5cb908004172

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/7773873116e9
Decouple ProfilerFront from ProfilerActor;r=fitzgen

Comment 17

[Eddy Bruel [:ejpbruel]]

Try push for the patch to decouple FramerateFront from FramerateActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2f76854d3a8

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This caused xpcshell failures like https://treeherder.mozilla.org/logviewer.html#?job_id=10176888&repo=fx-team

Backed out in https://hg.mozilla.org/integration/fx-team/rev/5ecfdfb4a997

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Additionally, looks like this caused an eslint failure: https://treeherder.mozilla.org/logviewer.html#?job_id=10170684&repo=fx-team

Comment 20

[Eddy Bruel [:ejpbruel]]

Will try to reland tomorrow.

Comment 21

[Eddy Bruel [:ejpbruel]]

New try run for the patch to decouple ProfilerFront from ProfilerActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8b721cc0d4c

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/649dfb23aea4
Decouple ProfilerFront from ProfilerActor;r=fitzgen

Comment 23

[Eddy Bruel [:ejpbruel]]

Try push for the patch to decouple FramerateFront from FramerateActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f005baba9ec9

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/649dfb23aea4

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/7eef107d8d31
Decouple FramerateFront from FramerateActor;r=fitzgen

Comment 26

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7eef107d8d31

Comment 27

[Eddy Bruel [:ejpbruel]]

Try push for the patch to decouple EventLoopLagFront from EventLoopLagActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=78bc585bb516

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/cab3629ad5fd
Decouple EventLoopLagFront from EventLoopActor. r=fitzgen

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cab3629ad5fd

Comment 30

[Greg Tatum [:gregtatum]]

This work appears to have been completed.

Comment 31

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.