[M100 merge] cras: Refresh Noise Cancellation state on login

Merge notes: Nothing changed from original CL. Noise cancellation support is fetched as soon as the Audio nodes are fetched so it is no longer required to fetch the support state at login. Fixes an edge case in which it was possible that the pref and actual noise cancellation state were mismatched on login. (cherry picked from commit a259036) Bug: b:218398659 Test: ash_components_unittest Change-Id: I65e7b16b735acca28a704b0cea438ba2c5b99791 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3534506 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#982944} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3541081 Cr-Commit-Position: refs/branch-heads/4896@{#759} Cr-Branched-From: 1f63ff4-refs/heads/main@{#972766}
chromium · Mar 21, 2022 · 9fbca5c · 9fbca5c
1 parent 617ca7e
commit 9fbca5c
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 23 deletions.
diff --git a/ash/components/audio/cras_audio_handler.cc b/ash/components/audio/cras_audio_handler.cc
@@ -449,6 +449,23 @@ bool CrasAudioHandler::GetNoiseCancellationState() const {
   return audio_pref_handler_->GetNoiseCancellationState();
 }
 
+void CrasAudioHandler::RefreshNoiseCancellationState() {
+  if (!noise_cancellation_supported()) {
+    return;
+  }
+
+  const AudioDevice* internal_mic =
+      GetDeviceByType(AudioDeviceType::kInternalMic);
+
+  if (!internal_mic) {
+    return;
+  }
+
+  SetNoiseCancellationState(
+      GetNoiseCancellationState() &&
+      (internal_mic->audio_effect & cras::EFFECT_TYPE_NOISE_CANCELLATION));
+}
+
 void CrasAudioHandler::SetNoiseCancellationState(bool state) {
   CrasAudioClient::Get()->SetNoiseCancellationEnabled(state);
 }
@@ -1708,15 +1725,7 @@ void CrasAudioHandler::HandleGetNodes(absl::optional<AudioNodeList> node_list) {
   UpdateDevicesAndSwitchActive(node_list.value());
 
   // Always set the input noise cancellation state on NodesChange event.
-  if (noise_cancellation_supported()) {
-    const AudioDevice* internal_mic =
-        GetDeviceByType(AudioDeviceType::kInternalMic);
-    if (internal_mic) {
-      SetNoiseCancellationState(
-          GetNoiseCancellationState() &&
-          (internal_mic->audio_effect & cras::EFFECT_TYPE_NOISE_CANCELLATION));
-    }
-  }
+  RefreshNoiseCancellationState();
 
   for (auto& observer : observers_)
     observer.OnAudioNodesChanged();

diff --git a/ash/components/audio/cras_audio_handler.h b/ash/components/audio/cras_audio_handler.h
@@ -272,6 +272,9 @@ class COMPONENT_EXPORT(ASH_COMPONENTS_AUDIO) CrasAudioHandler
   // Gets the state of input noise cancellation.
   bool GetNoiseCancellationState() const;
 
+  // Refreshes the input device noise cancellation state.
+  void RefreshNoiseCancellationState();
+
   // Sends a DBus signal to set the state of input noise cancellation.
   void SetNoiseCancellationState(bool state);
 

diff --git a/ash/components/audio/cras_audio_handler_unittest.cc b/ash/components/audio/cras_audio_handler_unittest.cc
@@ -419,6 +419,25 @@ class CrasAudioHandlerTest : public testing::TestWithParam<int> {
     base::RunLoop().RunUntilIdle();
   }
 
+  void SetUpCrasAudioHandlerWithPrimaryActiveNodeAndNoiseCancellationState(
+      const AudioNodeList& audio_nodes,
+      const AudioNode& primary_active_node,
+      bool noise_cancellation_enabled) {
+    CrasAudioClient::InitializeFake();
+    fake_cras_audio_client()->SetAudioNodesForTesting(audio_nodes);
+    fake_cras_audio_client()->SetActiveOutputNode(primary_active_node.id);
+    fake_cras_audio_client()->SetNoiseCancellationSupported(
+        /*noise_cancellation_supported=*/true);
+    audio_pref_handler_ = new AudioDevicesPrefHandlerStub();
+    audio_pref_handler_->SetNoiseCancellationState(noise_cancellation_enabled);
+    CrasAudioHandler::Initialize(fake_manager_->MakeRemote(),
+                                 audio_pref_handler_);
+    cras_audio_handler_ = CrasAudioHandler::Get();
+    test_observer_ = std::make_unique<TestObserver>();
+    cras_audio_handler_->AddAudioObserver(test_observer_.get());
+    base::RunLoop().RunUntilIdle();
+  }
+
   void ChangeAudioNodes(const AudioNodeList& audio_nodes) {
     fake_cras_audio_client()->SetAudioNodesAndNotifyObserversForTesting(
         audio_nodes);
@@ -1360,6 +1379,67 @@ TEST_P(CrasAudioHandlerTest, OneActiveAudioOutputAfterLoginNewUserSession) {
   }
 }
 
+TEST_P(CrasAudioHandlerTest, NoiseCancellationRefreshPrefEnabledNoNC) {
+  AudioNodeList audio_nodes = GenerateAudioNodeList({});
+  // Set up initial audio devices, only with internal mic.
+  AudioNode internalMic = GenerateAudioNode(kInternalMic);
+  // Clear the audio effect, no Noise Cancellation supported.
+  internalMic.audio_effect = 0u;
+  audio_nodes.push_back(internalMic);
+  // Simulate enable pref for noise cancellation.
+  SetUpCrasAudioHandlerWithPrimaryActiveNodeAndNoiseCancellationState(
+      audio_nodes, internalMic, /*noise_cancellation_enabled=*/true);
+
+  // Noise cancellation should still be disabled despite the pref being enabled
+  // since the audio_effect of the internal mic is unavailable.
+  EXPECT_FALSE(fake_cras_audio_client()->noise_cancellation_enabled());
+}
+
+TEST_P(CrasAudioHandlerTest, NoiseCancellationRefreshPrefEnabledWithNC) {
+  AudioNodeList audio_nodes = GenerateAudioNodeList({});
+  // Set up initial audio devices, only with internal mic.
+  AudioNode internalMic = GenerateAudioNode(kInternalMic);
+  // Enable noise cancellation effect.
+  internalMic.audio_effect = cras::EFFECT_TYPE_NOISE_CANCELLATION;
+  audio_nodes.push_back(internalMic);
+  // Simulate enable pref for noise cancellation.
+  SetUpCrasAudioHandlerWithPrimaryActiveNodeAndNoiseCancellationState(
+      audio_nodes, internalMic, /*noise_cancellation_enabled=*/true);
+
+  // Noise Cancellation is enabled.
+  EXPECT_TRUE(fake_cras_audio_client()->noise_cancellation_enabled());
+}
+
+TEST_P(CrasAudioHandlerTest, NoiseCancellationRefreshPrefDisableNoNC) {
+  AudioNodeList audio_nodes = GenerateAudioNodeList({});
+  // Set up initial audio devices, only with internal mic.
+  AudioNode internalMic = GenerateAudioNode(kInternalMic);
+  // Clear audio effect, no noise cancellation.
+  internalMic.audio_effect = 0u;
+  audio_nodes.push_back(internalMic);
+  // Simulate enable pref for noise cancellation.
+  SetUpCrasAudioHandlerWithPrimaryActiveNodeAndNoiseCancellationState(
+      audio_nodes, internalMic, /*noise_cancellation_enabled=*/false);
+
+  // Noise cancellation should still be disabled since the pref is disabled.
+  EXPECT_FALSE(fake_cras_audio_client()->noise_cancellation_enabled());
+}
+
+TEST_P(CrasAudioHandlerTest, NoiseCancellationRefreshPrefDisableWithNC) {
+  AudioNodeList audio_nodes = GenerateAudioNodeList({});
+  // Set up initial audio devices, only with internal mic.
+  AudioNode internalMic = GenerateAudioNode(kInternalMic);
+  // Enable noise cancellation effect.
+  internalMic.audio_effect = cras::EFFECT_TYPE_NOISE_CANCELLATION;
+  audio_nodes.push_back(internalMic);
+  // Simulate enable pref for noise cancellation.
+  SetUpCrasAudioHandlerWithPrimaryActiveNodeAndNoiseCancellationState(
+      audio_nodes, internalMic, /*noise_cancellation_enabled=*/false);
+
+  // Noise cancellation should still be disabled since the pref is disabled.
+  EXPECT_FALSE(fake_cras_audio_client()->noise_cancellation_enabled());
+}
+
 TEST_P(CrasAudioHandlerTest, BluetoothSpeakerIdChangedOnFly) {
   // Initialize with internal speaker and bluetooth headset.
   AudioNodeList audio_nodes =

diff --git a/chrome/browser/ash/login/session/user_session_initializer.cc b/chrome/browser/ash/login/session/user_session_initializer.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ash/login/session/user_session_initializer.h"
 
+#include "ash/components/audio/cras_audio_handler.h"
 #include "ash/components/peripheral_notification/peripheral_notification_manager.h"
 #include "ash/constants/ash_features.h"
 #include "ash/constants/ash_pref_names.h"
@@ -111,18 +112,6 @@ void OnGotNSSCertDatabaseForUser(net::NSSCertDatabase* database) {
   NetworkCertLoader::Get()->SetUserNSSDB(database);
 }
 
-void OnNoiseCancellationSupportedRetrieved(
-    absl::optional<bool> noise_cancellation_supported) {
-  if (noise_cancellation_supported.has_value() &&
-      noise_cancellation_supported.value()) {
-    PrefService* local_state = g_browser_process->local_state();
-    const bool noise_cancellation_enabled =
-        local_state->GetBoolean(prefs::kInputNoiseCancellationEnabled);
-    chromeos::CrasAudioClient::Get()->SetNoiseCancellationEnabled(
-        noise_cancellation_enabled);
-  }
-}
-
 }  // namespace
 
 UserSessionInitializer::UserSessionInitializer() {
@@ -290,8 +279,7 @@ void UserSessionInitializer::OnUserSessionStarted(bool is_primary_user) {
     PciguardClient::Get()->SendExternalPciDevicesPermissionState(
         chromeos::settings::PeripheralDataAccessHandler::GetPrefState());
 
-    chromeos::CrasAudioClient::Get()->GetNoiseCancellationSupported(
-        base::BindOnce(&OnNoiseCancellationSupportedRetrieved));
+    CrasAudioHandler::Get()->RefreshNoiseCancellationState();
   }
 }
 

diff --git a/chromeos/dbus/audio/fake_cras_audio_client.cc b/chromeos/dbus/audio/fake_cras_audio_client.cc
@@ -207,6 +207,7 @@ void FakeCrasAudioClient::SetNoiseCancellationSupported(
 
 void FakeCrasAudioClient::SetNoiseCancellationEnabled(
     bool noise_cancellation_on) {
+  noise_cancellation_enabled_ = noise_cancellation_on;
   ++noise_cancellation_enabled_counter_;
 }
 

diff --git a/chromeos/dbus/audio/fake_cras_audio_client.h b/chromeos/dbus/audio/fake_cras_audio_client.h
@@ -120,6 +120,10 @@ class COMPONENT_EXPORT(DBUS_AUDIO) FakeCrasAudioClient
     notify_volume_change_with_delay_ = notify_with_delay;
   }
 
+  bool noise_cancellation_enabled() const {
+    return noise_cancellation_enabled_;
+  }
+
  private:
   // Finds a node in the list based on the id.
   AudioNodeList::iterator FindNode(uint64_t node_id);
@@ -134,6 +138,7 @@ class COMPONENT_EXPORT(DBUS_AUDIO) FakeCrasAudioClient
   bool noise_cancellation_supported_ = false;
   uint32_t battery_level_ = 0;
   uint32_t noise_cancellation_enabled_counter_ = 0;
+  bool noise_cancellation_enabled_ = false;
   // Maps audio client type to the number of active input streams for clients
   // with the type specified
   ClientTypeToInputStreamCount active_input_streams_;