Closed Bug 1265727 Opened 4 years ago Closed 2 years ago

Decouple fronts from actors in performance tool.

Categories

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

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ejpbruel, Assigned: ejpbruel)

References

Details

Attachments

(5 files, 1 obsolete file)

No description provided.
Depends on: 1265429
Blocks: 1263289
Severity: normal → enhancement
Whiteboard: [devtools-html]
Flags: qe-verify-
Priority: -- → P2
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Iteration: --- → 49.1 - May 9
Priority: P2 → P1
Iteration: 49.1 - May 9 → 49.2 - May 23
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
Removed from release - blocked on dependency.
Assignee: nfitzgerald → nobody
Status: ASSIGNED → NEW
Iteration: 49.2 - May 23 → ---
Priority: P1 → P2
Blocks: 1277706
No longer blocks: 1263289
Blocks: 1263289
Assignee: nobody → ejpbruel
Status: NEW → ASSIGNED
Iteration: --- → 50.1
Priority: P2 → P1
Attachment #8761577 - Flags: review?(nfitzgerald)
Attachment #8761577 - Flags: review?(nfitzgerald) → review+
Attachment #8761578 - Flags: review?(nfitzgerald) → review+
Try push for the patch to decouple PerformanceFront from PerformanceActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a78c37bdeeb7
Pushed by ejpbruel@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/0bc46a85df8c
Decouple PerformanceFront from PerformanceActor;r=fitzgen
Try push for the patch to decouple PerformanceRecordingFront from PerformanceRecordingActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e93e44a7167e
Iteration: 50.1 → 50.2
Pushed by ejpbruel@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/42bfdef552ff
Decouple PerformanceRecordingFront from PerformanceRecordingActor;r=fitzgen
Keywords: leave-open
Attachment #8764601 - Flags: review?(nfitzgerald)
Attachment #8764601 - Attachment is obsolete: true
Attachment #8764601 - Flags: review?(nfitzgerald)
Attachment #8765484 - Flags: review?(nfitzgerald)
Attachment #8765485 - Flags: review?(nfitzgerald)
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.
Attachment #8765487 - Flags: review?(nfitzgerald)
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.
Attachment #8765487 - Flags: review?(nfitzgerald) → review+
Attachment #8765485 - Flags: review?(nfitzgerald) → review+
Attachment #8765484 - Flags: review?(nfitzgerald) → review+
Try push for the patch to decouple ProfilerFront from ProfilerActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5cb908004172
Pushed by ejpbruel@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/7773873116e9
Decouple ProfilerFront from ProfilerActor;r=fitzgen
Try push for the patch to decouple FramerateFront from FramerateActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2f76854d3a8
Additionally, looks like this caused an eslint failure: https://treeherder.mozilla.org/logviewer.html#?job_id=10170684&repo=fx-team
Will try to reland tomorrow.
Flags: needinfo?(ejpbruel)
New try run for the patch to decouple ProfilerFront from ProfilerActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8b721cc0d4c
Pushed by ejpbruel@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/649dfb23aea4
Decouple ProfilerFront from ProfilerActor;r=fitzgen
Try push for the patch to decouple FramerateFront from FramerateActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f005baba9ec9
Pushed by ejpbruel@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/7eef107d8d31
Decouple FramerateFront from FramerateActor;r=fitzgen
Iteration: 50.2 - Jul 4 → 50.3 - Jul 18
Try push for the patch to decouple EventLoopLagFront from EventLoopLagActor:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=78bc585bb516
Iteration: 50.3 - Jul 18 → 50.4 - Aug 1
Pushed by ejpbruel@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/cab3629ad5fd
Decouple EventLoopLagFront from EventLoopActor. r=fitzgen
Iteration: 50.4 - Aug 1 → ---
Whiteboard: [devtools-html]
Assignee: ejpbruel → nobody
Status: ASSIGNED → NEW
This work appears to have been completed.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Product: Firefox → DevTools
Assignee: nobody → ejpbruel
You need to log in before you can comment on or make changes to this bug.