[Telemetry] Report result for SetAudioGain in Management API

Setting audio gain should only be available for input audio nodes.
Implement this in `SetAudioGain` in Management API, including changing
the interface to have a boolean response.

Bug: b/303391740, b/317014106
Test: unit_tests --gtest_filter="TelemetryManagementServiceAshTest*"
Test: browser_tests --gtest_filter="TelemetryExtension*"
Test: testing/xvfb.py build/lacros/test_runner.py test out_linux_lacros/Release/lacros_chrome_browsertests --gtest_filter="TelemetryExtension*"
Test: testing/xvfb.py ./build/lacros/test_runner.py test out_linux_lacros/Release/lacros_chrome_browsertests --ash-chrome-path-override=lacros_version_skew_tests_v$version/test_ash_chrome --gtest_filter="TelemetryExtension*"
Change-Id: Ia6fb2b9a80acd959438e4c287f186eca4ba76c14
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5196714
Reviewed-by: Kerker Yang <kerker@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Denny Huang <dennyh@google.com>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1247374}

chrome/browser/ash/telemetry_extension/management/telemetry_management_service_ash.cc

@@ -66,9 +66,16 @@ void TelemetryManagementServiceAsh::SetAudioGain(
     uint64_t node_id,
     int32_t gain,
     SetAudioGainCallback callback) {
+  // Only input audio node is supported.
+  const AudioDevice* device = CrasAudioHandler::Get()->GetDeviceFromId(node_id);
+  if (!device || !device->is_input) {
+    std::move(callback).Run(false);
+    return;
+  }
+
   gain = std::clamp(gain, kMinAudioGain, kMaxAudioGain);
   CrasAudioHandler::Get()->SetVolumeGainPercentForDevice(node_id, gain);
-  std::move(callback).Run();
+  std::move(callback).Run(true);
 }
 
 void TelemetryManagementServiceAsh::SetAudioVolume(

chrome/browser/ash/telemetry_extension/management/telemetry_management_service_ash_unittest.cc

@@ -71,49 +71,64 @@ TEST_F(TelemetryManagementServiceAshTest, AudioSetGainSuccess) {
   cras_audio_handler().SetVolumeGainPercentForDevice(kFakeAudioInputNodeId, 10);
 
   constexpr int32_t expected_gain = 60;
-  base::test::TestFuture<void> future;
+  base::test::TestFuture<bool> future;
   management_service()->SetAudioGain(kFakeAudioInputNodeId, expected_gain,
                                      future.GetCallback());
-  EXPECT_TRUE(future.Wait());
+  EXPECT_TRUE(future.Get());
   EXPECT_EQ(
       cras_audio_handler().GetInputGainPercentForDevice(kFakeAudioInputNodeId),
       expected_gain);
 }
 
-// Tests that AudioSetGain returns false when |gain| is above max (100).
+// Tests that AudioSetGain sets gain to max (100) when |gain| exceeds max.
 TEST_F(TelemetryManagementServiceAshTest, AudioSetGainInvalidGainAboveMax) {
   // Set to an arbitrary value first.
   cras_audio_handler().SetVolumeGainPercentForDevice(kFakeAudioInputNodeId, 10);
 
-  base::test::TestFuture<void> future;
+  base::test::TestFuture<bool> future;
   management_service()->SetAudioGain(kFakeAudioInputNodeId, 999,
                                      future.GetCallback());
-  EXPECT_TRUE(future.Wait());
+  EXPECT_TRUE(future.Get());
   EXPECT_EQ(
       cras_audio_handler().GetInputGainPercentForDevice(kFakeAudioInputNodeId),
       100);
 }
 
-// Tests that AudioSetGain returns false when |gain| is below min (0).
+// Tests that AudioSetGain sets gain to min (0) when |gain| is below min.
 TEST_F(TelemetryManagementServiceAshTest, AudioSetGainInvalidGainBelowMin) {
   // Set to an arbitrary value first.
   cras_audio_handler().SetVolumeGainPercentForDevice(kFakeAudioInputNodeId, 10);
 
-  base::test::TestFuture<void> future;
+  base::test::TestFuture<bool> future;
   management_service()->SetAudioGain(kFakeAudioInputNodeId, -100,
                                      future.GetCallback());
-  EXPECT_TRUE(future.Wait());
+  EXPECT_TRUE(future.Get());
   EXPECT_EQ(
       cras_audio_handler().GetInputGainPercentForDevice(kFakeAudioInputNodeId),
       0);
 }
 
-// Tests that AudioSetGain returns true (but no-op) when |node_id| is invalid.
+// Tests that AudioSetGain returns false when |node_id| is invalid.
 TEST_F(TelemetryManagementServiceAshTest, AudioSetGainInvalidNodeId) {
-  base::test::TestFuture<void> future;
+  base::test::TestFuture<bool> future;
   management_service()->SetAudioGain(GetFakeAudioNodeId(), 60,
                                      future.GetCallback());
-  EXPECT_TRUE(future.Wait());
+  EXPECT_FALSE(future.Get());
+}
+
+// Tests that AudioSetGain return false if the audio node is an output node.
+TEST_F(TelemetryManagementServiceAshTest, AudioSetGainWithOutputNode) {
+  // Set to an arbitrary value first.
+  cras_audio_handler().SetVolumeGainPercentForDevice(kFakeAudioOutputNodeId,
+                                                     10);
+
+  base::test::TestFuture<bool> future;
+  management_service()->SetAudioGain(kFakeAudioOutputNodeId, 60,
+                                     future.GetCallback());
+  EXPECT_FALSE(future.Get());
+  EXPECT_EQ(
+      cras_audio_handler().GetInputGainPercentForDevice(kFakeAudioOutputNodeId),
+      10);
 }
 
 // Tests that AudioSetVolume forwards requests to CrasAudioHandler to set the

chrome/browser/chromeos/extensions/telemetry/api/management/fake_telemetry_management_service.cc

@@ -34,7 +34,7 @@ void FakeTelemetryManagementService::SetAudioGain(
     uint64_t node_id,
     int32_t gain,
     SetAudioGainCallback callback) {
-  std::move(callback).Run();
+  std::move(callback).Run(true);
 }
 
 void FakeTelemetryManagementService::SetAudioVolume(

chrome/browser/chromeos/extensions/telemetry/api/management/management_api.cc

@@ -72,8 +72,8 @@ void OsManagementSetAudioGainFunction::RunIfAllowed() {
                                    params.value().args.gain, std::move(cb));
 }
 
-void OsManagementSetAudioGainFunction::OnResult() {
-  Respond(NoArguments());
+void OsManagementSetAudioGainFunction::OnResult(bool is_success) {
+  Respond(WithArguments(is_success));
 }
 
 // OsManagementSetAudioVolumeFunction ------------------------------------------

chrome/browser/chromeos/extensions/telemetry/api/management/management_api.h

@@ -56,7 +56,7 @@ class OsManagementSetAudioGainFunction : public ManagementApiFunctionBase {
   // BaseTelemetryExtensionApiGuardFunction:
   void RunIfAllowed() override;
 
-  void OnResult();
+  void OnResult(bool is_success);
 };
 
 class OsManagementSetAudioVolumeFunction : public ManagementApiFunctionBase {

chrome/browser/chromeos/extensions/telemetry/api/management/management_api_browsertest.cc

@@ -88,10 +88,11 @@ IN_PROC_BROWSER_TEST_F(TelemetryExtensionManagementApiBrowserTest,
   CreateExtensionAndRunServiceWorker(R"(
     chrome.test.runTests([
       async function setAudioGain() {
-        await chrome.os.management.setAudioGain({
+        const result = await chrome.os.management.setAudioGain({
           nodeId: 1,
           gain: 100,
         });
+        chrome.test.assertTrue(result);
         chrome.test.succeed();
       }
     ]);

chrome/common/chromeos/extensions/api/management.idl

@@ -25,16 +25,13 @@ namespace os.management {
       boolean isMuted;
   };
 
-  callback VoidCallback = void();
-
   callback OperationCallback = void(boolean isSuccess);
 
   interface Functions {
-      // Sets the specified input audio device's gain to value. No-op if
+      // Sets the specified input audio device's gain to value. Returns false if
       // |args.nodeId| is invalid.
-      // TODO(b/317014106): Migrate to OperationCallback.
       [supportsPromises] static void setAudioGain(
-          SetAudioGainArguments args, VoidCallback callback);
+          SetAudioGainArguments args, OperationCallback callback);
       // Sets the specified output audio device's volume and mute state to the
       // given value. Returns false if |args.nodeId| is invalid.
       [supportsPromises] static void setAudioVolume(

chromeos/crosapi/mojom/telemetry_management_service.mojom

@@ -12,17 +12,20 @@ module crosapi.mojom;
 
 // Telemetry management interface exposed to Lacros.
 //
-// NextMinVersion: 2, NextIndex: 2
+// NextMinVersion: 3, NextIndex: 2
 [Stable, Uuid="562b12e3-8773-4085-9480-450e088ac252"]
 interface TelemetryManagementService {
-  // Sets the specified input audio device |node_id| gain to value |gain|. Will
-  // no-op if |node_id| is invalid.
-  // TODO(b/317014106): Support reporting whether the operation is successful.
+  // Sets the specified input audio device |node_id| gain to value |gain|.
   //
   // The request:
   // * |node_id| - Node id of the audio device to be configured.
   // * |gain| - Target gain percent in [0, 100]. Sets to 0 or 100 if outside.
-  SetAudioGain@0(uint64 node_id, int32 gain) => ();
+  //
+  // The response:
+  // * |is_success| - Whether the operation is successful. Returns false if
+  //                  |node_id| is invalid.
+  SetAudioGain@0(uint64 node_id, int32 gain)
+      => ([MinVersion=2] bool is_success);
 
   // Sets the specified output audio device |node_id| volume to value |volume|
   // and mute state to |is_muted|.

docs/telemetry_extension/api_overview.md

@@ -1100,5 +1100,5 @@ Source:
 
 | Function Name | Definition | Permission needed to access | Released in Chrome version | Description |
 ------------ | ------------- | ------------- | ------------- | ------------- |
-| setAudioGain | (args: SetAudioGainArguments) => Promise<void\> | `os.management.audio` | M122 | Sets the specified input audio device's gain to value. No-op if `args.nodeId` is invalid |
+| setAudioGain | (args: SetAudioGainArguments) => Promise<boolean\> | `os.management.audio` | M122 | Sets the specified input audio device's gain to value. Returns false if `args.nodeId` is invalid |
 | setAudioVolume | (args: SetAudioVolumeArguments) => Promise<boolean\> | `os.management.audio` | M122 | Sets the specified output audio device's volume and mute state to the given value. Returns false if `args.nodeId` is invalid |