Fix media session action handling

components/compositing/windowing.rs

@@ -104,7 +104,7 @@ pub enum WindowEvent {
     ToggleSamplingProfiler(Duration, Duration),
     /// Sent when the user triggers a media action through the UA exposed media UI
     /// (play, pause, seek, etc.).
-    MediaSessionAction(TopLevelBrowsingContextId, MediaSessionActionType),
+    MediaSessionAction(MediaSessionActionType),
 }
 
 impl Debug for WindowEvent {

components/constellation/constellation.rs

@@ -476,8 +476,8 @@ pub struct Constellation<Message, LTF, STF> {
     /// Mechanism to force the compositor to process events.
     event_loop_waker: Option<Box<dyn EventLoopWaker>>,
 
-    /// Browing context ID of the active media session.
-    active_media_session: Option<BrowsingContextId>,
+    /// Pipeline ID of the active media session.
+    active_media_session: Option<PipelineId>,
 }
 
 /// State needed to construct a constellation.
@@ -1546,8 +1546,8 @@ where
             FromCompositorMsg::ExitFullScreen(top_level_browsing_context_id) => {
                 self.handle_exit_fullscreen_msg(top_level_browsing_context_id);
             },
-            FromCompositorMsg::MediaSessionAction(top_level_browsing_context_id, action) => {
-                self.handle_media_session_action_msg(top_level_browsing_context_id, action);
+            FromCompositorMsg::MediaSessionAction(action) => {
+                self.handle_media_session_action_msg(action);
             },
         }
     }
@@ -1779,7 +1779,7 @@ where
                     new_value,
                 );
             },
-            FromScriptMsg::MediaSessionEvent(browsing_context_id, event) => {
+            FromScriptMsg::MediaSessionEvent(pipeline_id, event) => {
                 // Unlikely at this point, but we may receive events coming from
                 // different media sessions, so we set the active media session based
                 // on Playing events.
@@ -1797,7 +1797,7 @@ where
                         _ => (),
                     };
                 }
-                self.active_media_session = Some(browsing_context_id);
+                self.active_media_session = Some(pipeline_id);
                 self.embedder_proxy.send((
                     Some(source_top_ctx_id),
                     EmbedderMsg::MediaSessionEvent(event),
@@ -5052,33 +5052,28 @@ where
         }
     }
 
-    fn handle_media_session_action_msg(
-        &mut self,
-        top_level_browsing_context_id: TopLevelBrowsingContextId,
-        action: MediaSessionActionType,
-    ) {
-        let browsing_context_id = BrowsingContextId::from(top_level_browsing_context_id);
-        let pipeline_id = match self.browsing_contexts.get(&browsing_context_id) {
-            Some(browsing_context) => browsing_context.pipeline_id,
-            None => {
-                return warn!(
-                    "Browsing context {} got media session action request after closure.",
-                    browsing_context_id
-                );
-            },
-        };
-        let msg = ConstellationControlMsg::MediaSessionAction(browsing_context_id, action);
-        let result = match self.pipelines.get(&pipeline_id) {
-            None => {
-                return warn!(
-                    "Pipeline {} got media session action request after closure.",
-                    pipeline_id
-                )
-            },
-            Some(pipeline) => pipeline.event_loop.send(msg),
-        };
-        if let Err(e) = result {
-            self.handle_send_error(pipeline_id, e);
+    fn handle_media_session_action_msg(&mut self, action: MediaSessionActionType) {
+        if let Some(media_session_pipeline_id) = self.active_media_session {
+            let result = match self.pipelines.get(&media_session_pipeline_id) {
+                None => {
+                    return warn!(
+                        "Pipeline {} got media session action request after closure.",
+                        media_session_pipeline_id,
+                    )
+                },
+                Some(pipeline) => {
+                    let msg = ConstellationControlMsg::MediaSessionAction(
+                        media_session_pipeline_id,
+                        action,
+                    );
+                    pipeline.event_loop.send(msg)
+                },
+            };
+            if let Err(e) = result {
+                self.handle_send_error(media_session_pipeline_id, e);
+            }
+        } else {
+            error!("Got a media session action but no active media session is registered");
         }
     }
 }

components/script/dom/mediasession.rs

@@ -16,10 +16,9 @@ use crate::dom::bindings::root::{DomRoot, MutNullableDom};
 use crate::dom::htmlmediaelement::HTMLMediaElement;
 use crate::dom::mediametadata::MediaMetadata;
 use crate::dom::window::Window;
-use crate::script_thread::ScriptThread;
 use dom_struct::dom_struct;
 use embedder_traits::MediaSessionEvent;
-use msg::constellation_msg::BrowsingContextId;
+use msg::constellation_msg::PipelineId;
 use script_traits::MediaSessionActionType;
 use script_traits::ScriptMsg;
 use std::collections::HashMap;
@@ -42,22 +41,20 @@ pub struct MediaSession {
 
 impl MediaSession {
     #[allow(unrooted_must_root)]
-    fn new_inherited(browsing_context_id: BrowsingContextId) -> MediaSession {
+    fn new_inherited() -> MediaSession {
         let media_session = MediaSession {
             reflector_: Reflector::new(),
             metadata: Default::default(),
             playback_state: DomRefCell::new(MediaSessionPlaybackState::None),
             action_handlers: DomRefCell::new(HashMap::new()),
             media_instance: Default::default(),
         };
-        ScriptThread::register_media_session(&media_session, browsing_context_id);
         media_session
     }
 
     pub fn new(window: &Window) -> DomRoot<MediaSession> {
-        let browsing_context_id = window.window_proxy().browsing_context_id();
         reflect_dom_object(
-            Box::new(MediaSession::new_inherited(browsing_context_id)),
+            Box::new(MediaSession::new_inherited()),
             window,
             MediaSessionBinding::Wrap,
         )
@@ -68,7 +65,8 @@ impl MediaSession {
     }
 
     pub fn handle_action(&self, action: MediaSessionActionType) {
-        debug!("Handle media session action {:?} {:?}", action);
+        debug!("Handle media session action {:?}", action);
+
         if let Some(handler) = self.action_handlers.borrow().get(&action) {
             if handler.Call__(ExceptionHandling::Report).is_err() {
                 warn!("Error calling MediaSessionActionHandler callback");
@@ -100,8 +98,10 @@ impl MediaSession {
     pub fn send_event(&self, event: MediaSessionEvent) {
         let global = self.global();
         let window = global.as_window();
-        let browsing_context_id = window.window_proxy().browsing_context_id();
-        window.send_to_constellation(ScriptMsg::MediaSessionEvent(browsing_context_id, event));
+        let pipeline_id = window
+            .pipeline_id()
+            .expect("Cannot send media session event outside of a pipeline");
+        window.send_to_constellation(ScriptMsg::MediaSessionEvent(pipeline_id, event));
     }
 }
 
@@ -145,14 +145,6 @@ impl MediaSessionMethods for MediaSession {
     }
 }
 
-impl Drop for MediaSession {
-    fn drop(&mut self) {
-        let global = self.global();
-        let browsing_context_id = global.as_window().window_proxy().browsing_context_id();
-        ScriptThread::remove_media_session(browsing_context_id);
-    }
-}
-
 impl From<MediaSessionAction> for MediaSessionActionType {
     fn from(action: MediaSessionAction) -> MediaSessionActionType {
         match action {

components/script/dom/navigator.rs

@@ -21,7 +21,6 @@ use crate::dom::promise::Promise;
 use crate::dom::serviceworkercontainer::ServiceWorkerContainer;
 use crate::dom::window::Window;
 use crate::dom::xr::XR;
-use crate::script_thread::ScriptThread;
 use dom_struct::dom_struct;
 use std::rc::Rc;
 
@@ -194,23 +193,16 @@ impl NavigatorMethods for Navigator {
     /// https://w3c.github.io/mediasession/#dom-navigator-mediasession
     fn MediaSession(&self) -> DomRoot<MediaSession> {
         self.mediasession.or_init(|| {
-            // There is a single MediaSession instance per browsing context
+            // There is a single MediaSession instance per Pipeline
             // and only one active MediaSession globally.
             //
             // MediaSession creation can happen in two cases:
             //
             // - If content gets `navigator.mediaSession`
             // - If a media instance (HTMLMediaElement so far) starts playing media.
-            //
-            // The MediaSession constructor is in charge of registering itself with
-            // the script thread.
             let global = self.global();
             let window = global.as_window();
-            let browsing_context_id = window.window_proxy().browsing_context_id();
-            match ScriptThread::get_media_session(browsing_context_id) {
-                Some(session) => session,
-                None => MediaSession::new(window),
-            }
+            MediaSession::new(window)
         })
     }
 }

components/script/script_thread.rs

@@ -51,7 +51,6 @@ use crate::dom::event::{Event, EventBubbles, EventCancelable};
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::htmlanchorelement::HTMLAnchorElement;
 use crate::dom::htmliframeelement::{HTMLIFrameElement, NavigationType};
-use crate::dom::mediasession::MediaSession;
 use crate::dom::mutationobserver::MutationObserver;
 use crate::dom::node::{
     from_untrusted_node_address, window_from_node, Node, NodeDamage, ShadowIncluding,
@@ -697,11 +696,6 @@ pub struct ScriptThread {
 
     /// Code is running as a consequence of a user interaction
     is_user_interacting: Cell<bool>,
-
-    /// The MediaSessions known by this thread, if any.
-    /// There can only be one active MediaSession.
-    /// The constellation has the BrowsingContextId of the active MediaSession, if any.
-    media_sessions: DomRefCell<HashMap<BrowsingContextId, Dom<MediaSession>>>,
 }
 
 /// In the event of thread panic, all data on the stack runs its destructor. However, there
@@ -1371,7 +1365,6 @@ impl ScriptThread {
 
             node_ids: Default::default(),
             is_user_interacting: Cell::new(false),
-            media_sessions: DomRefCell::new(HashMap::new()),
         }
     }
 
@@ -1950,8 +1943,8 @@ impl ScriptThread {
             ConstellationControlMsg::PaintMetric(pipeline_id, metric_type, metric_value) => {
                 self.handle_paint_metric(pipeline_id, metric_type, metric_value)
             },
-            ConstellationControlMsg::MediaSessionAction(browsing_context_id, action) => {
-                self.handle_media_session_action(browsing_context_id, action)
+            ConstellationControlMsg::MediaSessionAction(pipeline_id, action) => {
+                self.handle_media_session_action(pipeline_id, action)
             },
             msg @ ConstellationControlMsg::AttachLayout(..) |
             msg @ ConstellationControlMsg::Viewport(..) |
@@ -3936,14 +3929,12 @@ impl ScriptThread {
         }
     }
 
-    fn handle_media_session_action(
-        &self,
-        browsing_context_id: BrowsingContextId,
-        action: MediaSessionActionType,
-    ) {
-        match self.media_sessions.borrow().get(&browsing_context_id) {
-            Some(session) => session.handle_action(action),
-            None => warn!("No MediaSession for this browsing context"),
+    fn handle_media_session_action(&self, pipeline_id: PipelineId, action: MediaSessionActionType) {
+        if let Some(window) = self.documents.borrow().find_window(pipeline_id) {
+            let media_session = window.Navigator().MediaSession();
+            media_session.handle_action(action);
+        } else {
+            warn!("No MediaSession for this pipeline ID");
         };
     }
 
@@ -3970,42 +3961,6 @@ impl ScriptThread {
             globals,
         )
     }
-
-    pub fn register_media_session(
-        media_session: &MediaSession,
-        browsing_context_id: BrowsingContextId,
-    ) {
-        SCRIPT_THREAD_ROOT.with(|root| {
-            let script_thread = unsafe { &*root.get().unwrap() };
-            script_thread
-                .media_sessions
-                .borrow_mut()
-                .insert(browsing_context_id, Dom::from_ref(media_session));
-        })
-    }
-
-    pub fn remove_media_session(browsing_context_id: BrowsingContextId) {
-        SCRIPT_THREAD_ROOT.with(|root| {
-            let script_thread = unsafe { &*root.get().unwrap() };
-            script_thread
-                .media_sessions
-                .borrow_mut()
-                .remove(&browsing_context_id);
-        })
-    }
-
-    pub fn get_media_session(
-        browsing_context_id: BrowsingContextId,
-    ) -> Option<DomRoot<MediaSession>> {
-        SCRIPT_THREAD_ROOT.with(|root| {
-            let script_thread = unsafe { &*root.get().unwrap() };
-            script_thread
-                .media_sessions
-                .borrow()
-                .get(&browsing_context_id)
-                .map(|s| DomRoot::from_ref(&**s))
-        })
-    }
 }
 
 impl Drop for ScriptThread {

components/script_traits/lib.rs

@@ -389,7 +389,7 @@ pub enum ConstellationControlMsg {
     /// Notifies the script thread about a new recorded paint metric.
     PaintMetric(PipelineId, ProgressiveWebMetricType, u64),
     /// Notifies the media session about a user requested media session action.
-    MediaSessionAction(BrowsingContextId, MediaSessionActionType),
+    MediaSessionAction(PipelineId, MediaSessionActionType),
 }
 
 impl fmt::Debug for ConstellationControlMsg {
@@ -881,7 +881,7 @@ pub enum ConstellationMsg {
     /// Request to exit from fullscreen mode
     ExitFullScreen(TopLevelBrowsingContextId),
     /// Media session action.
-    MediaSessionAction(TopLevelBrowsingContextId, MediaSessionActionType),
+    MediaSessionAction(MediaSessionActionType),
 }
 
 impl fmt::Debug for ConstellationMsg {

components/script_traits/script_msg.rs

@@ -256,7 +256,7 @@ pub enum ScriptMsg {
     GetScreenAvailSize(IpcSender<DeviceIntSize>),
     /// Notifies the constellation about media session events
     /// (i.e. when there is metadata for the active media session, playback state changes...).
-    MediaSessionEvent(BrowsingContextId, MediaSessionEvent),
+    MediaSessionEvent(PipelineId, MediaSessionEvent),
 }
 
 impl fmt::Debug for ScriptMsg {

components/servo/lib.rs

@@ -714,8 +714,8 @@ where
                 }
             },
 
-            WindowEvent::MediaSessionAction(ctx, a) => {
-                let msg = ConstellationMsg::MediaSessionAction(ctx, a);
+            WindowEvent::MediaSessionAction(a) => {
+                let msg = ConstellationMsg::MediaSessionAction(a);
                 if let Err(e) = self.constellation_chan.send(msg) {
                     warn!(
                         "Sending MediaSessionAction message to constellation failed ({:?}).",

ports/libsimpleservo/api/src/lib.rs

@@ -472,8 +472,7 @@ impl ServoGlue {
 
     pub fn media_session_action(&mut self, action: i32) -> Result<(), &'static str> {
         info!("Media session action {:?}", action);
-        let browser_id = self.get_browser_id()?;
-        self.process_event(WindowEvent::MediaSessionAction(browser_id, action.into()))
+        self.process_event(WindowEvent::MediaSessionAction(action.into()))
     }
 
     fn process_event(&mut self, event: WindowEvent) -> Result<(), &'static str> {