The style actors are used by the inspector actor, so the former will have to be decoupled before the latter.

I ran into some problems because the stylesheet actors are defined in the reverse order as their dependencies. This first patch changes the order in which the stylesheet actors are defined, so I can decouple the leaves of the dependency tree first.

All of this is pretty much the same was what we did for the inspector and highlighter actors.

Comment on attachment 8746500 [details] [diff] [review] Change the order in which the stylesheet actors are defined. Review of attachment 8746500 [details] [diff] [review]: ----------------------------------------------------------------- Why is this order change needed? Is it because of `addActorType` rejecting redeclaration? If so, please see my previous comment in bug 1265722 comment 13. I think we could change `addActorType` to allow this case, making p.js less confusing to work with.

I didn't realise that the style actors and the style editor actors are separate concepts, so I this bug is not actually a dependency for the inspector actors.

Comment on attachment 8746503 [details] [diff] [review] Decouple StyleSheetFront from StyleSheetActor. Review of attachment 8746503 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/server/actors/stylesheets.js @@ -868,5 @@ > - }.bind(this)); > - } > -}); > - > -exports.StyleSheetFront = StyleSheetFront; Is it safe to remove the export already? I would think you need to keep it until users are updated?

Comment on attachment 8746503 [details] [diff] [review] Decouple StyleSheetFront from StyleSheetActor. Review of attachment 8746503 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/server/actors/stylesheets.js @@ -868,5 @@ > - }.bind(this)); > - } > -}); > - > -exports.StyleSheetFront = StyleSheetFront; Ah, I see, it seems it wasn't used externally.

See question in comment 7.

Comment on attachment 8746500 [details] [diff] [review] Change the order in which the stylesheet actors are defined. This bug should be for the style editor actors only. I've opened a separate bug (bug 1268461) for the stylesheet actors, and will move the patches I already attached to this bug there.

I agree that we could probably refactor protocol.js to make addActorType not throw if the actor type is already defined. I am reluctant to do so, however, because sometimes redefining an actor really should be an error (for instance, when someone tries to create two different instances of ActorClass with the same typename). The problem, as I see it, is that protocol.js doesn't distinguish between 'declaring' an actor type, and 'defining' it, so addActorType is used for both. I'd really like us to think about this a bit better. At the same time, I want us to focus on the minimal effort required to finish the front/actor separation, so if the problem can be fixed by reordering a few classes, I prefer that over making architectural changes to protocol.js at the moment. I'd be happy to open a followup bug for this if you'd like.

(In reply to Eddy Bruel [:ejpbruel] from comment #13) > I agree that we could probably refactor protocol.js to make addActorType not > throw if the actor type is already defined. I am reluctant to do so, > however, because sometimes redefining an actor really should be an error > (for instance, when someone tries to create two different instances of > ActorClass with the same typename). The problem, as I see it, is that > protocol.js doesn't distinguish between 'declaring' an actor type, and > 'defining' it, so addActorType is used for both. > > I'd really like us to think about this a bit better. At the same time, I > want us to focus on the minimal effort required to finish the front/actor > separation, so if the problem can be fixed by reordering a few classes, I > prefer that over making architectural changes to protocol.js at the moment. > I'd be happy to open a followup bug for this if you'd like. Yes, please at least file a follow up bug for it. We should have changed this in protocol.js long ago.

I opened bug 1270751 for this.

Comment on attachment 8751765 [details] [diff] [review] Decouple StyleEditorFront from StyleEditorActor. Review of attachment 8751765 [details] [diff] [review]: ----------------------------------------------------------------- Since you have removed the front exports here, should you also update the other usages of the front code? Maybe that's in a separate patch.

Try push for the patch to change the order in which the style editor actors are defined: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fec58e650e87

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

Try push for the patch to decouple StyleEditorFront from StyleEditorActor: https://treeherder.mozilla.org/#/jobs?repo=try&revision=071396f9c94a

https://hg.mozilla.org/mozilla-central/rev/53942df4f384 https://hg.mozilla.org/mozilla-central/rev/02517c2c78bd

Pushed by ejpbruel@mozilla.com: https://hg.mozilla.org/integration/fx-team/rev/b6ba2b9fbe8a Decouple StyleEditorFront from StyleEditorActor;r=jryans

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=9709475&repo=fx-team

Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/fx-team/rev/486cf151cbef Backed out changeset b6ba2b9fbe8a for bc7 failures in browser_parsable_script.js

Pushed by ejpbruel@mozilla.com: https://hg.mozilla.org/integration/fx-team/rev/0979bb255f40 Decouple StyleEditorFront from StyleEditorActor;r=jryans

As it turns out, we *did* land the StyleEditorActor/Front decoupling, but we never removed the old StyleEditorActor/Front definitions. This didn't lead to problems because the new definitions came before the old ones, so they were always overridden. This patch removes the old StyleEditorActor/Front definitions.

Pushed by ejpbruel@mozilla.com: https://hg.mozilla.org/integration/fx-team/rev/7687a980146e Remove old StyleEditorActor/Front definitions;r=fitzgen