Commit e3b5f80a authored by Ahmed Fakhry's avatar Ahmed Fakhry Committed by Chromium LUCI CQ

capture_mode: Enable feature by default

BUG=1168437
TEST=Manual

Change-Id: Ic9f719f660bea1f5d50c2ee77e0053e9186ec57b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2639800
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: default avatarSammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845809}
parent 469ff0a6
...@@ -1068,7 +1068,7 @@ TEST_F(AcceleratorControllerTest, GlobalAccelerators) { ...@@ -1068,7 +1068,7 @@ TEST_F(AcceleratorControllerTest, GlobalAccelerators) {
// The "Take Screenshot", "Take Partial Screenshot", volume, brightness, and // The "Take Screenshot", "Take Partial Screenshot", volume, brightness, and
// keyboard brightness accelerators are only defined on ChromeOS. // keyboard brightness accelerators are only defined on ChromeOS.
{ if (!features::IsCaptureModeEnabled()) {
TestScreenshotDelegate* delegate = GetScreenshotDelegate(); TestScreenshotDelegate* delegate = GetScreenshotDelegate();
delegate->set_can_take_screenshot(false); delegate->set_can_take_screenshot(false);
EXPECT_TRUE(ProcessInController( EXPECT_TRUE(ProcessInController(
...@@ -1734,7 +1734,13 @@ TEST_F(AcceleratorControllerTest, ToggleCapsLockAccelerators) { ...@@ -1734,7 +1734,13 @@ TEST_F(AcceleratorControllerTest, ToggleCapsLockAccelerators) {
generator->PressLeftButton(); generator->PressLeftButton();
generator->MoveMouseTo(10, 10); generator->MoveMouseTo(10, 10);
generator->ReleaseLeftButton(); generator->ReleaseLeftButton();
if (features::IsCaptureModeEnabled()) {
auto* controller = CaptureModeController::Get();
EXPECT_TRUE(controller->IsActive());
EXPECT_EQ(CaptureModeSource::kRegion, controller->source());
} else {
EXPECT_EQ(1, delegate->handle_take_partial_screenshot_count()); EXPECT_EQ(1, delegate->handle_take_partial_screenshot_count());
}
// Press Search, Press Alt, Release Search, Release Alt. CapsLock should be // Press Search, Press Alt, Release Search, Release Alt. CapsLock should be
// triggered. // triggered.
......
...@@ -9,6 +9,8 @@ ...@@ -9,6 +9,8 @@
#include "ash/accelerators/accelerator_controller_impl.h" #include "ash/accelerators/accelerator_controller_impl.h"
#include "ash/accelerators/pre_target_accelerator_handler.h" #include "ash/accelerators/pre_target_accelerator_handler.h"
#include "ash/app_list/test/app_list_test_helper.h" #include "ash/app_list/test/app_list_test_helper.h"
#include "ash/capture_mode/capture_mode_controller.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/session/session_controller_impl.h" #include "ash/session/session_controller_impl.h"
#include "ash/shell.h" #include "ash/shell.h"
#include "ash/test/ash_test_base.h" #include "ash/test/ash_test_base.h"
...@@ -34,12 +36,15 @@ TEST_F(AcceleratorFilterTest, TestFilterWithoutFocus) { ...@@ -34,12 +36,15 @@ TEST_F(AcceleratorFilterTest, TestFilterWithoutFocus) {
EXPECT_EQ(0, delegate->handle_take_screenshot_count()); EXPECT_EQ(0, delegate->handle_take_screenshot_count());
ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow()); ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
// VKEY_SNAPSHOT opens capture mode when the feature is enabled. Otherwise,
// AcceleratorController calls ScreenshotDelegate::HandleTakeScreenshot() when // AcceleratorController calls ScreenshotDelegate::HandleTakeScreenshot() when
// VKEY_SNAPSHOT is pressed. See kAcceleratorData[] in // VKEY_SNAPSHOT is pressed. See kAcceleratorData[] in
// accelerator_controller.cc. // accelerator_controller.cc.
generator.PressKey(ui::VKEY_SNAPSHOT, 0); generator.PressKey(ui::VKEY_SNAPSHOT, 0);
EXPECT_EQ(1, delegate->handle_take_screenshot_count());
generator.ReleaseKey(ui::VKEY_SNAPSHOT, 0); generator.ReleaseKey(ui::VKEY_SNAPSHOT, 0);
if (features::IsCaptureModeEnabled())
EXPECT_TRUE(CaptureModeController::Get()->IsActive());
else
EXPECT_EQ(1, delegate->handle_take_screenshot_count()); EXPECT_EQ(1, delegate->handle_take_screenshot_count());
} }
...@@ -57,8 +62,10 @@ TEST_F(AcceleratorFilterTest, TestFilterWithFocus) { ...@@ -57,8 +62,10 @@ TEST_F(AcceleratorFilterTest, TestFilterWithFocus) {
// not focused. // not focused.
ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow()); ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
generator.PressKey(ui::VKEY_SNAPSHOT, 0); generator.PressKey(ui::VKEY_SNAPSHOT, 0);
EXPECT_EQ(0, delegate->handle_take_screenshot_count());
generator.ReleaseKey(ui::VKEY_SNAPSHOT, 0); generator.ReleaseKey(ui::VKEY_SNAPSHOT, 0);
if (features::IsCaptureModeEnabled())
EXPECT_FALSE(CaptureModeController::Get()->IsActive());
else
EXPECT_EQ(0, delegate->handle_take_screenshot_count()); EXPECT_EQ(0, delegate->handle_take_screenshot_count());
// Reset window before |test_delegate| gets deleted. // Reset window before |test_delegate| gets deleted.
...@@ -72,15 +79,23 @@ TEST_F(AcceleratorFilterTest, TestCapsLockMask) { ...@@ -72,15 +79,23 @@ TEST_F(AcceleratorFilterTest, TestCapsLockMask) {
ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow()); ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
generator.PressKey(ui::VKEY_SNAPSHOT, 0); generator.PressKey(ui::VKEY_SNAPSHOT, 0);
EXPECT_EQ(1, delegate->handle_take_screenshot_count());
generator.ReleaseKey(ui::VKEY_SNAPSHOT, 0); generator.ReleaseKey(ui::VKEY_SNAPSHOT, 0);
const bool capture_mode_enabled = features::IsCaptureModeEnabled();
auto* controller = CaptureModeController::Get();
if (capture_mode_enabled) {
EXPECT_TRUE(controller->IsActive());
controller->Stop();
} else {
EXPECT_EQ(1, delegate->handle_take_screenshot_count()); EXPECT_EQ(1, delegate->handle_take_screenshot_count());
}
// Check if AcceleratorFilter ignores the mask for Caps Lock. Note that there // Check if AcceleratorFilter ignores the mask for Caps Lock. Note that there
// is no ui::EF_ mask for Num Lock. // is no ui::EF_ mask for Num Lock.
generator.PressKey(ui::VKEY_SNAPSHOT, ui::EF_CAPS_LOCK_ON); generator.PressKey(ui::VKEY_SNAPSHOT, ui::EF_CAPS_LOCK_ON);
EXPECT_EQ(2, delegate->handle_take_screenshot_count());
generator.ReleaseKey(ui::VKEY_SNAPSHOT, ui::EF_CAPS_LOCK_ON); generator.ReleaseKey(ui::VKEY_SNAPSHOT, ui::EF_CAPS_LOCK_ON);
if (capture_mode_enabled)
EXPECT_TRUE(controller->IsActive());
else
EXPECT_EQ(2, delegate->handle_take_screenshot_count()); EXPECT_EQ(2, delegate->handle_take_screenshot_count());
} }
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "ash/accelerators/accelerator_controller_impl.h" #include "ash/accelerators/accelerator_controller_impl.h"
#include "ash/app_list/test/app_list_test_helper.h" #include "ash/app_list/test/app_list_test_helper.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/shell.h" #include "ash/shell.h"
#include "ash/shell_observer.h" #include "ash/shell_observer.h"
#include "ash/system/network/network_observer.h" #include "ash/system/network/network_observer.h"
...@@ -136,6 +137,11 @@ TEST_F(AcceleratorTest, Basic) { ...@@ -136,6 +137,11 @@ TEST_F(AcceleratorTest, Basic) {
// Tests full screenshot accelerators. // Tests full screenshot accelerators.
TEST_F(AcceleratorTest, FullScreenshot) { TEST_F(AcceleratorTest, FullScreenshot) {
if (features::IsCaptureModeEnabled()) {
// Capture mode shortcuts and behavior are tested elsewhere.
return;
}
TestScreenshotDelegate* screenshot_delegate = GetScreenshotDelegate(); TestScreenshotDelegate* screenshot_delegate = GetScreenshotDelegate();
screenshot_delegate->set_can_take_screenshot(true); screenshot_delegate->set_can_take_screenshot(true);
EXPECT_EQ(0, screenshot_delegate->handle_take_screenshot_count()); EXPECT_EQ(0, screenshot_delegate->handle_take_screenshot_count());
...@@ -159,6 +165,11 @@ TEST_F(AcceleratorTest, FullScreenshot) { ...@@ -159,6 +165,11 @@ TEST_F(AcceleratorTest, FullScreenshot) {
// Tests partial screenshot accelerators. // Tests partial screenshot accelerators.
TEST_F(AcceleratorTest, PartialScreenshot) { TEST_F(AcceleratorTest, PartialScreenshot) {
if (features::IsCaptureModeEnabled()) {
// Capture mode shortcuts and behavior are tested elsewhere.
return;
}
ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow()); ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
TestScreenshotDelegate* screenshot_delegate = GetScreenshotDelegate(); TestScreenshotDelegate* screenshot_delegate = GetScreenshotDelegate();
screenshot_delegate->set_can_take_screenshot(true); screenshot_delegate->set_can_take_screenshot(true);
...@@ -212,6 +223,11 @@ TEST_F(AcceleratorTest, PartialScreenshot) { ...@@ -212,6 +223,11 @@ TEST_F(AcceleratorTest, PartialScreenshot) {
// Tests window screenshot accelerators. // Tests window screenshot accelerators.
TEST_F(AcceleratorTest, WindowScreenshot) { TEST_F(AcceleratorTest, WindowScreenshot) {
if (features::IsCaptureModeEnabled()) {
// Capture mode shortcuts and behavior are tested elsewhere.
return;
}
TestScreenshotDelegate* screenshot_delegate = GetScreenshotDelegate(); TestScreenshotDelegate* screenshot_delegate = GetScreenshotDelegate();
screenshot_delegate->set_can_take_screenshot(true); screenshot_delegate->set_can_take_screenshot(true);
......
...@@ -27,8 +27,11 @@ FeaturePodButton* CaptureModeFeaturePodController::CreateButton() { ...@@ -27,8 +27,11 @@ FeaturePodButton* CaptureModeFeaturePodController::CreateButton() {
DCHECK(!button_); DCHECK(!button_);
button_ = new FeaturePodButton(this, /*is_togglable=*/false); button_ = new FeaturePodButton(this, /*is_togglable=*/false);
button_->SetVectorIcon(kCaptureModeIcon); button_->SetVectorIcon(kCaptureModeIcon);
button_->SetLabel( const auto label_text =
l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_CAPTURE_MODE_BUTTON_LABEL)); l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_CAPTURE_MODE_BUTTON_LABEL);
button_->SetLabel(label_text);
button_->icon_button()->SetTooltipText(label_text);
button_->SetLabelTooltip(label_text);
button_->SetVisible( button_->SetVisible(
!Shell::Get()->session_controller()->IsUserSessionBlocked()); !Shell::Get()->session_controller()->IsUserSessionBlocked());
return button_; return button_;
......
...@@ -21,7 +21,7 @@ const base::Feature kAutoNightLight{"AutoNightLight", ...@@ -21,7 +21,7 @@ const base::Feature kAutoNightLight{"AutoNightLight",
const base::Feature kBento{"Bento", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kBento{"Bento", base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kCaptureMode{"CaptureMode", const base::Feature kCaptureMode{"CaptureMode",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kContextualNudges{"ContextualNudges", const base::Feature kContextualNudges{"ContextualNudges",
base::FEATURE_ENABLED_BY_DEFAULT}; base::FEATURE_ENABLED_BY_DEFAULT};
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "ash/assistant/util/assistant_util.h" #include "ash/assistant/util/assistant_util.h"
#include "ash/highlighter/highlighter_controller.h" #include "ash/highlighter/highlighter_controller.h"
#include "ash/highlighter/highlighter_controller_test_api.h" #include "ash/highlighter/highlighter_controller_test_api.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/ash_pref_names.h" #include "ash/public/cpp/ash_pref_names.h"
#include "ash/public/cpp/ash_switches.h" #include "ash/public/cpp/ash_switches.h"
#include "ash/public/cpp/assistant/assistant_state.h" #include "ash/public/cpp/assistant/assistant_state.h"
...@@ -161,10 +162,12 @@ TEST_F(PaletteTrayTest, PaletteTrayWorkflow) { ...@@ -161,10 +162,12 @@ TEST_F(PaletteTrayTest, PaletteTrayWorkflow) {
// the palette tray button is will not be active. // the palette tray button is will not be active.
PerformTap(); PerformTap();
ASSERT_TRUE(test_api_->tray_bubble_wrapper()); ASSERT_TRUE(test_api_->tray_bubble_wrapper());
test_api_->palette_tool_manager()->ActivateTool( const auto capture_tool_id = features::IsCaptureModeEnabled()
PaletteToolId::CAPTURE_SCREEN); ? PaletteToolId::ENTER_CAPTURE_MODE
EXPECT_FALSE(test_api_->palette_tool_manager()->IsToolActive( : PaletteToolId::CAPTURE_SCREEN;
PaletteToolId::CAPTURE_SCREEN)); test_api_->palette_tool_manager()->ActivateTool(capture_tool_id);
EXPECT_FALSE(
test_api_->palette_tool_manager()->IsToolActive(capture_tool_id));
// Wait for the tray bubble widget to close. // Wait for the tray bubble widget to close.
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_FALSE(test_api_->tray_bubble_wrapper()); EXPECT_FALSE(test_api_->tray_bubble_wrapper());
......
...@@ -156,9 +156,13 @@ void PowerButtonMenuScreenView::OnWidgetShown( ...@@ -156,9 +156,13 @@ void PowerButtonMenuScreenView::OnWidgetShown(
double offset_percentage) { double offset_percentage) {
power_button_position_ = position; power_button_position_ = position;
power_button_offset_percentage_ = offset_percentage; power_button_offset_percentage_ = offset_percentage;
// The order here matters. RecreateItems() must be called before calling
// UpdateMenuBoundsOrigins(), since the latter relies on the
// power_button_menu_view_'s preferred size, which depends on the items added
// to the view.
power_button_menu_view_->RecreateItems();
if (power_button_position_ != PowerButtonPosition::NONE) if (power_button_position_ != PowerButtonPosition::NONE)
UpdateMenuBoundsOrigins(); UpdateMenuBoundsOrigins();
power_button_menu_view_->RecreateItems();
Layout(); Layout();
} }
......
...@@ -6,7 +6,9 @@ ...@@ -6,7 +6,9 @@
#include <memory> #include <memory>
#include "ash/capture_mode/capture_mode_metrics.h"
#include "ash/login_status.h" #include "ash/login_status.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/window_properties.h" #include "ash/public/cpp/window_properties.h"
#include "ash/shell.h" #include "ash/shell.h"
#include "ash/system/power/power_button_controller.h" #include "ash/system/power/power_button_controller.h"
...@@ -15,6 +17,8 @@ ...@@ -15,6 +17,8 @@
#include "ash/system/power/power_button_test_base.h" #include "ash/system/power/power_button_test_base.h"
#include "ash/test_screenshot_delegate.h" #include "ash/test_screenshot_delegate.h"
#include "ash/wm/lock_state_controller_test_api.h" #include "ash/wm/lock_state_controller_test_api.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/simple_test_tick_clock.h" #include "base/test/simple_test_tick_clock.h"
#include "chromeos/dbus/power/fake_power_manager_client.h" #include "chromeos/dbus/power/fake_power_manager_client.h"
#include "ui/aura/test/test_window_delegate.h" #include "ui/aura/test/test_window_delegate.h"
...@@ -69,6 +73,8 @@ class PowerButtonScreenshotControllerTest : public PowerButtonTestBase { ...@@ -69,6 +73,8 @@ class PowerButtonScreenshotControllerTest : public PowerButtonTestBase {
tick_clock_.Advance( tick_clock_.Advance(
PowerButtonController::kIgnorePowerButtonAfterResumeDelay + PowerButtonController::kIgnorePowerButtonAfterResumeDelay +
base::TimeDelta::FromMilliseconds(2)); base::TimeDelta::FromMilliseconds(2));
ResetScreenshotCount();
} }
protected: protected:
...@@ -91,11 +97,27 @@ class PowerButtonScreenshotControllerTest : public PowerButtonTestBase { ...@@ -91,11 +97,27 @@ class PowerButtonScreenshotControllerTest : public PowerButtonTestBase {
} }
int GetScreenshotCount() const { int GetScreenshotCount() const {
if (features::IsCaptureModeEnabled()) {
constexpr char kClamshellHistogram[] =
"Ash.CaptureModeController.EntryPoint.ClamshellMode";
constexpr char kTabletHistogram[] =
"Ash.CaptureModeController.EntryPoint.TabletMode";
if (Shell::Get()->tablet_mode_controller()->InTabletMode()) {
return histograme_tester_->GetBucketCount(
kTabletHistogram, CaptureModeEntryType::kCaptureAllDisplays);
}
return histograme_tester_->GetBucketCount(
kClamshellHistogram, CaptureModeEntryType::kCaptureAllDisplays);
}
return screenshot_delegate_->handle_take_screenshot_count(); return screenshot_delegate_->handle_take_screenshot_count();
} }
void ResetScreenshotCount() { void ResetScreenshotCount() {
return screenshot_delegate_->reset_handle_take_screenshot_count(); if (features::IsCaptureModeEnabled())
histograme_tester_ = std::make_unique<base::HistogramTester>();
else
screenshot_delegate_->reset_handle_take_screenshot_count();
} }
bool LastKeyConsumed() const { bool LastKeyConsumed() const {
...@@ -110,6 +132,9 @@ class PowerButtonScreenshotControllerTest : public PowerButtonTestBase { ...@@ -110,6 +132,9 @@ class PowerButtonScreenshotControllerTest : public PowerButtonTestBase {
// ReleaseKey(). // ReleaseKey().
std::unique_ptr<ui::KeyEvent> last_key_event_; std::unique_ptr<ui::KeyEvent> last_key_event_;
// Used to test capture mode invocations when the feature is on.
std::unique_ptr<base::HistogramTester> histograme_tester_;
DISALLOW_COPY_AND_ASSIGN(PowerButtonScreenshotControllerTest); DISALLOW_COPY_AND_ASSIGN(PowerButtonScreenshotControllerTest);
}; };
......
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