Make PowerButton+VolumeUp can also take screenshot.

Bug: 937907
Change-Id: I2cd5baccd1f5acef58b051937447794eb49b7338
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1607183
Commit-Queue: Min Chen <minch@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659326}

ash/system/power/power_button_screenshot_controller.cc

@@ -50,6 +50,7 @@ bool PowerButtonScreenshotController::OnPowerButtonEvent(
   power_button_pressed_ = down;
   if (power_button_pressed_) {
     volume_down_timer_.Stop();
+    volume_up_timer_.Stop();
     power_button_pressed_time_ = tick_clock_->NowTicks();
     if (InterceptScreenshotChord())
       return true;
@@ -71,81 +72,104 @@ void PowerButtonScreenshotController::OnKeyEvent(ui::KeyEvent* event) {
   if (key_code != ui::VKEY_VOLUME_DOWN && key_code != ui::VKEY_VOLUME_UP)
     return;
 
-  if (key_code == ui::VKEY_VOLUME_DOWN) {
-    if (event->type() == ui::ET_KEY_PRESSED) {
-      if (!volume_down_key_pressed_) {
+  const bool is_volume_down = key_code == ui::VKEY_VOLUME_DOWN;
+  if (event->type() == ui::ET_KEY_PRESSED) {
+    if (!volume_down_key_pressed_ && !volume_up_key_pressed_) {
+      if (is_volume_down) {
         volume_down_key_pressed_ = true;
         volume_down_key_pressed_time_ = tick_clock_->NowTicks();
         consume_volume_down_ = false;
-
+        InterceptScreenshotChord();
+      } else {
+        volume_up_key_pressed_ = true;
+        volume_up_key_pressed_time_ = tick_clock_->NowTicks();
+        consume_volume_up_ = false;
         InterceptScreenshotChord();
       }
-
-      // Do not propagate volume down key pressed event if the first
-      // one is consumed by screenshot.
-      if (consume_volume_down_)
-        event->StopPropagation();
-    } else {
-      volume_down_key_pressed_ = false;
     }
-  }
 
-  if (key_code == ui::VKEY_VOLUME_UP)
-    volume_up_key_pressed_ = event->type() == ui::ET_KEY_PRESSED;
+    if (consume_volume_down_ || consume_volume_up_)
+      event->StopPropagation();
+  } else {
+    is_volume_down ? volume_down_key_pressed_ = false
+                   : volume_up_key_pressed_ = false;
+  }
 
   // When volume key is pressed, cancel the ongoing power button behavior.
   if (volume_down_key_pressed_ || volume_up_key_pressed_)
     Shell::Get()->power_button_controller()->CancelPowerButtonEvent();
 
-  // On volume down key pressed while power button not pressed yet state, do not
-  // propagate volume down key pressed event for chord delay time. Start the
-  // timer to wait power button pressed for screenshot operation, and on timeout
-  // perform the delayed volume down operation.
-  if (volume_down_key_pressed_ && !power_button_pressed_) {
-    base::TimeTicks now = tick_clock_->NowTicks();
-    if (now <= volume_down_key_pressed_time_ + kScreenshotChordDelay) {
-      event->StopPropagation();
+  // On volume down/up key pressed while power button not pressed yet state, do
+  // not propagate volume down/up key pressed event for chord delay time. Start
+  // the timer to wait power button pressed for screenshot operation, and on
+  // timeout perform the delayed volume down/up operation.
+  if (power_button_pressed_)
+    return;
 
-      if (!volume_down_timer_.IsRunning()) {
-        volume_down_timer_.Start(
-            FROM_HERE, kScreenshotChordDelay,
-            base::BindOnce(
-                &PowerButtonScreenshotController::OnVolumeDownTimeout,
-                base::Unretained(this), ui::Accelerator(*event)));
-      }
+  base::TimeTicks now = tick_clock_->NowTicks();
+  if (volume_down_key_pressed_ && is_volume_down &&
+      now <= volume_down_key_pressed_time_ + kScreenshotChordDelay) {
+    event->StopPropagation();
+    if (!volume_down_timer_.IsRunning()) {
+      volume_down_timer_.Start(
+          FROM_HERE, kScreenshotChordDelay,
+          base::BindOnce(
+              &PowerButtonScreenshotController::OnVolumeControlTimeout,
+              base::Unretained(this), ui::Accelerator(*event), /*down=*/true));
+    }
+  } else if (volume_up_key_pressed_ && !is_volume_down &&
+             now <= volume_up_key_pressed_time_ + kScreenshotChordDelay) {
+    event->StopPropagation();
+    if (!volume_up_timer_.IsRunning()) {
+      volume_up_timer_.Start(
+          FROM_HERE, kScreenshotChordDelay,
+          base::BindOnce(
+              &PowerButtonScreenshotController::OnVolumeControlTimeout,
+              base::Unretained(this), ui::Accelerator(*event), /*down=*/false));
     }
   }
 }
 
 bool PowerButtonScreenshotController::InterceptScreenshotChord() {
-  if (volume_down_key_pressed_ && power_button_pressed_) {
-    // Record the delay when power button and volume down key are both pressed,
-    // which indicates user might want to use accelerator to take screenshot.
-    // This will help us determine the best chord delay among metrics.
-    const base::TimeDelta key_pressed_delay =
-        power_button_pressed_time_ - volume_down_key_pressed_time_;
-    UMA_HISTOGRAM_TIMES("Ash.PowerButtonScreenshot.DelayBetweenAccelKeyPressed",
-                        key_pressed_delay.magnitude());
-
-    base::TimeTicks now = tick_clock_->NowTicks();
-    if (now <= volume_down_key_pressed_time_ + kScreenshotChordDelay &&
-        now <= power_button_pressed_time_ + kScreenshotChordDelay) {
-      Shell::Get()->accelerator_controller()->PerformActionIfEnabled(
-          TAKE_SCREENSHOT, {});
-      consume_volume_down_ = true;
-
-      base::RecordAction(
-          base::UserMetricsAction("Accel_PowerButton_Screenshot"));
-      return true;
-    }
+  if (!power_button_pressed_ ||
+      (!volume_down_key_pressed_ && !volume_up_key_pressed_)) {
+    return false;
+  }
+
+  // Record the delay when power button and volume down/up key are both pressed,
+  // which indicates user might want to use accelerator to take screenshot.
+  // This will help us determine the best chord delay among metrics.
+  const base::TimeDelta key_pressed_delay =
+      volume_down_key_pressed_
+          ? power_button_pressed_time_ - volume_down_key_pressed_time_
+          : power_button_pressed_time_ - volume_up_key_pressed_time_;
+  UMA_HISTOGRAM_TIMES("Ash.PowerButtonScreenshot.DelayBetweenAccelKeyPressed",
+                      key_pressed_delay.magnitude());
+
+  base::TimeTicks now = tick_clock_->NowTicks();
+  if (now > power_button_pressed_time_ + kScreenshotChordDelay)
+    return false;
+
+  consume_volume_down_ =
+      volume_down_key_pressed_ &&
+      now <= volume_down_key_pressed_time_ + kScreenshotChordDelay;
+  consume_volume_up_ =
+      volume_up_key_pressed_ &&
+      now <= volume_up_key_pressed_time_ + kScreenshotChordDelay;
+  if (consume_volume_down_ || consume_volume_up_) {
+    Shell::Get()->accelerator_controller()->PerformActionIfEnabled(
+        TAKE_SCREENSHOT, {});
+
+    base::RecordAction(base::UserMetricsAction("Accel_PowerButton_Screenshot"));
   }
-  return false;
+  return consume_volume_down_ || consume_volume_up_;
 }
 
-void PowerButtonScreenshotController::OnVolumeDownTimeout(
-    const ui::Accelerator& accelerator) {
-  Shell::Get()->accelerator_controller()->PerformActionIfEnabled(VOLUME_DOWN,
-                                                                 accelerator);
+void PowerButtonScreenshotController::OnVolumeControlTimeout(
+    const ui::Accelerator& accelerator,
+    bool down) {
+  Shell::Get()->accelerator_controller()->PerformActionIfEnabled(
+      down ? VOLUME_DOWN : VOLUME_UP, accelerator);
 }
 
 }  // namespace ash

ash/system/power/power_button_screenshot_controller.h

@@ -47,31 +47,33 @@ class ASH_EXPORT PowerButtonScreenshotController : public ui::EventHandler {
   // indicate that power button and volume down key is consumed by screenshot.
   bool InterceptScreenshotChord();
 
-  // Called by |volume_down_timer_| to perform volume down accelerator.
-  void OnVolumeDownTimeout(const ui::Accelerator& accelerator);
+  // Called by |volume_down_timer_| or |volume_up_timer_| to perform volume down
+  // or up accelerator.
+  void OnVolumeControlTimeout(const ui::Accelerator& accelerator, bool down);
 
-  // True if volume down key is pressed.
+  // True if volume down/up key is pressed.
   bool volume_down_key_pressed_ = false;
-
-  // True if volume up key is pressed.
   bool volume_up_key_pressed_ = false;
 
-  // True if volume down key is consumed by screenshot accelerator.
+  // True if volume down/up key is consumed by screenshot accelerator.
   bool consume_volume_down_ = false;
+  bool consume_volume_up_ = false;
 
   // True if power button is pressed.
   bool power_button_pressed_ = false;
 
-  // Saves the most recent volume down key pressed time.
+  // Saves the most recent volume down/up key pressed time.
   base::TimeTicks volume_down_key_pressed_time_;
+  base::TimeTicks volume_up_key_pressed_time_;
 
   // Saves the most recent power button pressed time.
   base::TimeTicks power_button_pressed_time_;
 
-  // Started when volume down key is pressed and power button is not pressed.
-  // Stopped when power button is pressed. Runs OnVolumeDownTimeout to perform a
-  // volume down accelerator.
+  // Started when volume down/up key is pressed and power button is not pressed.
+  // Stopped when power button is pressed. Runs OnVolumeControlTimeout to
+  // perform a volume down/up accelerator.
   base::OneShotTimer volume_down_timer_;
+  base::OneShotTimer volume_up_timer_;
 
   // Time source for performed action times.
   const base::TickClock* tick_clock_;  // Not owned.

ash/system/power/power_button_screenshot_controller_test_api.cc

@@ -25,4 +25,12 @@ bool PowerButtonScreenshotControllerTestApi::TriggerVolumeDownTimer() {
   return true;
 }
 
+bool PowerButtonScreenshotControllerTestApi::TriggerVolumeUpTimer() {
+  if (!controller_->volume_up_timer_.IsRunning())
+    return false;
+
+  controller_->volume_up_timer_.FireNow();
+  return true;
+}
+
 }  // namespace ash

ash/system/power/power_button_screenshot_controller_test_api.h

@@ -24,6 +24,10 @@ class PowerButtonScreenshotControllerTestApi {
   // and returns true. Otherwise returns false.
   bool TriggerVolumeDownTimer() WARN_UNUSED_RESULT;
 
+  // If |controller_->volume_up_timer_| is running, stops it, runs its task,
+  // and returns true. Otherwise returns false.
+  bool TriggerVolumeUpTimer() WARN_UNUSED_RESULT;
+
  private:
   PowerButtonScreenshotController* controller_;