Commit 93132287 authored by Mitsuru Oshima's avatar Mitsuru Oshima Committed by Commit Bot

Allow pwa/chrome app to consume system key in tablet mode

* Screenshot should still work if the power button is pressed first.

Bug: 1024118
Test: Covered by unittests. Manually tested on Slate.
Change-Id: I14c07d0f8419b5755776346d2c5f5e2e156861c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2130638
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: default avatarKuo Jen Wei <inker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758723}
parent d0ca7b09
......@@ -5,9 +5,11 @@
#include "ash/system/power/power_button_screenshot_controller.h"
#include "ash/accelerators/accelerator_controller_impl.h"
#include "ash/public/cpp/window_properties.h"
#include "ash/shell.h"
#include "ash/system/power/power_button_controller.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "ash/wm/window_util.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h"
#include "base/time/tick_clock.h"
......@@ -21,6 +23,11 @@ bool IsTabletMode() {
return Shell::Get()->tablet_mode_controller()->InTabletMode();
}
bool VolumeKeyMaybeUsedByApp() {
aura::Window* active = window_util::GetActiveWindow();
return active && active->GetProperty(ash::kCanConsumeSystemKeysKey);
}
} // namespace
constexpr base::TimeDelta
......@@ -70,9 +77,21 @@ void PowerButtonScreenshotController::OnKeyEvent(ui::KeyEvent* event) {
if (key_code != ui::VKEY_VOLUME_DOWN && key_code != ui::VKEY_VOLUME_UP)
return;
bool did_consume_volume_keys =
volume_down_key_pressed_ || volume_up_key_pressed_;
bool volume_key_maybe_used_by_app = VolumeKeyMaybeUsedByApp();
// Even if the app is requesting to consume volume key, do not give it if
// 1) power button is already pressed. This should trigger screenshot.
// 2) if this is already handling volume key. We need to continue processing
// volume eve after power button is released, until volume keys are released.
if (volume_key_maybe_used_by_app && !power_button_pressed_ &&
!did_consume_volume_keys) {
return;
}
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 (!did_consume_volume_keys) {
if (is_volume_down) {
volume_down_key_pressed_ = true;
volume_down_key_pressed_time_ = tick_clock_->NowTicks();
......@@ -86,8 +105,12 @@ void PowerButtonScreenshotController::OnKeyEvent(ui::KeyEvent* event) {
}
}
if (consume_volume_down_ || consume_volume_up_)
// Do no pass the event to the app if the events are being
// processed
if (consume_volume_down_ || consume_volume_up_ ||
volume_key_maybe_used_by_app) {
event->StopPropagation();
}
} else {
is_volume_down ? volume_down_key_pressed_ = false
: volume_up_key_pressed_ = false;
......
......@@ -7,6 +7,7 @@
#include <memory>
#include "ash/login_status.h"
#include "ash/public/cpp/window_properties.h"
#include "ash/shell.h"
#include "ash/system/power/power_button_controller.h"
#include "ash/system/power/power_button_controller_test_api.h"
......@@ -16,10 +17,37 @@
#include "ash/wm/lock_state_controller_test_api.h"
#include "base/test/simple_test_tick_clock.h"
#include "chromeos/dbus/power/fake_power_manager_client.h"
#include "ui/aura/test/test_window_delegate.h"
#include "ui/events/event.h"
#include "ui/wm/core/window_util.h"
namespace ash {
namespace {
class KeyEventWindowDelegate : public aura::test::TestWindowDelegate {
public:
KeyEventWindowDelegate() = default;
~KeyEventWindowDelegate() override = default;
KeyEventWindowDelegate(const KeyEventWindowDelegate&) = delete;
KeyEventWindowDelegate& operator=(const KeyEventWindowDelegate&) = delete;
void OnKeyEvent(ui::KeyEvent* event) override {
key_code_ = event->key_code();
}
ui::KeyboardCode GetReceivedKeyCodeAndReset() {
ui::KeyboardCode tmp = key_code_;
key_code_ = ui::VKEY_UNKNOWN;
return tmp;
}
private:
ui::KeyboardCode key_code_ = ui::VKEY_UNKNOWN;
};
} // namespace
// Test fixture used for testing power button screenshot behavior under tablet
// power button.
class PowerButtonScreenshotControllerTest : public PowerButtonTestBase {
......@@ -85,9 +113,34 @@ class PowerButtonScreenshotControllerTest : public PowerButtonTestBase {
DISALLOW_COPY_AND_ASSIGN(PowerButtonScreenshotControllerTest);
};
class PowerButtonScreenshotControllerWithSystemKeysTest
: public PowerButtonScreenshotControllerTest,
public ::testing::WithParamInterface<bool> {
public:
PowerButtonScreenshotControllerWithSystemKeysTest() = default;
~PowerButtonScreenshotControllerWithSystemKeysTest() override = default;
PowerButtonScreenshotControllerWithSystemKeysTest(
const PowerButtonScreenshotControllerWithSystemKeysTest&) = delete;
PowerButtonScreenshotControllerWithSystemKeysTest& operator=(
const PowerButtonScreenshotControllerWithSystemKeysTest&) = delete;
void SetUp() override {
PowerButtonScreenshotControllerTest::SetUp();
if (GetParam()) {
aura::Window* window =
CreateTestWindowInShellWithDelegate(&delegate_, 1, gfx::Rect());
window->SetProperty(ash::kCanConsumeSystemKeysKey, true);
}
}
private:
KeyEventWindowDelegate delegate_;
};
// Tests the functionalities that press the power button first and then press
// volume down and volume up key alternative.
TEST_F(PowerButtonScreenshotControllerTest,
TEST_P(PowerButtonScreenshotControllerWithSystemKeysTest,
PowerButtonPressedFirst_Screenshot) {
PressPowerButton();
tick_clock_.Advance(PowerButtonScreenshotController::kScreenshotChordDelay -
......@@ -135,6 +188,10 @@ TEST_F(PowerButtonScreenshotControllerTest,
EXPECT_FALSE(LastKeyConsumed());
}
INSTANTIATE_TEST_SUITE_P(All,
PowerButtonScreenshotControllerWithSystemKeysTest,
testing::Bool());
// Tests the functionalities that press the volume key first and then press
// volume down and volume up key alternative.
TEST_F(PowerButtonScreenshotControllerTest, VolumeKeyPressedFirst_Screenshot) {
......@@ -186,6 +243,56 @@ TEST_F(PowerButtonScreenshotControllerTest, VolumeKeyPressedFirst_Screenshot) {
EXPECT_FALSE(LastKeyConsumed());
}
// If the window with kConsumeSystemKeysKey property is active in tablet mode,
// volume keys will be passed to the window if they are pressed first.
TEST_F(PowerButtonScreenshotControllerTest, WindowWithSystemKeys) {
EnableTabletMode(true);
KeyEventWindowDelegate delegate;
std::unique_ptr<aura::Window> window = base::WrapUnique(
CreateTestWindowInShellWithDelegate(&delegate, 1, gfx::Rect()));
window->SetProperty(ash::kCanConsumeSystemKeysKey, true);
::wm::ActivateWindow(window.get());
// Tests when volume up pressed first, it's consumed by an app.
// screenshot chord.
GetEventGenerator()->PressKey(ui::VKEY_VOLUME_UP, ui::EF_NONE);
EXPECT_EQ(ui::VKEY_VOLUME_UP, delegate.GetReceivedKeyCodeAndReset());
GetEventGenerator()->PressKey(ui::VKEY_VOLUME_UP, ui::EF_NONE);
EXPECT_EQ(ui::VKEY_VOLUME_UP, delegate.GetReceivedKeyCodeAndReset());
EXPECT_EQ(0, GetScreenshotCount());
// Tests when volume down pressed first, it's consumed by an app.
// screenshot chord.
GetEventGenerator()->PressKey(ui::VKEY_VOLUME_DOWN, ui::EF_NONE);
EXPECT_EQ(ui::VKEY_VOLUME_DOWN, delegate.GetReceivedKeyCodeAndReset());
GetEventGenerator()->PressKey(ui::VKEY_VOLUME_DOWN, ui::EF_NONE);
EXPECT_EQ(ui::VKEY_VOLUME_DOWN, delegate.GetReceivedKeyCodeAndReset());
EXPECT_EQ(0, GetScreenshotCount());
// Delete the window, and volume will be consumed by shortcut.
// Screenshot using up.
window.reset();
GetEventGenerator()->PressKey(ui::VKEY_VOLUME_UP, ui::EF_NONE);
EXPECT_EQ(ui::VKEY_UNKNOWN, delegate.GetReceivedKeyCodeAndReset());
PressPowerButton();
ReleasePowerButton();
GetEventGenerator()->ReleaseKey(ui::VKEY_VOLUME_UP, ui::EF_NONE);
EXPECT_EQ(ui::VKEY_UNKNOWN, delegate.GetReceivedKeyCodeAndReset());
EXPECT_EQ(1, GetScreenshotCount());
// Screenshot using down.
GetEventGenerator()->PressKey(ui::VKEY_VOLUME_DOWN, ui::EF_NONE);
EXPECT_EQ(ui::VKEY_UNKNOWN, delegate.GetReceivedKeyCodeAndReset());
PressPowerButton();
ReleasePowerButton();
GetEventGenerator()->ReleaseKey(ui::VKEY_VOLUME_DOWN, ui::EF_NONE);
EXPECT_EQ(ui::VKEY_UNKNOWN, delegate.GetReceivedKeyCodeAndReset());
EXPECT_EQ(2, GetScreenshotCount());
}
class PowerButtonScreenshotControllerWithKeyCodeTest
: public PowerButtonScreenshotControllerTest,
public testing::WithParamInterface<ui::KeyboardCode> {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment