constellation: Broadcast preference changes to all content processes (#38716)

Building on the preference observer work from #38649, we now
automatically install an observer when multiprocess mode is enabled.
This observer notifies the constellation of updated preferences, which
in turn notifies each content process so the changes will be reflected
into script/layout as expected. There's a unit test that verifies this
works correctly by checking a preference-gated WebIDL property before
and after the preference is toggled.

Testing: New unit test added.
Fixes: #35966

Depends on #38649.

---------

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

Cargo.lock

@@ -1499,6 +1499,7 @@ dependencies = [
  "pixels",
  "profile_traits",
  "serde",
+ "servo_config",
  "servo_malloc_size_of",
  "servo_url",
  "strum",
@@ -7309,6 +7310,7 @@ dependencies = [
  "pixels",
  "profile_traits",
  "serde",
+ "servo_config",
  "servo_malloc_size_of",
  "servo_url",
  "strum",

components/config/prefs.rs

@@ -12,10 +12,10 @@ pub use crate::pref_util::PrefValue;
 static PREFERENCES: RwLock<Preferences> = RwLock::new(Preferences::const_default());
 
 pub trait Observer: Send + Sync {
-    fn prefs_changed(&self, _changes: Vec<(&'static str, PrefValue)>) {}
+    fn prefs_changed(&self, _changes: &[(&'static str, PrefValue)]) {}
 }
 
-static OBSERVER: RwLock<Option<Box<dyn Observer>>> = RwLock::new(None);
+static OBSERVERS: RwLock<Vec<Box<dyn Observer>>> = RwLock::new(Vec::new());
 
 #[inline]
 /// Get the current set of global preferences for Servo.
@@ -23,8 +23,8 @@ pub fn get() -> RwLockReadGuard<'static, Preferences> {
     PREFERENCES.read().unwrap()
 }
 
-pub fn set_observer(observer: Box<dyn Observer>) {
-    *OBSERVER.write().unwrap() = Some(observer);
+pub fn add_observer(observer: Box<dyn Observer>) {
+    OBSERVERS.write().unwrap().push(observer);
 }
 
 pub fn set(preferences: Preferences) {
@@ -57,8 +57,8 @@ pub fn set(preferences: Preferences) {
 
     *PREFERENCES.write().unwrap() = preferences;
 
-    if let Some(observer) = OBSERVER.read().unwrap().as_deref() {
-        observer.prefs_changed(changed);
+    for observer in &*OBSERVERS.read().unwrap() {
+        observer.prefs_changed(&changed);
     }
 }
 

components/constellation/constellation.rs

@@ -160,6 +160,7 @@ use script_traits::{
     ScriptThreadMessage, UpdatePipelineIdReason,
 };
 use serde::{Deserialize, Serialize};
+use servo_config::prefs::{self, PrefValue};
 use servo_config::{opts, pref};
 use servo_rand::{Rng, ServoRng, SliceRandom, random};
 use servo_url::{Host, ImmutableOrigin, ServoUrl};
@@ -254,6 +255,18 @@ struct BrowsingContextGroup {
     webgpus: HashMap<Host, WebGPU>,
 }
 
+struct PreferenceForwarder(Sender<EmbedderToConstellationMessage>);
+
+impl prefs::Observer for PreferenceForwarder {
+    fn prefs_changed(&self, changes: &[(&'static str, PrefValue)]) {
+        let _ = self
+            .0
+            .send(EmbedderToConstellationMessage::PreferencesUpdated(
+                changes.to_owned(),
+            ));
+    }
+}
+
 /// The `Constellation` itself. In the servo browser, there is one
 /// constellation, which maintains all of the browser global data.
 /// In embedded applications, there may be more than one constellation,
@@ -583,6 +596,7 @@ where
         hard_fail: bool,
     ) -> Sender<EmbedderToConstellationMessage> {
         let (compositor_sender, compositor_receiver) = unbounded();
+        let compositor_sender_self = compositor_sender.clone();
 
         // service worker manager to communicate with constellation
         let (swmanager_ipc_sender, swmanager_ipc_receiver) =
@@ -654,6 +668,10 @@ where
 
                 let rippy_data = resources::read_bytes(Resource::RippyPNG);
 
+                if opts::get().multiprocess {
+                    prefs::add_observer(Box::new(PreferenceForwarder(compositor_sender_self)));
+                }
+
                 let mut constellation: Constellation<STF, SWF> = Constellation {
                     namespace_receiver,
                     namespace_ipc_sender,
@@ -1507,6 +1525,20 @@ where
             EmbedderToConstellationMessage::SetWebDriverResponseSender(sender) => {
                 self.webdriver_input_command_reponse_sender = Some(sender);
             },
+            EmbedderToConstellationMessage::PreferencesUpdated(updates) => {
+                let event_loops = self
+                    .pipelines
+                    .values()
+                    .map(|pipeline| pipeline.event_loop.clone());
+                for event_loop in event_loops {
+                    let _ = event_loop.send(ScriptThreadMessage::PreferencesUpdated(
+                        updates
+                            .iter()
+                            .map(|(name, value)| (String::from(*name), value.clone()))
+                            .collect(),
+                    ));
+                }
+            },
         }
     }
 

components/constellation/tracing.rs

@@ -78,6 +78,7 @@ mod from_compositor {
                 Self::CreateMemoryReport(..) => target!("CreateMemoryReport"),
                 Self::SendImageKeysForPipeline(..) => target!("SendImageKeysForPipeline"),
                 Self::SetWebDriverResponseSender(..) => target!("SetWebDriverResponseSender"),
+                Self::PreferencesUpdated(..) => target!("PreferencesUpdated"),
             }
         }
     }

components/script/messaging.rs

@@ -97,6 +97,7 @@ impl MixedMessage {
                 ScriptThreadMessage::SetScrollStates(id, ..) => Some(*id),
                 ScriptThreadMessage::EvaluateJavaScript(id, _, _) => Some(*id),
                 ScriptThreadMessage::SendImageKeysBatch(..) => None,
+                ScriptThreadMessage::PreferencesUpdated(..) => None,
             },
             MixedMessage::FromScript(inner_msg) => match inner_msg {
                 MainThreadScriptMsg::Common(CommonScriptMsg::Task(_, _, pipeline_id, _)) => {

components/script/script_thread.rs

@@ -88,7 +88,7 @@ use script_traits::{
     ConstellationInputEvent, DiscardBrowsingContext, DocumentActivity, InitialScriptState,
     NewLayoutInfo, Painter, ProgressiveWebMetricType, ScriptThreadMessage, UpdatePipelineIdReason,
 };
-use servo_config::opts;
+use servo_config::{opts, prefs};
 use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl};
 use style::thread_state::{self, ThreadState};
 use style_traits::CSSPixel;
@@ -1887,6 +1887,13 @@ impl ScriptThread {
             ScriptThreadMessage::RefreshCursor(pipeline_id, cursor_position) => {
                 self.handle_refresh_cursor(pipeline_id, cursor_position);
             },
+            ScriptThreadMessage::PreferencesUpdated(updates) => {
+                let mut current_preferences = prefs::get().clone();
+                for (name, value) in updates {
+                    current_preferences.set_value(&name, value);
+                }
+                prefs::set(current_preferences);
+            },
         }
     }
 

components/servo/Cargo.toml

@@ -149,3 +149,7 @@ harness = false
 [[test]]
 name = "servo"
 harness = false
+
+[[test]]
+name = "multiprocess"
+harness = false

components/servo/tests/common/mod.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+use std::cell::{Cell, RefCell};
 use std::rc::Rc;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, Ordering};
@@ -11,15 +12,20 @@ use anyhow::Error;
 use compositing_traits::rendering_context::{RenderingContext, SoftwareRenderingContext};
 use dpi::PhysicalSize;
 use embedder_traits::EventLoopWaker;
-use servo::{Servo, ServoBuilder};
+use servo::{
+    JSValue, JavaScriptEvaluationError, LoadStatus, Servo, ServoBuilder, WebView, WebViewDelegate,
+};
 
 macro_rules! run_api_tests {
     ($($test_function:ident), +) => {
+        run_api_tests!(setup: |builder| builder, $($test_function),+)
+    };
+    (setup: $builder:expr, $($test_function:ident), +) => {
         let mut failed = false;
 
         // Be sure that `servo_test` is dropped before exiting early.
         {
-            let servo_test = ServoTest::new();
+            let servo_test = ServoTest::new($builder);
             $(
                 common::run_test($test_function, stringify!($test_function), &servo_test, &mut failed);
             )+
@@ -28,7 +34,7 @@ macro_rules! run_api_tests {
         if failed {
             std::process::exit(1);
         }
-    }
+    };
 }
 
 pub(crate) use run_api_tests;
@@ -64,7 +70,10 @@ impl Drop for ServoTest {
 }
 
 impl ServoTest {
-    pub(crate) fn new() -> Self {
+    pub(crate) fn new<F>(customize: F) -> Self
+    where
+        F: FnOnce(ServoBuilder) -> ServoBuilder,
+    {
         let rendering_context = Rc::new(
             SoftwareRenderingContext::new(PhysicalSize {
                 width: 500,
@@ -87,9 +96,10 @@ impl ServoTest {
         }
 
         let user_event_triggered = Arc::new(AtomicBool::new(false));
-        let servo = ServoBuilder::new(rendering_context.clone())
-            .event_loop_waker(Box::new(EventLoopWakerImpl(user_event_triggered)))
-            .build();
+        let builder = ServoBuilder::new(rendering_context.clone())
+            .event_loop_waker(Box::new(EventLoopWakerImpl(user_event_triggered)));
+        let builder = customize(builder);
+        let servo = builder.build();
         Self { servo }
     }
 
@@ -122,3 +132,42 @@ impl ServoTest {
         Ok(())
     }
 }
+
+#[derive(Default)]
+pub(crate) struct WebViewDelegateImpl {
+    pub(crate) url_changed: Cell<bool>,
+}
+
+impl WebViewDelegateImpl {
+    pub(crate) fn reset(&self) {
+        self.url_changed.set(false);
+    }
+}
+
+impl WebViewDelegate for WebViewDelegateImpl {
+    fn notify_url_changed(&self, _webview: servo::WebView, _url: url::Url) {
+        self.url_changed.set(true);
+    }
+}
+
+pub(crate) fn evaluate_javascript(
+    servo_test: &ServoTest,
+    webview: WebView,
+    script: impl ToString,
+) -> Result<JSValue, JavaScriptEvaluationError> {
+    let load_webview = webview.clone();
+    let _ = servo_test.spin(move || Ok(load_webview.load_status() != LoadStatus::Complete));
+
+    let saved_result = Rc::new(RefCell::new(None));
+    let callback_result = saved_result.clone();
+    webview.evaluate_javascript(script, move |result| {
+        *callback_result.borrow_mut() = Some(result)
+    });
+
+    let spin_result = saved_result.clone();
+    let _ = servo_test.spin(move || Ok(spin_result.borrow().is_none()));
+
+    (*saved_result.borrow())
+        .clone()
+        .expect("Should have waited until value available")
+}

components/servo/tests/multiprocess.rs

@@ -0,0 +1,68 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+//! Unit tests exercising Servo API functionality with multiprocess mode enabled.
+//!
+//! Since all Servo tests must run serially on the same thread, it is important
+//! that tests never panic. In order to ensure this, use `anyhow::ensure!` instead
+//! of `assert!` for test assertions. `ensure!` will produce a `Result::Err` in
+//! place of panicking.
+
+#[allow(dead_code)]
+mod common;
+
+use std::rc::Rc;
+
+use anyhow::ensure;
+use common::{ServoTest, WebViewDelegateImpl, evaluate_javascript, run_api_tests};
+use servo::{JSValue, WebViewBuilder, run_content_process};
+use servo_config::{opts, prefs};
+
+fn test_multiprocess_preference_observer(servo_test: &ServoTest) -> Result<(), anyhow::Error> {
+    let delegate = Rc::new(WebViewDelegateImpl::default());
+    let webview = WebViewBuilder::new(servo_test.servo())
+        .delegate(delegate.clone())
+        .build();
+
+    let result = evaluate_javascript(servo_test, webview.clone(), "window.gc");
+    ensure!(matches!(result, Ok(JSValue::Undefined)));
+
+    let mut prefs = prefs::get().clone();
+    prefs.dom_servo_helpers_enabled = true;
+    prefs::set(prefs);
+
+    webview.reload();
+
+    let result = evaluate_javascript(servo_test, webview.clone(), "window.gc");
+    ensure!(matches!(result, Ok(JSValue::Object(..))));
+
+    Ok(())
+}
+
+fn main() {
+    let mut token = None;
+    let mut take_next = false;
+    for arg in std::env::args() {
+        if take_next {
+            token = Some(arg);
+            break;
+        }
+        if arg == "--content-process" {
+            take_next = true;
+        }
+    }
+
+    if let Some(token) = token {
+        return run_content_process(token);
+    }
+
+    run_api_tests!(
+        setup: |builder| {
+            let mut opts = opts::Opts::default();
+            opts.multiprocess = true;
+            builder.opts(opts)
+        },
+        test_multiprocess_preference_observer
+    );
+}

components/servo/tests/servo.rs

@@ -9,6 +9,7 @@
 //! of `assert!` for test assertions. `ensure!` will produce a `Result::Err` in
 //! place of panicking.
 
+#[allow(dead_code)]
 mod common;
 
 use anyhow::ensure;

components/servo/tests/webview.rs

@@ -11,33 +11,13 @@
 
 mod common;
 
-use std::cell::{Cell, RefCell};
 use std::rc::Rc;
 
 use anyhow::ensure;
-use common::{ServoTest, run_api_tests};
-use servo::{
-    JSValue, JavaScriptEvaluationError, LoadStatus, Theme, WebView, WebViewBuilder, WebViewDelegate,
-};
+use common::{ServoTest, WebViewDelegateImpl, evaluate_javascript, run_api_tests};
+use servo::{JSValue, JavaScriptEvaluationError, Theme, WebViewBuilder};
 use url::Url;
 
-#[derive(Default)]
-struct WebViewDelegateImpl {
-    url_changed: Cell<bool>,
-}
-
-impl WebViewDelegateImpl {
-    pub(crate) fn reset(&self) {
-        self.url_changed.set(false);
-    }
-}
-
-impl WebViewDelegate for WebViewDelegateImpl {
-    fn notify_url_changed(&self, _webview: servo::WebView, _url: url::Url) {
-        self.url_changed.set(true);
-    }
-}
-
 fn test_create_webview(servo_test: &ServoTest) -> Result<(), anyhow::Error> {
     let delegate = Rc::new(WebViewDelegateImpl::default());
     let webview = WebViewBuilder::new(servo_test.servo())
@@ -53,28 +33,6 @@ fn test_create_webview(servo_test: &ServoTest) -> Result<(), anyhow::Error> {
     Ok(())
 }
 
-fn evaluate_javascript(
-    servo_test: &ServoTest,
-    webview: WebView,
-    script: impl ToString,
-) -> Result<JSValue, JavaScriptEvaluationError> {
-    let load_webview = webview.clone();
-    let _ = servo_test.spin(move || Ok(load_webview.load_status() != LoadStatus::Complete));
-
-    let saved_result = Rc::new(RefCell::new(None));
-    let callback_result = saved_result.clone();
-    webview.evaluate_javascript(script, move |result| {
-        *callback_result.borrow_mut() = Some(result)
-    });
-
-    let spin_result = saved_result.clone();
-    let _ = servo_test.spin(move || Ok(spin_result.borrow().is_none()));
-
-    (*saved_result.borrow())
-        .clone()
-        .expect("Should have waited until value available")
-}
-
 fn test_evaluate_javascript_basic(servo_test: &ServoTest) -> Result<(), anyhow::Error> {
     let delegate = Rc::new(WebViewDelegateImpl::default());
     let webview = WebViewBuilder::new(servo_test.servo())

components/shared/constellation/Cargo.toml

@@ -31,6 +31,7 @@ net_traits = { workspace = true }
 pixels = { path = "../../pixels" }
 profile_traits = { workspace = true }
 serde = { workspace = true }
+servo_config = { path = "../../config" }
 servo_url = { path = "../../url" }
 strum = { workspace = true }
 strum_macros = { workspace = true }

components/shared/constellation/lib.rs

@@ -28,6 +28,7 @@ use ipc_channel::ipc::IpcSender;
 use malloc_size_of_derive::MallocSizeOf;
 use profile_traits::mem::MemoryReportResult;
 use serde::{Deserialize, Serialize};
+use servo_config::prefs::PrefValue;
 use servo_url::{ImmutableOrigin, ServoUrl};
 pub use structured_data::*;
 use strum_macros::IntoStaticStr;
@@ -102,6 +103,8 @@ pub enum EmbedderToConstellationMessage {
     SendImageKeysForPipeline(PipelineId, Vec<ImageKey>),
     /// Set WebDriver input event handled sender.
     SetWebDriverResponseSender(IpcSender<WebDriverCommandResponse>),
+    /// A set of preferences were updated with the given new values.
+    PreferencesUpdated(Vec<(&'static str, PrefValue)>),
 }
 
 /// A description of a paint metric that is sent from the Servo renderer to the

components/shared/script/Cargo.toml

@@ -35,6 +35,7 @@ net_traits = { workspace = true }
 pixels = { path = "../../pixels" }
 profile_traits = { workspace = true }
 serde = { workspace = true }
+servo_config = { path = "../../config" }
 servo_url = { path = "../../url" }
 strum = { workspace = true, features = ["derive"] }
 strum_macros = { workspace = true }

components/shared/script/lib.rs

@@ -43,6 +43,7 @@ use net_traits::storage_thread::StorageType;
 use pixels::PixelFormat;
 use profile_traits::mem;
 use serde::{Deserialize, Serialize};
+use servo_config::prefs::PrefValue;
 use servo_url::{ImmutableOrigin, ServoUrl};
 use strum_macros::IntoStaticStr;
 use style_traits::{CSSPixel, SpeculativePainter};
@@ -257,6 +258,8 @@ pub enum ScriptThreadMessage {
     EvaluateJavaScript(PipelineId, JavaScriptEvaluationId, String),
     /// A new batch of keys for the image cache for the specific pipeline.
     SendImageKeysBatch(PipelineId, Vec<ImageKey>),
+    /// Preferences were updated in the parent process.
+    PreferencesUpdated(Vec<(String, PrefValue)>),
 }
 
 impl fmt::Debug for ScriptThreadMessage {

ports/servoshell/desktop/webxr.rs

@@ -63,7 +63,7 @@ impl XrDiscoveryWebXrRegistry {
 struct XrPrefObserver(Arc<AtomicBool>);
 
 impl prefs::Observer for XrPrefObserver {
-    fn prefs_changed(&self, changes: Vec<(&'static str, prefs::PrefValue)>) {
+    fn prefs_changed(&self, changes: &[(&'static str, prefs::PrefValue)]) {
         if let Some((_, value)) = changes.iter().find(|(name, _)| *name == "dom_webxr_test") {
             let prefs::PrefValue::Bool(value) = value else {
                 return;
@@ -79,7 +79,7 @@ impl WebXrRegistry for XrDiscoveryWebXrRegistry {
 
         let mock_enabled = Arc::new(AtomicBool::new(pref!(dom_webxr_test)));
         xr.register_mock(HeadlessMockDiscovery::new(mock_enabled.clone()));
-        prefs::set_observer(Box::new(XrPrefObserver(mock_enabled)));
+        prefs::add_observer(Box::new(XrPrefObserver(mock_enabled)));
 
         if let Some(xr_discovery) = self.xr_discovery.take() {
             match xr_discovery {