Compositor+Others: Fix a flake in HTMLVideoElement-resize-event-during-playback.html

Libraries/LibIPC/AttachmentMachPort.cpp

@@ -8,6 +8,7 @@
 #include <LibCore/MachPort.h>
 #include <LibCore/System.h>
 #include <LibIPC/Attachment.h>
+#include <errno.h>
 
 // fileport_makeport() and fileport_makefd() are private macOS APIs that convert
 // between file descriptors and Mach port rights. Since Mach messages can only
@@ -51,7 +52,10 @@ int Attachment::to_fd()
 {
     VERIFY(MACH_PORT_VALID(m_port.port()));
     int fd = fileport_makefd(m_port.port());
-    VERIFY(fd >= 0);
+    if (fd < 0) {
+        dbgln("IPC::Attachment: Failed to obtain a file descriptor from a file port: {}", Error::from_errno(errno));
+        return -1;
+    }
     mach_port_deallocate(mach_task_self(), m_port.release());
     return fd;
 }

Libraries/LibIPC/Connection.cpp

@@ -109,7 +109,7 @@ ConnectionBase::PeerEOF ConnectionBase::drain_messages_from_peer()
     });
 
     if (parse_error) {
-        dbgln("IPC::ConnectionBase ({:p}): Disconnecting misbehaving peer due to malformed message", this);
+        dbgln("IPC::ConnectionBase ({:p}): Disconnecting peer after failing to parse a message", this);
         schedule_shutdown = Transport::ShouldShutdown::Yes;
     }
 

Libraries/LibMedia/PlaybackManager.cpp

@@ -491,7 +491,7 @@ void PlaybackManager::set_video_sink_ticking(VideoSinkHandle handle, bool tickin
     manager->update_pipeline_state();
 }
 
-void PlaybackManager::detach_lost_video_sink(VideoSinkHandle handle)
+void PlaybackManager::detach_video_sink(VideoSinkHandle handle)
 {
     auto* track_data = find_video_data_for_handle(handle);
     if (!track_data)
@@ -541,12 +541,15 @@ RefPtr<VideoFrame> PlaybackManager::current_presented_frame(VideoSinkHandle hand
     return track_data.video_sink->current_frame();
 }
 
-void PlaybackManager::release_video_edge(VideoSinkHandle handle)
+void PlaybackManager::release_video_edge(VideoSinkHandle handle, VideoSink const& released_sink)
 {
     auto* manager = video_sink_registrations().get(handle).value_or(nullptr);
     if (!manager)
         return;
-    manager->disable_video_sink_by_handle(handle);
+    auto* track_data = manager->find_video_data_for_handle(handle);
+    if (!track_data || track_data->video_sink != &released_sink)
+        return;
+    manager->detach_video_sink(handle);
 }
 
 void PlaybackManager::enable_an_audio_track(Track const& track)

Libraries/LibMedia/PlaybackManager.h

@@ -73,7 +73,7 @@ class MEDIA_API PlaybackManager final {
     VideoSinkHandle reserve_video_sink_handle(Track const&);
     void disable_video_sink_by_handle(VideoSinkHandle);
     static void set_video_sink_ticking(VideoSinkHandle, bool);
-    void detach_lost_video_sink(VideoSinkHandle);
+    void detach_video_sink(VideoSinkHandle);
     void set_video_resize_handler(VideoSinkHandle, Function<void(Gfx::Size<u32>)>);
 
     void enable_an_audio_track(Track const&);
@@ -114,7 +114,7 @@ class MEDIA_API PlaybackManager final {
     static ErrorOr<RemoteVideoEdge> create_video_edge(VideoSinkHandle, RemoteVideoSink::Delegates);
     static void attach_video_edge(VideoSinkHandle, NonnullRefPtr<RemoteVideoSink> const&);
     static RefPtr<VideoFrame> current_presented_frame(VideoSinkHandle);
-    static void release_video_edge(VideoSinkHandle);
+    static void release_video_edge(VideoSinkHandle, VideoSink const& released_sink);
 
 private:
     struct VideoTrackData {

Libraries/LibMedia/VideoPresentation/VideoPresentationServerConnection.cpp

@@ -25,7 +25,7 @@ void VideoPresentationServerConnection::die()
 {
     revoke_weak_refs();
     for (auto& entry : m_edge_states)
-        PlaybackManager::release_video_edge(entry.value.handle);
+        PlaybackManager::release_video_edge(entry.value.handle, *entry.value.pump);
     m_edge_states.clear();
 }
 
@@ -97,8 +97,9 @@ void VideoPresentationServerConnection::release_video_edge(u64 edge_id)
     if (it == m_edge_states.end())
         return;
     auto handle = it->value.handle;
+    auto pump = it->value.pump;
     m_edge_states.remove(it);
-    PlaybackManager::release_video_edge(handle);
+    PlaybackManager::release_video_edge(handle, *pump);
 }
 
 void VideoPresentationServerConnection::request_start(u64 edge_id)

Libraries/LibWeb/HTML/HTMLMediaElement.cpp

@@ -167,6 +167,8 @@ void HTMLMediaElement::initialize_element()
             m_remote_fetch_data->fetch_controller->stop_fetch();
             m_remote_fetch_data->fetch_controller = nullptr;
         }
+
+        detach_video_sink_edge();
     });
 
     m_document_observer->set_document_became_active([this]() {
@@ -176,6 +178,8 @@ void HTMLMediaElement::initialize_element()
             if (m_remote_fetch_data->stream->next_chunk_start() != m_remote_fetch_data->stream->expected_size())
                 load_remote_resource(UntilEnd { m_remote_fetch_data->stream->next_chunk_start() });
         }
+
+        add_current_video_sink();
     });
 
     m_document_observer->set_document_visibility_state_observer([this](VisibilityState) {
@@ -1811,14 +1815,14 @@ void HTMLMediaElement::add_current_video_sink()
         add_current_video_sink(*handle);
 }
 
-void HTMLMediaElement::detach_video_sink_after_compositor_lost()
+void HTMLMediaElement::detach_video_sink_edge()
 {
     auto handle = video_sink_handle();
     if (!m_playback_manager || !handle.has_value())
         return;
     if (m_active_video_sink)
         m_active_video_sink->unregister();
-    m_playback_manager->detach_lost_video_sink(*handle);
+    m_playback_manager->detach_video_sink(*handle);
 }
 
 void HTMLMediaElement::release_active_video_sink()

Libraries/LibWeb/HTML/HTMLMediaElement.h

@@ -167,7 +167,7 @@ class HTMLMediaElement : public HTMLElement {
     void set_selected_video_track(Badge<VideoTrack>, GC::Ptr<HTML::VideoTrack> video_track);
 
     void add_current_video_sink();
-    void detach_video_sink_after_compositor_lost();
+    void detach_video_sink_edge();
 
     GC::Ref<TextTrack> add_text_track(Bindings::TextTrackKind kind, Utf16View label, Utf16View language);
 

Libraries/LibWeb/Page/Page.cpp

@@ -808,7 +808,7 @@ void Page::restore_all_media_element_video_sinks()
 void Page::detach_all_media_element_video_sinks_after_compositor_lost()
 {
     for_each_media_element([&](auto& media_element) {
-        media_element.detach_video_sink_after_compositor_lost();
+        media_element.detach_video_sink_edge();
     });
 }
 

Services/Compositor/CompositorState.cpp

@@ -149,9 +149,19 @@ void CompositorState::update_display_list(Web::Compositor::CompositorContextId c
     context->apply_display_list_resource_transaction(move(resource_transaction));
     context->install_display_list_update(move(display_list), move(visual_context_tree), move(scroll_state_snapshot));
     resolve_video_sinks(*context);
+    note_display_list_installed_for_video_sinks(context->web_content_client());
     update_video_sink_ticking_states();
 }
 
+void CompositorState::note_display_list_installed_for_video_sinks(CompositorStateWebContentClient& client)
+{
+    auto client_sinks = m_video_sink_states.get(&client);
+    if (!client_sinks.has_value())
+        return;
+    for (auto& sink_entry : *client_sinks)
+        sink_entry.value.display_list_installed_since_registration = true;
+}
+
 void CompositorState::update_image_frame_resources(Web::Compositor::CompositorContextId context_id, Vector<Web::Painting::DisplayListImageFrameResource> image_frames)
 {
     auto* context = context_if_present(context_id);
@@ -222,6 +232,15 @@ bool CompositorState::video_sink_updates_are_admitted(VideoSinkState const& sink
     return has_flag(sink_state.update_flags, Web::Compositor::VideoUpdateFlags::Visible) && painted;
 }
 
+bool CompositorState::video_sink_ticking_state_is_determined(VideoSinkState const& sink_state, bool client_can_paint)
+{
+    if (!has_flag(sink_state.update_flags, Web::Compositor::VideoUpdateFlags::Visible))
+        return true;
+    if (!client_can_paint)
+        return true;
+    return sink_state.sink != nullptr && sink_state.display_list_installed_since_registration;
+}
+
 void CompositorState::on_video_sink_ready(CompositorStateWebContentClient& client, Media::VideoSinkHandle handle, NonnullRefPtr<Media::DisplayingVideoSink> const& sink)
 {
     auto* sink_state = video_sink_state(client, handle);
@@ -250,6 +269,10 @@ void CompositorState::update_video_sink_ticking_states()
             auto is_painted = client_painted.has_value() && client_painted->contains(sink_entry.key);
             auto ticking = video_sink_updates_are_admitted(sink_state, is_painted);
             any_unpainted_sink_admits_updates |= ticking && !is_painted;
+
+            if (!video_sink_ticking_state_is_determined(sink_state, client_painted.has_value()))
+                continue;
+
             if (ticking == sink_state.ticking)
                 continue;
             sink_state.ticking = ticking;

Services/Compositor/CompositorState.h

@@ -192,9 +192,12 @@ class CompositorState final : public RefCounted<CompositorState> {
         // notification is only sent once this diverges from it.
         bool ticking { true };
         bool requires_updates { false };
+        bool display_list_installed_since_registration { false };
     };
     VideoSinkState* video_sink_state(CompositorStateWebContentClient&, Media::VideoSinkHandle);
     static bool video_sink_updates_are_admitted(VideoSinkState const&, bool painted);
+    static bool video_sink_ticking_state_is_determined(VideoSinkState const&, bool client_can_paint);
+    void note_display_list_installed_for_video_sinks(CompositorStateWebContentClient&);
     void update_video_sink_ticking_states();
     HashMap<CompositorStateWebContentClient*, HashMap<Media::VideoSinkHandle, VideoSinkState>> m_video_sink_states;
     RefPtr<Core::Timer> m_unpainted_video_update_timer;

Tests/LibWeb/Text/expected/HTML/HTMLVideoElement-sink-reattached-after-inactive-document.txt

@@ -0,0 +1 @@
+presented after reattachment: 640x480

Tests/LibWeb/Text/input/HTML/HTMLVideoElement-sink-reattached-after-inactive-document.html

@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<script src="../include.js"></script>
+<video id="video"></video>
+<script>
+    // Moving a video element into a document that is not fully active releases its video sink, since an inactive
+    // document has no compositor context to present through. Moving it back must rebuild the sink, otherwise the
+    // element would never present another frame.
+    const video = document.getElementById("video");
+
+    asyncTest(async done => {
+        video.src = "../../../Assets/resize-320x240-640x480.webm";
+        await new Promise(resolve => video.addEventListener("loadedmetadata", resolve, { once: true }));
+
+        const inactiveDocument = new DOMParser().parseFromString("<body></body>", "text/html");
+        inactiveDocument.body.appendChild(video);
+        document.body.appendChild(video);
+
+        // The mid-stream resize is only reported once a frame has been presented through the rebuilt sink.
+        const sawResize = new Promise(resolve => video.addEventListener("resize", resolve, { once: true }));
+        await video.play();
+        await new Promise(resolve => video.addEventListener("ended", resolve, { once: true }));
+        await sawResize;
+
+        println(`presented after reattachment: ${video.videoWidth}x${video.videoHeight}`);
+        done();
+    });
+</script>