From f3d76b42b3553afae0eaa3887f73ffb35e42c245 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Sun, 24 Jul 2022 13:14:51 +0200
Subject: [PATCH] Ensure that `OptionalContentGroup.visible` cannot be modified
 from the "outside"

Given that Optional Content visibility is only intended/supported to be updated via the `OptionalContentConfig.setVisibility`-method, this patch actually enforces that now.
Note that this will be used by the next patch in the series, and will help prevent inconsistent state in the `OptionalContentConfig`-class.

*Please note:* This patch also uncovered a pre-existing bug, related to iterating through the visibility groups in the constructor, for the `baseState === "OFF"` case.
---
 src/display/optional_content_config.js | 34 ++++++++++++++++++++------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/src/display/optional_content_config.js b/src/display/optional_content_config.js
index a759d28af..370ffecf1 100644
--- a/src/display/optional_content_config.js
+++ b/src/display/optional_content_config.js
@@ -13,14 +13,34 @@
  * limitations under the License.
  */
 
-import { objectFromMap, warn } from "../shared/util.js";
+import { objectFromMap, unreachable, warn } from "../shared/util.js";
+
+const INTERNAL = Symbol("INTERNAL");
 
 class OptionalContentGroup {
+  #visible = true;
+
   constructor(name, intent) {
-    this.visible = true;
     this.name = name;
     this.intent = intent;
   }
+
+  /**
+   * @type {boolean}
+   */
+  get visible() {
+    return this.#visible;
+  }
+
+  /**
+   * @ignore
+   */
+  _setVisible(internal, visible) {
+    if (internal !== INTERNAL) {
+      unreachable("Internal method `_setVisible` called.");
+    }
+    this.#visible = visible;
+  }
 }
 
 class OptionalContentConfig {
@@ -46,17 +66,17 @@ class OptionalContentConfig {
     }
 
     if (data.baseState === "OFF") {
-      for (const group of this.#groups) {
-        group.visible = false;
+      for (const group of this.#groups.values()) {
+        group._setVisible(INTERNAL, false);
       }
     }
 
     for (const on of data.on) {
-      this.#groups.get(on).visible = true;
+      this.#groups.get(on)._setVisible(INTERNAL, true);
     }
 
     for (const off of data.off) {
-      this.#groups.get(off).visible = false;
+      this.#groups.get(off)._setVisible(INTERNAL, false);
     }
   }
 
@@ -174,7 +194,7 @@ class OptionalContentConfig {
       warn(`Optional content group not found: ${id}`);
       return;
     }
-    this.#groups.get(id).visible = !!visible;
+    this.#groups.get(id)._setVisible(INTERNAL, !!visible);
   }
 
   getOrder() {