Ensure MediaStreamManager doesn't cache failed enumerations

Add tracking to CacheInfo of whether the last received update was a
success, and if not, trigger a new enumeration when needed.

Tested locally injecting errors in the camera service (with DeviceMonitorMac feature enabled this time). Previous testing using
the root cause of crbug.com/1379392 required passing --disable-features=DeviceMonitorMac for local builds, hiding this bug.

Bug: 1313822
Change-Id: I20b926abe802ac5e041db0b19a66985c08650c41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4079649
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Tony Herre <toprice@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1080311}

content/browser/renderer_host/media/media_devices_manager.cc

@@ -253,11 +253,7 @@ struct MediaDevicesManager::EnumerationRequest {
 // redundant enumerations.
 class MediaDevicesManager::CacheInfo {
  public:
-  CacheInfo()
-      : current_event_sequence_(0),
-        seq_last_update_(0),
-        seq_last_invalidation_(0),
-        is_update_ongoing_(false) {}
+  CacheInfo() = default;
 
   void InvalidateCache() {
     DCHECK(thread_checker_.CalledOnValidThread());
@@ -266,7 +262,8 @@ class MediaDevicesManager::CacheInfo {
 
   bool IsLastUpdateValid() const {
     DCHECK(thread_checker_.CalledOnValidThread());
-    return seq_last_update_ > seq_last_invalidation_ && !is_update_ongoing_;
+    return seq_last_update_ > seq_last_invalidation_ && !is_update_ongoing_ &&
+           (last_update_result_ == UpdateResult::kSuccess);
   }
 
   void UpdateStarted() {
@@ -276,10 +273,12 @@ class MediaDevicesManager::CacheInfo {
     is_update_ongoing_ = true;
   }
 
-  void UpdateCompleted() {
+  enum class UpdateResult { kSuccess, kFailure };
+  void UpdateCompleted(UpdateResult result) {
     DCHECK(thread_checker_.CalledOnValidThread());
     DCHECK(is_update_ongoing_);
     is_update_ongoing_ = false;
+    last_update_result_ = result;
   }
 
   bool is_update_ongoing() const {
@@ -293,10 +292,11 @@ class MediaDevicesManager::CacheInfo {
     return ++current_event_sequence_;
   }
 
-  int64_t current_event_sequence_;
-  int64_t seq_last_update_;
-  int64_t seq_last_invalidation_;
-  bool is_update_ongoing_;
+  int64_t current_event_sequence_ = 0;
+  int64_t seq_last_update_ = 0;
+  int64_t seq_last_invalidation_ = 0;
+  bool is_update_ongoing_ = false;
+  UpdateResult last_update_result_ = UpdateResult::kSuccess;
   base::ThreadChecker thread_checker_;
 };
 
@@ -414,7 +414,11 @@ void MediaDevicesManager::EnumerateDevices(
   bool all_results_cached = true;
   for (size_t i = 0;
        i < static_cast<size_t>(MediaDeviceType::NUM_MEDIA_DEVICE_TYPES); ++i) {
-    if (requested_types[i] && cache_policies_[i] == CachePolicy::NO_CACHE) {
+    bool value_cached_or_being_updated = cache_infos_[i].IsLastUpdateValid() ||
+                                         cache_infos_[i].is_update_ongoing();
+    bool enumeration_required = cache_policies_[i] == CachePolicy::NO_CACHE ||
+                                !value_cached_or_being_updated;
+    if (requested_types[i] && enumeration_required) {
       all_results_cached = false;
       DoEnumerateDevices(static_cast<MediaDeviceType>(i));
     }
@@ -966,7 +970,7 @@ void MediaDevicesManager::VideoInputDevicesEnumerated(
       return false;
     });
     cache_infos_[static_cast<size_t>(MediaDeviceType::MEDIA_VIDEO_INPUT)]
-        .UpdateCompleted();
+        .UpdateCompleted(CacheInfo::UpdateResult::kFailure);
 
     return;
   }
@@ -997,7 +1001,8 @@ void MediaDevicesManager::DevicesEnumerated(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(blink::IsValidMediaDeviceType(type));
   UpdateSnapshot(type, snapshot);
-  cache_infos_[static_cast<size_t>(type)].UpdateCompleted();
+  cache_infos_[static_cast<size_t>(type)].UpdateCompleted(
+      CacheInfo::UpdateResult::kSuccess);
   has_seen_result_[static_cast<size_t>(type)] = true;
   SendLogMessage(GetDevicesEnumeratedLogString(type, snapshot));
 

content/browser/renderer_host/media/media_devices_manager.h

@@ -208,8 +208,8 @@ class CONTENT_EXPORT MediaDevicesManager
   // EnumerateDevices is called. The results of a new or in-progress low-level
   // device enumeration are used.
   // The SYSTEM_MONITOR policy is such that previous results are re-used,
-  // provided they were produced by a low-level device enumeration issued after
-  // the last call to OnDevicesChanged.
+  // provided they were successfully produced by a low-level device enumeration
+  // issued after the last call to OnDevicesChanged.
   enum class CachePolicy {
     NO_CACHE,
     SYSTEM_MONITOR,

content/browser/renderer_host/media/media_devices_manager_unittest.cc

@@ -1424,4 +1424,125 @@ TEST_F(MediaDevicesManagerTest,
   }
 }
 
+TEST_F(MediaDevicesManagerTest, EnumerateCacheVideoErrorFirstCall) {
+  EXPECT_CALL(*audio_manager_, MockGetAudioInputDeviceNames(_))
+      .Times(kNumCalls);
+  EXPECT_CALL(*video_capture_device_factory_, MockGetDevicesInfo()).Times(1);
+  EXPECT_CALL(media_devices_manager_client_,
+              InputDevicesChangedUI(MediaDeviceType::MEDIA_AUDIO_INPUT, _));
+  EXPECT_CALL(media_devices_manager_client_,
+              InputDevicesChangedUI(MediaDeviceType::MEDIA_VIDEO_INPUT, _));
+
+  // Inject an UnknownError when attempting to fetch initial state after
+  // enabling caching.
+  EXPECT_CALL(*mock_video_capture_provider_, GetDeviceInfosAsync(_))
+      .WillOnce(Invoke(
+          [&](VideoCaptureProvider::GetDeviceInfosCallback result_callback) {
+            std::move(result_callback)
+                .Run(DeviceEnumerationResult::kUnknownError, {});
+          }));
+
+  EnableCache(MediaDeviceType::MEDIA_VIDEO_INPUT);
+
+  // Inject one error and one success when media_devices_manager attempts
+  // re-enumerating using the lower level APIs. Further calls to
+  // media_devices_manager_->EnumerateDevices() should use the cache instead of
+  // triggering this API.
+  EXPECT_CALL(*mock_video_capture_provider_, GetDeviceInfosAsync(_))
+      .WillOnce(Invoke(
+          [&](VideoCaptureProvider::GetDeviceInfosCallback result_callback) {
+            std::move(result_callback)
+                .Run(DeviceEnumerationResult::kUnknownError, {});
+          }))
+      .WillOnce(Invoke(
+          [&](VideoCaptureProvider::GetDeviceInfosCallback result_callback) {
+            in_process_video_capture_provider_->GetDeviceInfosAsync(
+                std::move(result_callback));
+          }));
+
+  MediaDevicesManager::BoolDeviceTypes devices_to_enumerate;
+  devices_to_enumerate[static_cast<size_t>(
+      MediaDeviceType::MEDIA_VIDEO_INPUT)] = true;
+  devices_to_enumerate[static_cast<size_t>(
+      MediaDeviceType::MEDIA_AUDIO_INPUT)] = true;
+  for (int i = 0; i < kNumCalls; i++) {
+    base::RunLoop run_loop;
+    media_devices_manager_->EnumerateDevices(
+        -1, -1, devices_to_enumerate, true, true,
+        base::BindOnce(
+            [](bool expect_success, base::RunLoop* run_loop,
+               EnumerationResponsePtr response) {
+              EXPECT_EQ(response->result_code,
+                        expect_success
+                            ? DeviceEnumerationResult::kSuccess
+                            : DeviceEnumerationResult::kUnknownError);
+              run_loop->Quit();
+            },
+            /*expect_success=*/i > 0, &run_loop));
+    run_loop.Run();
+  }
+}
+
+TEST_F(MediaDevicesManagerTest, EnumerateCacheVideoErrorAfterChange) {
+  EXPECT_CALL(*video_capture_device_factory_, MockGetDevicesInfo()).Times(2);
+  EXPECT_CALL(media_devices_manager_client_,
+              InputDevicesChangedUI(MediaDeviceType::MEDIA_AUDIO_INPUT, _));
+  EXPECT_CALL(media_devices_manager_client_,
+              InputDevicesChangedUI(MediaDeviceType::MEDIA_VIDEO_INPUT, _));
+  EXPECT_CALL(*audio_manager_, MockGetAudioInputDeviceNames(_)).Times(2);
+  // Insert a success (for the first enumeration in EnableCache()), a failure
+  // (for the re-enumeration on device change event) and finally a success (when
+  // we actually call EnumerateDevices)
+  EXPECT_CALL(*mock_video_capture_provider_, GetDeviceInfosAsync(_))
+      .WillOnce(Invoke(
+          [&](VideoCaptureProvider::GetDeviceInfosCallback result_callback) {
+            in_process_video_capture_provider_->GetDeviceInfosAsync(
+                std::move(result_callback));
+          }))
+      .WillOnce(Invoke(
+          [&](VideoCaptureProvider::GetDeviceInfosCallback result_callback) {
+            std::move(result_callback)
+                .Run(DeviceEnumerationResult::kUnknownError, {});
+          }))
+      .WillOnce(Invoke(
+          [&](VideoCaptureProvider::GetDeviceInfosCallback result_callback) {
+            in_process_video_capture_provider_->GetDeviceInfosAsync(
+                std::move(result_callback));
+          }));
+
+  EnableCache(MediaDeviceType::MEDIA_VIDEO_INPUT);
+  // Perform an initial enumerate devices to wait for the initial enumeration
+  // triggered by EnableCache to finish.
+  EXPECT_CALL(*this, MockCallback(DeviceEnumerationResult::kSuccess, _));
+  RunEnumerateDevices();
+
+  // Invalidate the cache of video devices, triggering a re-enumeration against
+  // the mock_video_capture_provider_ which will fail, but not be cached.
+  media_devices_manager_->OnDevicesChanged(
+      base::SystemMonitor::DEVTYPE_VIDEO_CAPTURE);
+
+  // Calling EnumerateDevices should trigger a re-enumeration against the
+  // mock_video_capture_provider_ which succeeds and returns the full result.
+  MediaDevicesManager::BoolDeviceTypes devices_to_enumerate;
+  devices_to_enumerate[static_cast<size_t>(
+      MediaDeviceType::MEDIA_VIDEO_INPUT)] = true;
+  devices_to_enumerate[static_cast<size_t>(
+      MediaDeviceType::MEDIA_AUDIO_INPUT)] = true;
+  base::RunLoop run_loop;
+  media_devices_manager_->EnumerateDevices(
+      -1, -1, devices_to_enumerate, true, true,
+      base::BindOnce(
+          [](base::RunLoop* run_loop, EnumerationResponsePtr response) {
+            EXPECT_EQ(response->result_code, DeviceEnumerationResult::kSuccess);
+            EXPECT_GT(response
+                          ->enumeration[static_cast<size_t>(
+                              MediaDeviceType::MEDIA_VIDEO_INPUT)]
+                          .size(),
+                      size_t(0));
+            run_loop->Quit();
+          },
+          &run_loop));
+  run_loop.Run();
+}
+
 }  // namespace content