From 1c61f416b2d2e4f4519bc3b51dc64986401494bc Mon Sep 17 00:00:00 2001
From: ahmedmoussa <ahmedmoussa@google.com>
Date: Wed, 17 Jan 2024 20:20:05 +0000
Subject: [PATCH] Fix an edge case with CameraVideoFrameHandler
The edge case might lead to unexpected behaviour. It happens when `Close()` function is called, then for some reason `camera_video_source_remote_` mojo becomes disconnected, leading to a call to `OnFatalErrorOrDisconnection()`. During `OnFatalErrorOrDisconnection()`, `camera_video_stream_subsciption_remote_` is reset, which would invoke the destructor of `CameraVideoFrameHandler`, while still in the middle of the function.
Change-Id: I9d72d7b8a0ac14fc6dd4aabdb765dcfe03d4f113
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5181880
Commit-Queue: Ahmed Moussa <ahmedmoussa@google.com>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1248321}
---
components/capture_mode/camera_video_frame_handler.cc | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/components/capture_mode/camera_video_frame_handler.cc b/components/capture_mode/camera_video_frame_handler.cc
index c6a9588edc9ae..44ffb71f3731e 100644
--- a/components/capture_mode/camera_video_frame_handler.cc
+++ b/components/capture_mode/camera_video_frame_handler.cc
@@ -689,7 +689,16 @@ void CameraVideoFrameHandler::OnFatalErrorOrDisconnection() {
weak_ptr_factory_.InvalidateWeakPtrs();
video_frame_handler_receiver_.reset();
camera_video_source_remote_.reset();
+
+ // There is a chance where resetting `camera_video_stream_subsciption_remote_`
+ // causes the deletion of `this`. That is the reason why we need to use a weak
+ // pointer, as to avoid unexpected behaviour.
+ // For more info check `Close()` function.
+ auto weak_ptr = weak_ptr_factory_.GetWeakPtr();
camera_video_stream_subsciption_remote_.reset();
+ if (!weak_ptr) {
+ return;
+ }
if (delegate_) {
delegate_->OnFatalErrorOrDisconnection();