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

[Zaggy1024]

The first commit fixes the flake, and the rest are improvements to prevent repeated video tests from crashing the content processes.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds error handling for Mach file-port conversion and updates malformed IPC disconnect logging. It renames video sink detachment APIs and passes the released sink during video edge cleanup. HTMLMediaElement now detaches sinks for inactive documents and reattaches them when documents become active. The compositor waits for display-list installation before updating ticking for visible paintable sinks. A regression test verifies video presentation after sink reattachment.

Sequence Diagram(s)

sequenceDiagram
  participant HTMLMediaElement
  participant PlaybackManager
  participant CompositorState
  HTMLMediaElement->>PlaybackManager: Detach video sink edge
  HTMLMediaElement->>PlaybackManager: Re-add current video sink
  CompositorState->>CompositorState: Record display-list installation
  CompositorState->>PlaybackManager: Update video sink ticking state

Loading

Suggested reviewers: awesomekling, kalenikaliaksandr

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description addresses the video test flake and safeguards that match the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Services/Compositor/CompositorState.cpp`:
- Around line 156-163: Update
CompositorState::note_display_list_installed_for_video_sinks to accept the
relevant ContextState and mark only sink handles returned by
context.video_sink_handles(), rather than every sink owned by the client. Update
its declaration and the call site around the existing display-list installation
flow to pass the ContextState.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e4c220ad-0c37-4ef7-bbf0-937fb8cf41af

📥 Commits

Reviewing files that changed from the base of the PR and between de4a568 and a51b7bc.

📒 Files selected for processing (12)

• Libraries/LibIPC/AttachmentMachPort.cpp
• Libraries/LibIPC/Connection.cpp
• Libraries/LibMedia/PlaybackManager.cpp
• Libraries/LibMedia/PlaybackManager.h
• Libraries/LibMedia/VideoPresentation/VideoPresentationServerConnection.cpp
• Libraries/LibWeb/HTML/HTMLMediaElement.cpp
• Libraries/LibWeb/HTML/HTMLMediaElement.h
• Libraries/LibWeb/Page/Page.cpp
• Services/Compositor/CompositorState.cpp
• Services/Compositor/CompositorState.h
• Tests/LibWeb/Text/expected/HTML/HTMLVideoElement-sink-reattached-after-inactive-document.txt
• Tests/LibWeb/Text/input/HTML/HTMLVideoElement-sink-reattached-after-inactive-document.html

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Track display-list installation per context sink.

This helper marks every sink owned by client after any one context installs a display list. If the client owns multiple contexts, an unrelated display-list update can satisfy readiness for a visible sink that its own context has not displayed yet. update_video_sink_ticking_states() can then untick that sink before its first presentation.

Pass ContextState to this helper. Mark only handles in context.video_sink_handles(). Update the declaration and the call at Line 152.

Proposed fix

-void note_display_list_installed_for_video_sinks(CompositorStateWebContentClient&);
+void note_display_list_installed_for_video_sinks(ContextState const&);

-    note_display_list_installed_for_video_sinks(context->web_content_client());
+    note_display_list_installed_for_video_sinks(*context);

-void CompositorState::note_display_list_installed_for_video_sinks(CompositorStateWebContentClient& client)
+void CompositorState::note_display_list_installed_for_video_sinks(ContextState const& context)
 {
-    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;
+    auto& client = context.web_content_client();
+    for (auto const& resource_entry : context.video_sink_handles()) {
+        if (auto* sink_state = video_sink_state(client, resource_entry.value))
+            sink_state->display_list_installed_since_registration = true;
+    }
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	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::note_display_list_installed_for_video_sinks(ContextState const& context)
	{
	auto& client = context.web_content_client();
	for (auto const& resource_entry : context.video_sink_handles()) {
	if (auto* sink_state = video_sink_state(client, resource_entry.value))
	sink_state->display_list_installed_since_registration = true;
	}
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Services/Compositor/CompositorState.cpp` around lines 156 - 163, Update
CompositorState::note_display_list_installed_for_video_sinks to accept the
relevant ContextState and mark only sink handles returned by
context.video_sink_handles(), rather than every sink owned by the client. Update
its declaration and the call site around the existing display-list installation
flow to pass the ContextState.

[tcl3]

This rename doesn't seem to match the commit title