Commit 775d5c53 authored by Wei Lee's avatar Wei Lee Committed by Chromium LUCI CQ

camera: Fix DCHECK failure when taking photo via CCA

This CL tweaks the logic in CameraAppDeviceImpl a bit to make sure the
calls which need to be run on Mojo thread actually run on it.

Bug: b/175185495
Test: Manually
Change-Id: I2480d43fec56667741cb096f39f1a189e0326757
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2592182Reviewed-by: default avatarShik Chen <shik@chromium.org>
Commit-Queue: Shik Chen <shik@chromium.org>
Auto-Submit: Wei Lee <wtlee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837931}
parent 1182fb1d
......@@ -4,6 +4,7 @@
#include "media/capture/video/chromeos/camera_app_device_impl.h"
#include "media/base/bind_to_current_loop.h"
#include "media/capture/video/chromeos/camera_metadata_utils.h"
namespace media {
......@@ -61,7 +62,7 @@ CameraAppDeviceImpl::CameraAppDeviceImpl(const std::string& device_id,
cros::mojom::CameraInfoPtr camera_info)
: device_id_(device_id),
camera_info_(std::move(camera_info)),
task_runner_(base::ThreadTaskRunnerHandle::Get()),
creation_task_runner_(base::ThreadTaskRunnerHandle::Get()),
capture_intent_(cros::mojom::CaptureIntent::DEFAULT),
next_metadata_observer_id_(0),
next_camera_event_observer_id_(0),
......@@ -69,12 +70,13 @@ CameraAppDeviceImpl::CameraAppDeviceImpl(const std::string& device_id,
std::make_unique<base::WeakPtrFactory<CameraAppDeviceImpl>>(this)) {}
CameraAppDeviceImpl::~CameraAppDeviceImpl() {
task_runner_->DeleteSoon(FROM_HERE, std::move(weak_ptr_factory_));
creation_task_runner_->DeleteSoon(FROM_HERE, std::move(weak_ptr_factory_));
}
void CameraAppDeviceImpl::BindReceiver(
mojo::PendingReceiver<cros::mojom::CameraAppDevice> receiver) {
receivers_.Add(this, std::move(receiver));
mojo_task_runner_ = base::ThreadTaskRunnerHandle::Get();
}
void CameraAppDeviceImpl::ConsumeReprocessOptions(
......@@ -130,26 +132,29 @@ void CameraAppDeviceImpl::OnResultMetadataAvailable(
}
void CameraAppDeviceImpl::OnShutterDone() {
base::AutoLock lock(camera_event_observers_lock_);
for (auto& observer : camera_event_observers_) {
observer.second->OnShutterDone();
}
mojo_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&CameraAppDeviceImpl::NotifyShutterDoneOnMojoThread,
weak_ptr_factory_->GetWeakPtr()));
}
void CameraAppDeviceImpl::GetCameraInfo(GetCameraInfoCallback callback) {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
DCHECK(camera_info_);
std::move(callback).Run(camera_info_.Clone());
}
void CameraAppDeviceImpl::SetReprocessOption(
cros::mojom::Effect effect,
SetReprocessOptionCallback reprocess_result_callback) {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
ReprocessTask task;
task.effect = effect;
task.callback = base::BindOnce(&CameraAppDeviceImpl::SetReprocessResult,
weak_ptr_factory_->GetWeakPtr(),
std::move(reprocess_result_callback));
task.callback = media::BindToCurrentLoop(base::BindOnce(
&CameraAppDeviceImpl::SetReprocessResultOnMojoThread,
weak_ptr_factory_->GetWeakPtr(), std::move(reprocess_result_callback)));
if (effect == cros::mojom::Effect::PORTRAIT_MODE) {
auto e = BuildMetadataEntry(
......@@ -165,6 +170,8 @@ void CameraAppDeviceImpl::SetReprocessOption(
void CameraAppDeviceImpl::SetFpsRange(const gfx::Range& fps_range,
SetFpsRangeCallback callback) {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
const int entry_length = 2;
auto& static_metadata = camera_info_->static_camera_characteristics;
......@@ -198,6 +205,8 @@ void CameraAppDeviceImpl::SetFpsRange(const gfx::Range& fps_range,
void CameraAppDeviceImpl::SetStillCaptureResolution(
const gfx::Size& resolution,
SetStillCaptureResolutionCallback callback) {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
base::AutoLock lock(still_capture_resolution_lock_);
still_capture_resolution_ = resolution;
std::move(callback).Run();
......@@ -206,6 +215,8 @@ void CameraAppDeviceImpl::SetStillCaptureResolution(
void CameraAppDeviceImpl::SetCaptureIntent(
cros::mojom::CaptureIntent capture_intent,
SetCaptureIntentCallback callback) {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
base::AutoLock lock(capture_intent_lock_);
capture_intent_ = capture_intent;
std::move(callback).Run();
......@@ -215,6 +226,8 @@ void CameraAppDeviceImpl::AddResultMetadataObserver(
mojo::PendingRemote<cros::mojom::ResultMetadataObserver> observer,
cros::mojom::StreamType stream_type,
AddResultMetadataObserverCallback callback) {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
base::AutoLock lock(metadata_observers_lock_);
uint32_t id = next_metadata_observer_id_++;
......@@ -228,6 +241,8 @@ void CameraAppDeviceImpl::AddResultMetadataObserver(
void CameraAppDeviceImpl::RemoveResultMetadataObserver(
uint32_t id,
RemoveResultMetadataObserverCallback callback) {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
base::AutoLock lock(metadata_observers_lock_);
if (metadata_observers_.erase(id) == 0) {
......@@ -245,7 +260,7 @@ void CameraAppDeviceImpl::RemoveResultMetadataObserver(
void CameraAppDeviceImpl::AddCameraEventObserver(
mojo::PendingRemote<cros::mojom::CameraEventObserver> observer,
AddCameraEventObserverCallback callback) {
base::AutoLock lock(camera_event_observers_lock_);
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
uint32_t id = next_camera_event_observer_id_++;
camera_event_observers_[id] =
......@@ -256,7 +271,7 @@ void CameraAppDeviceImpl::AddCameraEventObserver(
void CameraAppDeviceImpl::RemoveCameraEventObserver(
uint32_t id,
RemoveCameraEventObserverCallback callback) {
base::AutoLock lock(camera_event_observers_lock_);
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
bool is_success = camera_event_observers_.erase(id) == 1;
std::move(callback).Run(is_success);
......@@ -274,17 +289,21 @@ void CameraAppDeviceImpl::DisableEeNr(ReprocessTask* task) {
task->extra_metadata.push_back(std::move(nr_entry));
}
void CameraAppDeviceImpl::SetReprocessResult(
void CameraAppDeviceImpl::SetReprocessResultOnMojoThread(
SetReprocessOptionCallback callback,
const int32_t status,
media::mojom::BlobPtr blob) {
auto callback_on_mojo_thread = base::BindOnce(
[](const int32_t status, media::mojom::BlobPtr blob,
SetReprocessOptionCallback callback) {
std::move(callback).Run(status, std::move(blob));
},
status, std::move(blob), std::move(callback));
task_runner_->PostTask(FROM_HERE, std::move(callback_on_mojo_thread));
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
std::move(callback).Run(status, std::move(blob));
}
void CameraAppDeviceImpl::NotifyShutterDoneOnMojoThread() {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
for (auto& observer : camera_event_observers_) {
observer.second->OnShutterDone();
}
}
} // namespace media
......@@ -121,9 +121,11 @@ class CAPTURE_EXPORT CameraAppDeviceImpl : public cros::mojom::CameraAppDevice {
private:
static void DisableEeNr(ReprocessTask* task);
void SetReprocessResult(SetReprocessOptionCallback callback,
const int32_t status,
media::mojom::BlobPtr blob);
void SetReprocessResultOnMojoThread(SetReprocessOptionCallback callback,
const int32_t status,
media::mojom::BlobPtr blob);
void NotifyShutterDoneOnMojoThread();
std::string device_id_;
......@@ -131,7 +133,11 @@ class CAPTURE_EXPORT CameraAppDeviceImpl : public cros::mojom::CameraAppDevice {
cros::mojom::CameraInfoPtr camera_info_;
const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
// It is used to invalidate weak pointer.
const scoped_refptr<base::SingleThreadTaskRunner> creation_task_runner_;
// It is used for calls which should run on the mojo thread.
scoped_refptr<base::SingleThreadTaskRunner> mojo_task_runner_;
// The queue will be enqueued and dequeued from different threads.
base::Lock reprocess_tasks_lock_;
......@@ -159,12 +165,9 @@ class CAPTURE_EXPORT CameraAppDeviceImpl : public cros::mojom::CameraAppDevice {
base::flat_map<cros::mojom::StreamType, base::flat_set<uint32_t>>
stream_metadata_observer_ids_ GUARDED_BY(metadata_observers_lock_);
// Those maps will be changed and used from different threads.
base::Lock camera_event_observers_lock_;
uint32_t next_camera_event_observer_id_
GUARDED_BY(camera_event_observers_lock_);
uint32_t next_camera_event_observer_id_;
base::flat_map<uint32_t, mojo::Remote<cros::mojom::CameraEventObserver>>
camera_event_observers_ GUARDED_BY(camera_event_observers_lock_);
camera_event_observers_;
std::unique_ptr<base::WeakPtrFactory<CameraAppDeviceImpl>> weak_ptr_factory_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment