Commit 567a6178 authored by Shik Chen's avatar Shik Chen Committed by Commit Bot

[Video Capture, Chrome OS] Fix a race of ProcessDevicesChanged()

We need to wait until |camera_info_| is updated in
OnGotCameraInfoOnIpcThread() before we fire ProcessDevicesChanged() to
prevent the race condition.

To easily reproduce the race without this CL, add
--disable-features=MojoVideoCapture as the command line argument of
Chrome.

BUG=b:118418068,b:118408452,b:64996728,b:77833131
TEST=10.times do
       Plug the external camera
       Check the external camera is shown in settings
       Unplug the extencal camera
       Check the external camera is not shown in settings
     end

Change-Id: I3774c456099724f2bb5ac2d21caf3229d0045a4e
Reviewed-on: https://chromium-review.googlesource.com/c/1345677Reviewed-by: default avatarRicky Liang <jcliang@chromium.org>
Commit-Queue: Shik Chen <shik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609934}
parent b96a2016
...@@ -71,6 +71,15 @@ bool IsVividLoaded() { ...@@ -71,6 +71,15 @@ bool IsVividLoaded() {
}); });
} }
void NotifyVideoCaptureDevicesChanged() {
base::SystemMonitor* monitor = base::SystemMonitor::Get();
// |monitor| might be nullptr in unittest.
if (monitor) {
monitor->ProcessDevicesChanged(
base::SystemMonitor::DeviceType::DEVTYPE_VIDEO_CAPTURE);
}
}
} // namespace } // namespace
CameraHalDelegate::CameraHalDelegate( CameraHalDelegate::CameraHalDelegate(
...@@ -383,7 +392,6 @@ void CameraHalDelegate::OnGotCameraInfoOnIpcThread( ...@@ -383,7 +392,6 @@ void CameraHalDelegate::OnGotCameraInfoOnIpcThread(
if (result) { if (result) {
LOG(ERROR) << "Failed to get camera info. Camera id: " << camera_id; LOG(ERROR) << "Failed to get camera info. Camera id: " << camera_id;
} }
// In case of error |camera_info| is empty.
SortCameraMetadata(&camera_info->static_camera_characteristics); SortCameraMetadata(&camera_info->static_camera_characteristics);
base::AutoLock lock(camera_info_lock_); base::AutoLock lock(camera_info_lock_);
...@@ -404,6 +412,9 @@ void CameraHalDelegate::OnGotCameraInfoOnIpcThread( ...@@ -404,6 +412,9 @@ void CameraHalDelegate::OnGotCameraInfoOnIpcThread(
if (all_updated) { if (all_updated) {
builtin_camera_info_updated_.Signal(); builtin_camera_info_updated_.Signal();
} }
} else {
// It's an external camera.
NotifyVideoCaptureDevicesChanged();
} }
if (camera_info_.size() == 1) { if (camera_info_.size() == 1) {
...@@ -448,6 +459,7 @@ void CameraHalDelegate::CameraDeviceStatusChange( ...@@ -448,6 +459,7 @@ void CameraHalDelegate::CameraDeviceStatusChange(
if (camera_info_.empty()) { if (camera_info_.empty()) {
has_camera_connected_.Reset(); has_camera_connected_.Reset();
} }
NotifyVideoCaptureDevicesChanged();
} else { } else {
LOG(WARNING) << "Ignore nonexistent camera_id = " << camera_id; LOG(WARNING) << "Ignore nonexistent camera_id = " << camera_id;
} }
...@@ -455,12 +467,6 @@ void CameraHalDelegate::CameraDeviceStatusChange( ...@@ -455,12 +467,6 @@ void CameraHalDelegate::CameraDeviceStatusChange(
default: default:
NOTREACHED() << "Unexpected new status " << new_status; NOTREACHED() << "Unexpected new status " << new_status;
} }
base::SystemMonitor* monitor = base::SystemMonitor::Get();
// |monitor| might be nullptr in unittest.
if (monitor) {
monitor->ProcessDevicesChanged(
base::SystemMonitor::DeviceType::DEVTYPE_VIDEO_CAPTURE);
}
} }
void CameraHalDelegate::TorchModeStatusChange( void CameraHalDelegate::TorchModeStatusChange(
......
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