Revert "Update VolumeController behavior for hardware keys"

This reverts commit 9d157bc0.

Reason for revert: New behavior is not good, see bug/thread.

Original change's description:
> Update VolumeController behavior for hardware keys
> 
> Changing behavior as requested, and updating tests:
> 1) The mute button should toggle mute (rename mojo function).
> 2) Volume up/down buttons should adjust up/down normally when muted.
> 3) Volume up/down buttons should also un-mute (if above default mute level).
> 4) Volume down will still mute if the level is below the default mute level.
> 
> Bug: 895550, 895552
> Change-Id: I353ab3124bb0375f109b96841cce6df3c9839388
> Reviewed-on: https://chromium-review.googlesource.com/c/1289990
> Commit-Queue: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#601191}

TBR=msw@chromium.org,tsepez@chromium.org,afakhry@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 895550, 895552
Change-Id: I6f4000a832bd0230f4da6a21e40b17b13b3823d2
Reviewed-on: https://chromium-review.googlesource.com/c/1306407Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603691}

ash/accelerators/accelerator_controller.cc

@@ -965,7 +965,7 @@ void HandleVolumeMute(mojom::VolumeController* volume_controller,
     base::RecordAction(UserMetricsAction("Accel_VolumeMute_F8"));
 
   if (volume_controller)
-    volume_controller->VolumeMuteToggle();
+    volume_controller->VolumeMute();
 }
 
 void HandleVolumeUp(mojom::VolumeController* volume_controller,

ash/public/interfaces/volume.mojom

@@ -11,10 +11,10 @@ module ash.mojom;
 // control interface or both ash and chrome should directly access the
 // CrasAudioHandler volume control functions.
 interface VolumeController {
-  // Toggle the audio volume between muted and un-muted.
-  VolumeMuteToggle();
-  // Decrease the audio volume, may also mute or un-mute the volume.
+  // Mute the audio volume.
+  VolumeMute();
+  // Decrease the audio volume.
   VolumeDown();
-  // Increase the audio volume, may also un-mute the volume.
+  // Increase the audio volume.
   VolumeUp();
 };

chrome/browser/ui/ash/volume_controller.cc

@@ -58,28 +58,37 @@ VolumeController::VolumeController() : binding_(this) {
   }
 }
 
-VolumeController::~VolumeController() = default;
+VolumeController::~VolumeController() {}
 
-void VolumeController::VolumeMuteToggle() {
-  chromeos::CrasAudioHandler* audio = chromeos::CrasAudioHandler::Get();
-  audio->SetOutputMute(!audio->IsOutputMuted());
+void VolumeController::VolumeMute() {
+  chromeos::CrasAudioHandler::Get()->SetOutputMute(true);
 }
 
 void VolumeController::VolumeDown() {
-  chromeos::CrasAudioHandler* audio = chromeos::CrasAudioHandler::Get();
-  audio->AdjustOutputVolumeByPercent(-kStepPercentage);
-  audio->SetOutputMute(audio->IsOutputVolumeBelowDefaultMuteLevel());
-  if (!audio->IsOutputMuted())
-    PlayVolumeAdjustSound();
+  chromeos::CrasAudioHandler* audio_handler = chromeos::CrasAudioHandler::Get();
+  if (audio_handler->IsOutputMuted()) {
+    audio_handler->SetOutputVolumePercent(0);
+  } else {
+    audio_handler->AdjustOutputVolumeByPercent(-kStepPercentage);
+    if (audio_handler->IsOutputVolumeBelowDefaultMuteLevel())
+      audio_handler->SetOutputMute(true);
+    else
+      PlayVolumeAdjustSound();
+  }
 }
 
 void VolumeController::VolumeUp() {
-  chromeos::CrasAudioHandler* audio = chromeos::CrasAudioHandler::Get();
-  const bool play_sound = audio->GetOutputVolumePercent() != 100;
-  audio->AdjustOutputVolumeByPercent(kStepPercentage);
-  if (audio->IsOutputVolumeBelowDefaultMuteLevel())
-    audio->AdjustOutputVolumeToAudibleLevel();
-  audio->SetOutputMute(false);
+  chromeos::CrasAudioHandler* audio_handler = chromeos::CrasAudioHandler::Get();
+  bool play_sound = false;
+  if (audio_handler->IsOutputMuted()) {
+    audio_handler->SetOutputMute(false);
+    audio_handler->AdjustOutputVolumeToAudibleLevel();
+    play_sound = true;
+  } else {
+    play_sound = audio_handler->GetOutputVolumePercent() != 100;
+    audio_handler->AdjustOutputVolumeByPercent(kStepPercentage);
+  }
+
   if (play_sound)
     PlayVolumeAdjustSound();
 }

chrome/browser/ui/ash/volume_controller.h

@@ -18,7 +18,7 @@ class VolumeController : public ash::mojom::VolumeController {
   ~VolumeController() override;
 
   // Overridden from ash::mojom::VolumeClient:
-  void VolumeMuteToggle() override;
+  void VolumeMute() override;
   void VolumeDown() override;
   void VolumeUp() override;
 

chrome/browser/ui/ash/volume_controller_browsertest.cc

@@ -25,7 +25,7 @@ class SoundsManagerTestImpl : public media::SoundsManager {
       : is_sound_initialized_(chromeos::SOUND_COUNT),
         num_play_requests_(chromeos::SOUND_COUNT) {}
 
-  ~SoundsManagerTestImpl() override = default;
+  ~SoundsManagerTestImpl() override {}
 
   bool Initialize(SoundKey key, const base::StringPiece& /* data */) override {
     is_sound_initialized_[key] = true;
@@ -58,11 +58,11 @@ class SoundsManagerTestImpl : public media::SoundsManager {
 
 class VolumeControllerTest : public InProcessBrowserTest {
  public:
-  VolumeControllerTest() = default;
-  ~VolumeControllerTest() override = default;
+  VolumeControllerTest() {}
+  ~VolumeControllerTest() override {}
 
   void SetUpOnMainThread() override {
-    volume_controller_ = std::make_unique<VolumeController>();
+    volume_controller_.reset(new VolumeController());
     audio_handler_ = chromeos::CrasAudioHandler::Get();
   }
 
@@ -75,7 +75,7 @@ class VolumeControllerTest : public InProcessBrowserTest {
 };
 
 IN_PROC_BROWSER_TEST_F(VolumeControllerTest, VolumeUpAndDown) {
-  // Set initial value as 50%.
+  // Set initial value as 50%
   const int kInitVolume = 50;
   audio_handler_->SetOutputVolumePercent(kInitVolume);
 
@@ -102,7 +102,7 @@ IN_PROC_BROWSER_TEST_F(VolumeControllerTest, VolumeDownToZero) {
 }
 
 IN_PROC_BROWSER_TEST_F(VolumeControllerTest, VolumeUpTo100) {
-  // Setting to almost max.
+  // Setting to almost max
   audio_handler_->SetOutputVolumePercent(99);
 
   volume_controller_->VolumeUp();
@@ -113,50 +113,39 @@ IN_PROC_BROWSER_TEST_F(VolumeControllerTest, VolumeUpTo100) {
   EXPECT_GT(100, audio_handler_->GetOutputVolumePercent());
 }
 
-IN_PROC_BROWSER_TEST_F(VolumeControllerTest, Mute) {
+IN_PROC_BROWSER_TEST_F(VolumeControllerTest, Mutes) {
   ASSERT_FALSE(audio_handler_->IsOutputMuted());
   const int initial_volume = audio_handler_->GetOutputVolumePercent();
 
-  // Toggling mute should work without changing the volume level.
-  volume_controller_->VolumeMuteToggle();
+  volume_controller_->VolumeMute();
   EXPECT_TRUE(audio_handler_->IsOutputMuted());
-  EXPECT_EQ(initial_volume, audio_handler_->GetOutputVolumePercent());
-  volume_controller_->VolumeMuteToggle();
-  EXPECT_FALSE(audio_handler_->IsOutputMuted());
-  EXPECT_EQ(initial_volume, audio_handler_->GetOutputVolumePercent());
 
-  // Volume up should un-mute and raise the volume.
-  volume_controller_->VolumeMuteToggle();
+  // Further mute buttons doesn't have effects.
+  volume_controller_->VolumeMute();
   EXPECT_TRUE(audio_handler_->IsOutputMuted());
-  volume_controller_->VolumeUp();
-  EXPECT_FALSE(audio_handler_->IsOutputMuted());
-  EXPECT_LT(initial_volume, audio_handler_->GetOutputVolumePercent());
 
-  // Volume down should un-mute and lower the volume.
-  audio_handler_->SetOutputMute(true);
-  volume_controller_->VolumeDown();
+  // Right after the volume up after set_mute recovers to original volume.
+  volume_controller_->VolumeUp();
   EXPECT_FALSE(audio_handler_->IsOutputMuted());
   EXPECT_EQ(initial_volume, audio_handler_->GetOutputVolumePercent());
 
-  // Volume down will mute if the level falls below the default mute volume (1).
-  audio_handler_->SetOutputVolumePercent(2);
-  audio_handler_->SetOutputMute(false);
+  volume_controller_->VolumeMute();
+  // After the volume down, the volume goes down to zero explicitly.
   volume_controller_->VolumeDown();
   EXPECT_TRUE(audio_handler_->IsOutputMuted());
-  EXPECT_TRUE(audio_handler_->IsOutputVolumeBelowDefaultMuteLevel());
+  EXPECT_EQ(0, audio_handler_->GetOutputVolumePercent());
 
-  // Volume up will un-mute and bring the level above the default mute volume.
-  audio_handler_->SetOutputVolumePercent(0);
-  audio_handler_->SetOutputMute(true);
+  // Thus, further VolumeUp doesn't recover the volume, it's just slightly
+  // bigger than 0.
   volume_controller_->VolumeUp();
-  EXPECT_FALSE(audio_handler_->IsOutputMuted());
-  EXPECT_FALSE(audio_handler_->IsOutputVolumeBelowDefaultMuteLevel());
+  EXPECT_LT(0, audio_handler_->GetOutputVolumePercent());
+  EXPECT_GT(initial_volume, audio_handler_->GetOutputVolumePercent());
 }
 
 class VolumeControllerSoundsTest : public VolumeControllerTest {
  public:
-  VolumeControllerSoundsTest() = default;
-  ~VolumeControllerSoundsTest() override = default;
+  VolumeControllerSoundsTest() : sounds_manager_(NULL) {}
+  ~VolumeControllerSoundsTest() override {}
 
   void SetUpInProcessBrowserTestFixture() override {
     sounds_manager_ = new SoundsManagerTestImpl();
@@ -172,7 +161,7 @@ class VolumeControllerSoundsTest : public VolumeControllerTest {
   }
 
  private:
-  SoundsManagerTestImpl* sounds_manager_ = nullptr;
+  SoundsManagerTestImpl* sounds_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(VolumeControllerSoundsTest);
 };
@@ -210,11 +199,9 @@ IN_PROC_BROWSER_TEST_F(VolumeControllerSoundsTest, EdgeCases) {
   volume_controller_->VolumeUp();
   EXPECT_EQ(3, num_play_requests());
 
-  // Check that sound isn't played when audio is muted. Note that the volume
-  // will be un-muted if the volume up/down key is pressed and the resulting
-  // level is above the default mute level (1), so press down with a low level.
-  audio_handler_->SetOutputVolumePercent(1);
-  audio_handler_->SetOutputMute(true);
+  // Check that sound isn't played when audio is muted.
+  audio_handler_->SetOutputVolumePercent(50);
+  volume_controller_->VolumeMute();
   volume_controller_->VolumeDown();
   ASSERT_TRUE(audio_handler_->IsOutputMuted());
   EXPECT_EQ(3, num_play_requests());
@@ -227,8 +214,8 @@ IN_PROC_BROWSER_TEST_F(VolumeControllerSoundsTest, EdgeCases) {
 
 class VolumeControllerSoundsDisabledTest : public VolumeControllerSoundsTest {
  public:
-  VolumeControllerSoundsDisabledTest() = default;
-  ~VolumeControllerSoundsDisabledTest() override = default;
+  VolumeControllerSoundsDisabledTest() {}
+  ~VolumeControllerSoundsDisabledTest() override {}
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     VolumeControllerSoundsTest::SetUpCommandLine(command_line);