Commit 1402722e authored by Jasmine Chen's avatar Jasmine Chen Committed by Chromium LUCI CQ

VCD: Use ObserverListThreadSafe for CameraActiveClientObserver

Use ObserverListThreadSafe for CameraActiveClientObserver instead of
ObserverList. Also add a lock to guard the list.

Bug: b/170075468
Test: simplechrome
Change-Id: I976200eed4e24f036e96481a145250aa90fdc3fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2566329Reviewed-by: default avatarWei Lee <wtlee@chromium.org>
Commit-Queue: Jasmine Chen <lnishan@google.com>
Cr-Commit-Position: refs/heads/master@{#833613}
parent 1f9ef980
......@@ -187,25 +187,20 @@ bool CameraHalDispatcherImpl::IsStarted() {
void CameraHalDispatcherImpl::AddActiveClientObserver(
CameraActiveClientObserver* observer) {
base::WaitableEvent observer_added_event;
proxy_thread_.task_runner()->PostTask(
FROM_HERE,
base::BindOnce(
&CameraHalDispatcherImpl::AddActiveClientObserverOnProxyThread,
base::Unretained(this), std::move(observer), &observer_added_event));
observer_added_event.Wait();
base::AutoLock lock(opened_camera_id_map_lock_);
for (auto& opened_camera_id_pair : opened_camera_id_map_) {
const auto& camera_client_type = opened_camera_id_pair.first;
const auto& camera_id_set = opened_camera_id_pair.second;
if (!camera_id_set.empty()) {
observer->OnActiveClientChange(camera_client_type, /*is_active=*/true);
}
}
active_client_observers_->AddObserver(observer);
}
void CameraHalDispatcherImpl::RemoveActiveClientObserver(
CameraActiveClientObserver* observer) {
base::WaitableEvent observer_removed_event;
proxy_thread_.task_runner()->PostTask(
FROM_HERE,
base::BindOnce(
&CameraHalDispatcherImpl::RemoveActiveClientObserverOnProxyThread,
base::Unretained(this), std::move(observer),
&observer_removed_event));
observer_removed_event.Wait();
active_client_observers_->RemoveObserver(observer);
}
void CameraHalDispatcherImpl::RegisterPluginVmToken(
......@@ -326,6 +321,7 @@ void CameraHalDispatcherImpl::CameraDeviceActivityChange(
cros::mojom::CameraClientType type) {
VLOG(1) << type << (opened ? " opened " : " closed ") << "camera "
<< camera_id;
base::AutoLock lock(opened_camera_id_map_lock_);
auto& camera_id_set = opened_camera_id_map_[type];
if (opened) {
auto result = camera_id_set.insert(camera_id);
......@@ -336,9 +332,9 @@ void CameraHalDispatcherImpl::CameraDeviceActivityChange(
}
if (camera_id_set.size() == 1) {
VLOG(1) << type << " is active";
for (auto& observer : active_client_observers_) {
observer.OnActiveClientChange(type, /*is_active=*/true);
}
active_client_observers_->Notify(
FROM_HERE, &CameraActiveClientObserver::OnActiveClientChange, type,
/*is_active=*/true);
}
} else {
auto it = camera_id_set.find(camera_id);
......@@ -353,9 +349,9 @@ void CameraHalDispatcherImpl::CameraDeviceActivityChange(
camera_id_set.erase(it);
if (camera_id_set.empty()) {
VLOG(1) << type << " is inactive";
for (auto& observer : active_client_observers_) {
observer.OnActiveClientChange(type, /*is_active=*/false);
}
active_client_observers_->Notify(
FROM_HERE, &CameraActiveClientObserver::OnActiveClientChange, type,
/*is_active=*/false);
}
}
}
......@@ -532,29 +528,6 @@ void CameraHalDispatcherImpl::AddClientObserverOnProxyThread(
VLOG(1) << "Camera HAL client registered";
}
void CameraHalDispatcherImpl::AddActiveClientObserverOnProxyThread(
CameraActiveClientObserver* observer,
base::WaitableEvent* observer_added_event) {
DCHECK(proxy_task_runner_->BelongsToCurrentThread());
for (auto& opened_camera_id_pair : opened_camera_id_map_) {
const auto& camera_client_type = opened_camera_id_pair.first;
const auto& camera_id_set = opened_camera_id_pair.second;
if (!camera_id_set.empty()) {
observer->OnActiveClientChange(camera_client_type, /*is_active=*/true);
}
}
active_client_observers_.AddObserver(observer);
observer_added_event->Signal();
}
void CameraHalDispatcherImpl::RemoveActiveClientObserverOnProxyThread(
CameraActiveClientObserver* observer,
base::WaitableEvent* observer_removed_event) {
DCHECK(proxy_task_runner_->BelongsToCurrentThread());
active_client_observers_.RemoveObserver(observer);
observer_removed_event->Signal();
}
void CameraHalDispatcherImpl::EstablishMojoChannel(
CameraClientObserver* client_observer) {
DCHECK(proxy_task_runner_->BelongsToCurrentThread());
......@@ -577,6 +550,7 @@ void CameraHalDispatcherImpl::OnPeerConnected(
void CameraHalDispatcherImpl::OnCameraHalServerConnectionError() {
DCHECK(proxy_task_runner_->BelongsToCurrentThread());
base::AutoLock lock(opened_camera_id_map_lock_);
VLOG(1) << "Camera HAL server connection lost";
camera_hal_server_.reset();
camera_hal_server_callbacks_.reset();
......@@ -584,9 +558,9 @@ void CameraHalDispatcherImpl::OnCameraHalServerConnectionError() {
auto camera_client_type = opened_camera_id_pair.first;
const auto& camera_id_set = opened_camera_id_pair.second;
if (!camera_id_set.empty()) {
for (auto& observer : active_client_observers_) {
observer.OnActiveClientChange(camera_client_type, /*is_active=*/false);
}
active_client_observers_->Notify(
FROM_HERE, &CameraActiveClientObserver::OnActiveClientChange,
camera_client_type, /*is_active=*/false);
}
}
opened_camera_id_map_.clear();
......@@ -595,18 +569,18 @@ void CameraHalDispatcherImpl::OnCameraHalServerConnectionError() {
void CameraHalDispatcherImpl::OnCameraHalClientConnectionError(
CameraClientObserver* client_observer) {
DCHECK(proxy_task_runner_->BelongsToCurrentThread());
auto type = client_observer->GetType();
auto opened_it = opened_camera_id_map_.find(type);
base::AutoLock lock(opened_camera_id_map_lock_);
auto camera_client_type = client_observer->GetType();
auto opened_it = opened_camera_id_map_.find(camera_client_type);
if (opened_it == opened_camera_id_map_.end()) {
// This can happen if this camera client never opened a camera.
return;
}
const auto& camera_id_set = opened_it->second;
if (!camera_id_set.empty()) {
for (auto& observer : active_client_observers_) {
observer.OnActiveClientChange(type,
/*is_active=*/false);
}
active_client_observers_->Notify(
FROM_HERE, &CameraActiveClientObserver::OnActiveClientChange,
camera_client_type, /*is_active=*/false);
}
opened_camera_id_map_.erase(opened_it);
......
......@@ -12,10 +12,12 @@
#include "base/containers/flat_set.h"
#include "base/containers/unique_ptr_adapters.h"
#include "base/files/scoped_file.h"
#include "base/memory/scoped_refptr.h"
#include "base/memory/singleton.h"
#include "base/observer_list.h"
#include "base/observer_list_threadsafe.h"
#include "base/observer_list_types.h"
#include "base/synchronization/waitable_event.h"
#include "base/thread_annotations.h"
#include "base/threading/thread.h"
#include "base/unguessable_token.h"
#include "components/chromeos_camera/common/jpeg_encode_accelerator.mojom.h"
......@@ -190,14 +192,6 @@ class CAPTURE_EXPORT CameraHalDispatcherImpl final
std::unique_ptr<CameraClientObserver> observer,
base::OnceCallback<void(int32_t)> result_callback);
void AddActiveClientObserverOnProxyThread(
CameraActiveClientObserver* observer,
base::WaitableEvent* observer_added_event);
void RemoveActiveClientObserverOnProxyThread(
CameraActiveClientObserver* observer,
base::WaitableEvent* observer_removed_event);
void EstablishMojoChannel(CameraClientObserver* client_observer);
// Handler for incoming Mojo connection on the unix domain socket.
......@@ -239,9 +233,12 @@ class CAPTURE_EXPORT CameraHalDispatcherImpl final
TokenManager token_manager_;
base::Lock opened_camera_id_map_lock_;
base::flat_map<cros::mojom::CameraClientType, base::flat_set<int32_t>>
opened_camera_id_map_;
base::ObserverList<CameraActiveClientObserver> active_client_observers_;
opened_camera_id_map_ GUARDED_BY(opened_camera_id_map_lock_);
scoped_refptr<base::ObserverListThreadSafe<CameraActiveClientObserver>>
active_client_observers_;
DISALLOW_COPY_AND_ASSIGN(CameraHalDispatcherImpl);
};
......
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