Bug 1593944 - Emit event with StyleRuleActor as argument when its underlying CSS rule is updated. r=pbro

The fix for [Bug 1557689](https://bugzilla.mozilla.org/show_bug.cgi?id=1557689) created a situation in the [`Rule.onDeclarationsChanged()`](https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/devtools/client/inspector/rules/models/rule.js#869-887) whereby the `isUsed` state of  client-side declarations was made to point to a fixed `isUsed` state received from the server, thus losing sync with the latest state of the `StyleRuleActor`. Until another "declarations-update" event was fired from the `StyleRuleActor`, the rule's declarations' `isUsed` flag would point to the state with which they were overwritten on the last event handler call.

As a reminder, the root cause of [Bug 1557689](https://bugzilla.mozilla.org/show_bug.cgi?id=1557689) was the inability force a "refresh" of the `StyleRuleFront` so it picked-up the latest `isUsed` state for its declarations when they depend on other declarations from other rules (ex: `justify-content: center` depends on `display:flex`). Therefore, the "declarations-updated" event was introduced with a payload of changed declarations to overwrite the client-side ones. It was convoluted, but it worked.

However, while investigating the cause of this newer bug [Bug 1593944](https://bugzilla.mozilla.org/show_bug.cgi?id=1593944), I discovered an unusual but perhaps expected side-effect: when dispatching an event over the protocol with a payload of the `StyleRuleActor`, its corresponding `StyleRuleFront` on the client would "refresh" automatically (meaning that, looking up state on the previous front reference would point to the latest state from the actor) . The client doesn't even need to use this payload to replace its previous front reference. Surprisingly, the client doesn't even need to explicitly listen to the event which carries the `StyleRuleActor`/`StyleRuleFront` reference. So long as a previous reference of the front exists on the client, it will point to the updated state of the actor. I haven't been able to identify whether this is a known and expected behavior, so I'm pinging @jdescottes and @ochameau to weigh in on the validity of these findings.

Relying on this behavior, the fix for both [Bug 1557689](https://bugzilla.mozilla.org/show_bug.cgi?id=1557689) and [Bug 1557689](https://bugzilla.mozilla.org/show_bug.cgi?id=1557689) involves simply emitting an event, "rule-updated", with the `StyleRuleActor` instance as payload whenever its internal state changes meaningfully so the corresponding front updates. The client will pick-up the latest `isUsed` state of declarations from its reference to the `StyleRuleFront`.


---

Another way to check this behavior and hypothesis is to do the following:
- In [`StyleRuleActor.setRuleText()`](https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/devtools/server/actors/styles.js#1704) replace `return this` with `return null`; (this will no longer return the `StyleRuleActor` over the protocol; it's not explicitly used on the client anyway).
- In the spec, replace the [`setRuleText()`](https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/devtools/shared/specs/styles.js#222) return value  with `RetVal("nullable:domstylerule")` so the protocol doesn't throw an error when getting the `null` from the actor.
- Make a build.
- Then, open the Inspector -> Rules View and change the value of a valid declaration, say: `display: block`, to something invalid, like `display: booo`. Notice that the declaration is no longer marked invalid with a warning sign. That's because the declaration's [`isValid`](https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/devtools/server/actors/styles.js#1447) flag is set on the actor but it no longer syncs with the client which uses the corresponding front to render the declaration after the change. Not returning the `StyleRuleActor` over the protocol breaks this sync actor-front sync.

Differential Revision: https://phabricator.services.mozilla.com/D52560

--HG--
extra : moz-landing-system : lando

devtools/client/inspector/rules/models/rule.js

@@ -77,9 +77,16 @@ class Rule {
     this.getUniqueSelector = this.getUniqueSelector.bind(this);
     this.onDeclarationsUpdated = this.onDeclarationsUpdated.bind(this);
     this.onLocationChanged = this.onLocationChanged.bind(this);
+    this.onStyleRuleFrontUpdated = this.onStyleRuleFrontUpdated.bind(this);
     this.updateSourceLocation = this.updateSourceLocation.bind(this);
 
-    this.domRule.on("declarations-updated", this.onDeclarationsUpdated);
+    // Added in Firefox 72 for backwards compatibility of initial fix for Bug 1557689.
+    // See follow-up fix in Bug 1593944.
+    if (this.domRule.traits.emitsRuleUpdatedEvent) {
+      this.domRule.on("rule-updated", this.onStyleRuleFrontUpdated);
+    } else {
+      this.domRule.on("declarations-updated", this.onDeclarationsUpdated);
+    }
   }
 
   destroy() {
@@ -87,7 +94,13 @@ class Rule {
       this.unsubscribeSourceMap();
     }
 
-    this.domRule.off("declarations-updated", this.onDeclarationsUpdated);
+    // Added in Firefox 72
+    if (this.domRule.traits.emitsRuleUpdatedEvent) {
+      this.domRule.off("rule-updated", this.onStyleRuleFrontUpdated);
+    } else {
+      this.domRule.off("declarations-updated", this.onDeclarationsUpdated);
+    }
+
     this.domRule.off("location-changed", this.onLocationChanged);
   }
 
@@ -576,6 +589,22 @@ class Rule {
     });
   }
 
+  /**
+   * Event handler for "rule-updated" event fired by StyleRuleActor.
+   *
+   * @param {StyleRuleFront} front
+   */
+  onStyleRuleFrontUpdated(front) {
+    // Overwritting this reference is not required, but it's here to avoid confusion.
+    // Whenever an actor is passed over the protocol, either as a return value or as
+    // payload on an event, the `form` of its corresponding front will be automatically
+    // updated. No action required.
+    // Even if this `domRule` reference here is not explicitly updated, lookups of
+    // `this.domRule.declarations` will point to the latest state of declarations set
+    // on the actor. Everything on `StyleRuleForm.form` will point to the latest state.
+    this.domRule = front;
+  }
+
   /**
    * Get the list of TextProperties from the style. Needs
    * to parse the style's authoredText.
@@ -856,6 +885,8 @@ class Rule {
   }
 
   /**
+   * TODO: Remove after Firefox 75. Keep until then for backwards-compatibility for Bug
+   * 1557689 which has an updated fix from Bug 1593944.
    * Handler for "declarations-updated" events fired from the StyleRuleActor for a
    * CSS rule when the status of any of its CSS declarations change.
    *

devtools/server/actors/styles.js

@@ -1120,7 +1120,7 @@ var PageStyleActor = protocol.ActorClassWithSpec(pageStyleSpec, {
    *
    * This is necessary because changes in one rule can cause the declarations in another
    * to not be applicable (inactive CSS). The observers of those rules should be notified.
-   * Rules will fire a "declarations-updated" event if their declarations changed states.
+   * Rules will fire a "rule-updated" event if any of their declarations changed state.
    *
    * Call this method whenever a CSS rule is mutated:
    * - a CSS declaration is added/changed/disabled/removed
@@ -1356,9 +1356,12 @@ var StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, {
       line: this.line || undefined,
       column: this.column,
       traits: {
-        // Whether the style rule actor implements the setRuleText
-        // method.
+        // Indicates whether StyleRuleActor implements and can use the setRuleText method.
+        // It cannot use it if the stylesheet was programmatically mutated via the CSSOM.
         canSetRuleText: this.canSetRuleText,
+        // Indicates that StyleRuleActor emits the "rule-updated" event.
+        // Added in Firefox 72.
+        emitsRuleUpdatedEvent: true,
       },
     };
 
@@ -1701,6 +1704,8 @@ var StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, {
     this.authoredText = newText;
     this.pageStyle.refreshObservedRules();
 
+    // Returning this updated actor over the protocol will update its corresponding front
+    // and any references to it.
     return this;
   },
 
@@ -2006,8 +2011,7 @@ var StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, {
    * check the states of declarations in this CSS rule.
    *
    * If any have changed their used/unused state, potentially as a result of changes in
-   * another rule, fire a "declarations-updated" event with all declarations and their
-   * updated states.
+   * another rule, fire a "rule-updated" event with this rule actor in its latest state.
    */
   refresh() {
     let hasChanged = false;
@@ -2030,7 +2034,16 @@ var StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, {
     }
 
     if (hasChanged) {
-      this.emit("declarations-updated", this._declarations);
+      // ⚠️ IMPORTANT ⚠️
+      // When an event is emitted via the protocol with the StyleRuleActor as payload, the
+      // corresponding StyleRuleFront will be automatically updated under the hood.
+      // Therefore, when the client looks up properties on the front reference it already
+      // has, it will get the latest values set on the actor, not the ones it originally
+      // had when the front was created. The client is not required to explicitly replace
+      // its previous front reference to the one it receives as this event's payload.
+      // The client doesn't even need to explicitly listen for this event.
+      // The update of the front happens automatically.
+      this.emit("rule-updated", this);
     }
   },
 });

devtools/shared/fronts/styles.js

@@ -94,6 +94,7 @@ class StyleRuleFront extends FrontClassWithSpec(styleRuleSpec) {
   form(form) {
     this.actorID = form.actor;
     this._form = form;
+    this.traits = form.traits || {};
     if (this._mediaText) {
       this._mediaText = null;
     }

devtools/shared/specs/styles.js

@@ -202,9 +202,9 @@ const styleRuleSpec = generateActorSpec({
       line: Arg(0, "number"),
       column: Arg(1, "number"),
     },
-    "declarations-updated": {
-      type: "declarationsUpdated",
-      declarations: Arg(0, "array:json"),
+    "rule-updated": {
+      type: "ruleUpdated",
+      rule: Arg(0, "domstylerule"),
     },
   },