Skip to content

Commit

Permalink
cros: Enable views-based lock by default.
Browse files Browse the repository at this point in the history
Bug: 719015
Change-Id: I83b9207720d0166b69058c67a63e57d0ada69527
Reviewed-on: https://chromium-review.googlesource.com/726240
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513987}
  • Loading branch information
jacobdufault-google authored and Commit Bot committed Nov 4, 2017
1 parent 800d45f commit a309db5
Show file tree
Hide file tree
Showing 16 changed files with 72 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ LockScreenActionBackgroundController::Create() {
// Web UI based lock screen implements its own background - use the stub
// lock action background controller implementation unless md-based lock UI
// is used.
if (!switches::IsUsingMdLogin())
if (switches::IsUsingWebUiLock())
return std::make_unique<LockScreenActionBackgroundControllerStub>();
return std::make_unique<LockScreenActionBackgroundControllerImpl>();
}
Expand Down
8 changes: 0 additions & 8 deletions ash/login/ui/login_test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@

#include <string>

#include "ash/public/cpp/ash_switches.h"
#include "ash/public/cpp/config.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/public/interfaces/tray_action.mojom.h"
#include "ash/shell.h"
#include "base/command_line.h"
#include "services/ui/public/cpp/property_type_converters.h"
#include "services/ui/public/interfaces/window_manager.mojom.h"
#include "ui/views/widget/widget.h"
Expand Down Expand Up @@ -92,12 +90,6 @@ void LoginTestBase::SetUserCount(size_t count) {
data_dispatcher_.NotifyUsers(users_);
}

void LoginTestBase::SetUp() {
base::CommandLine::ForCurrentProcess()->AppendSwitch(switches::kShowMdLogin);

AshTestBase::SetUp();
}

void LoginTestBase::TearDown() {
widget_.reset();

Expand Down
1 change: 0 additions & 1 deletion ash/login/ui/login_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class LoginTestBase : public AshTestBase {
LoginDataDispatcher* data_dispatcher() { return &data_dispatcher_; }

// AshTestBase:
void SetUp() override;
void TearDown() override;

private:
Expand Down
9 changes: 5 additions & 4 deletions ash/public/cpp/ash_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,9 @@ const char kForceClamshellPowerButton[] = "force-clamshell-power-button";
// Whether this device has an internal stylus.
const char kHasInternalStylus[] = "has-internal-stylus";

// If true, the views-based md login and lock screens will be shown.
const char kShowMdLogin[] = "show-md-login";
// If true, the webui lock screen wil be shown. This is deprecated and will be
// removed in the future.
const char kShowWebUiLock[] = "show-webui-lock";

// Number of recent accelerometer samples to examine to determine if a power
// button event was spurious.
Expand Down Expand Up @@ -174,8 +175,8 @@ bool IsNightLightEnabled() {
kAshEnableNightLight);
}

bool IsUsingMdLogin() {
return base::CommandLine::ForCurrentProcess()->HasSwitch(kShowMdLogin);
bool IsUsingWebUiLock() {
return base::CommandLine::ForCurrentProcess()->HasSwitch(kShowWebUiLock);
}

} // namespace switches
Expand Down
5 changes: 2 additions & 3 deletions ash/public/cpp/ash_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ ASH_PUBLIC_EXPORT extern const char kAshTouchHud[];
ASH_PUBLIC_EXPORT extern const char kAuraLegacyPowerButton[];
ASH_PUBLIC_EXPORT extern const char kForceClamshellPowerButton[];
ASH_PUBLIC_EXPORT extern const char kHasInternalStylus[];
ASH_PUBLIC_EXPORT extern const char kShowMdLogin[];
ASH_PUBLIC_EXPORT extern const char kShowWebUiLock[];
ASH_PUBLIC_EXPORT extern const char kSpuriousPowerButtonWindow[];
ASH_PUBLIC_EXPORT extern const char kSpuriousPowerButtonAccelCount[];
ASH_PUBLIC_EXPORT extern const char kSpuriousPowerButtonScreenAccel[];
Expand All @@ -65,8 +65,7 @@ ASH_PUBLIC_EXPORT extern const char kUseIMEService[];

ASH_PUBLIC_EXPORT bool IsNightLightEnabled();

// Returns true if the md based login/lock UI is enabled.
ASH_PUBLIC_EXPORT bool IsUsingMdLogin();
ASH_PUBLIC_EXPORT bool IsUsingWebUiLock();

} // namespace switches
} // namespace ash
Expand Down
6 changes: 3 additions & 3 deletions ash/root_window_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -877,9 +877,9 @@ void RootWindowController::CreateContainers() {

// The shelf should be displayed on lock screen if md-based login/lock UI is
// enabled.
aura::Window* shelf_container_parent = switches::IsUsingMdLogin()
? lock_screen_related_containers
: non_lock_screen_containers;
aura::Window* shelf_container_parent = switches::IsUsingWebUiLock()
? non_lock_screen_containers
: lock_screen_related_containers;
aura::Window* shelf_container = CreateContainer(
kShellWindowId_ShelfContainer, "ShelfContainer", shelf_container_parent);
wm::SetSnapsChildrenToPhysicalPixelBoundary(shelf_container);
Expand Down
2 changes: 1 addition & 1 deletion ash/shelf/shelf_widget.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ ShelfWidget::DelegateView::~DelegateView() {}

// static
bool ShelfWidget::IsUsingMdLoginShelf() {
return switches::IsUsingMdLogin() &&
return !switches::IsUsingWebUiLock() &&
(Shell::Get()->session_controller()->GetSessionState() ==
session_manager::SessionState::LOCKED ||
Shell::Get()->session_controller()->GetSessionState() ==
Expand Down
3 changes: 3 additions & 0 deletions ash/system/status_area_widget_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ class StatusAreaWidgetFocusTest : public AshTestBase {

// AshTestBase:
void SetUp() override {
base::CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kShowWebUiLock);

AshTestBase::SetUp();
test_observer_.reset(new SystemTrayFocusTestObserver);
Shell::Get()->system_tray_notifier()->AddSystemTrayFocusObserver(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ void StateController::RequestNewLockScreenNote(LockScreenNoteOrigin origin) {
// This is not needed for requests that come from the lock UI as the lock
// screen UI sends these requests *after* the note action launch animation.
if (origin == LockScreenNoteOrigin::kStylusEject &&
!ash::switches::IsUsingMdLogin()) {
ash::switches::IsUsingWebUiLock()) {
app_launch_delayed_for_animation_ = true;
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -766,17 +766,17 @@ class LockScreenAppStateNoStylusInputTest : public LockScreenAppStateTest {
};

// Tests with show-md-login flag set.
class LockScreenAppStateMdLoginTest : public LockScreenAppStateTest {
class LockScreenAppStateWebUiLockTest : public LockScreenAppStateTest {
public:
LockScreenAppStateMdLoginTest() = default;
~LockScreenAppStateMdLoginTest() override = default;
LockScreenAppStateWebUiLockTest() = default;
~LockScreenAppStateWebUiLockTest() override = default;

void SetUpCommandLine(base::CommandLine* command_line) override {
command_line->AppendSwitch(ash::switches::kShowMdLogin);
command_line->AppendSwitch(ash::switches::kShowWebUiLock);
}

private:
DISALLOW_COPY_AND_ASSIGN(LockScreenAppStateMdLoginTest);
DISALLOW_COPY_AND_ASSIGN(LockScreenAppStateWebUiLockTest);
};

} // namespace
Expand Down Expand Up @@ -1223,7 +1223,7 @@ TEST_F(LockScreenAppStateTest, HandleActionWithLaunchFailure) {
EXPECT_EQ(2, app_manager()->launch_count());
}

TEST_F(LockScreenAppStateTest, LaunchActionWhenStylusGetsRemoved) {
TEST_F(LockScreenAppStateWebUiLockTest, LaunchActionWhenStylusGetsRemoved) {
ui::test::DeviceDataManagerTestAPI devices_test_api;
ASSERT_TRUE(InitializeNoteTakingApp(TrayActionState::kAvailable,
true /* enable_app_launch */));
Expand All @@ -1248,7 +1248,7 @@ TEST_F(LockScreenAppStateTest, LaunchActionWhenStylusGetsRemoved) {
EXPECT_EQ(1, app_manager()->launch_count());
}

TEST_F(LockScreenAppStateMdLoginTest, LaunchActionWhenStylusGetsRemoved) {
TEST_F(LockScreenAppStateTest, LaunchActionWhenStylusGetsRemoved) {
ui::test::DeviceDataManagerTestAPI devices_test_api;
ASSERT_TRUE(InitializeNoteTakingApp(TrayActionState::kAvailable,
true /* enable_app_launch */));
Expand All @@ -1271,7 +1271,7 @@ TEST_F(LockScreenAppStateMdLoginTest, LaunchActionWhenStylusGetsRemoved) {
EXPECT_EQ(1, app_manager()->launch_count());
}

TEST_F(LockScreenAppStateTest,
TEST_F(LockScreenAppStateWebUiLockTest,
LaunchActionWhenStylusRemoved_ActionClosedBeforeAnimationDone) {
ui::test::DeviceDataManagerTestAPI devices_test_api;
ASSERT_TRUE(InitializeNoteTakingApp(TrayActionState::kAvailable,
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/login/chrome_restart_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ void DeriveCommandLine(const GURL& start_url,
ash::switches::kAshTouchHud,
ash::switches::kAuraLegacyPowerButton,
ash::switches::kForceClamshellPowerButton,
ash::switches::kShowMdLogin,
ash::switches::kShowWebUiLock,
chromeos::switches::kDefaultWallpaperLarge,
chromeos::switches::kDefaultWallpaperSmall,
chromeos::switches::kGuestWallpaperLarge,
Expand Down
27 changes: 15 additions & 12 deletions chrome/browser/chromeos/login/lock/screen_locker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,18 @@ void ScreenLocker::Init() {

authenticator_ = UserSessionManager::GetInstance()->CreateAuthenticator(this);
extended_authenticator_ = ExtendedAuthenticator::Create(this);
if (ash::switches::IsUsingMdLogin()) {
if (ash::switches::IsUsingWebUiLock()) {
web_ui_.reset(new WebUIScreenLocker(this));
delegate_ = web_ui_.get();
web_ui_->LockScreen();

// Ownership of |icon_image_source| is passed.
screenlock_icon_provider_ = base::MakeUnique<ScreenlockIconProvider>();
ScreenlockIconSource* screenlock_icon_source =
new ScreenlockIconSource(screenlock_icon_provider_->AsWeakPtr());
content::URLDataSource::Add(web_ui_->web_contents()->GetBrowserContext(),
screenlock_icon_source);
} else {
// Create delegate that calls into the views-based lock screen via mojo.
views_screen_locker_ = base::MakeUnique<ViewsScreenLocker>(this);
delegate_ = views_screen_locker_.get();
Expand All @@ -215,17 +226,6 @@ void ScreenLocker::Init() {
views_screen_locker_.get()));

views_screen_locker_->Init();
} else {
web_ui_.reset(new WebUIScreenLocker(this));
delegate_ = web_ui_.get();
web_ui_->LockScreen();

// Ownership of |icon_image_source| is passed.
screenlock_icon_provider_ = base::MakeUnique<ScreenlockIconProvider>();
ScreenlockIconSource* screenlock_icon_source =
new ScreenlockIconSource(screenlock_icon_provider_->AsWeakPtr());
content::URLDataSource::Add(web_ui_->web_contents()->GetBrowserContext(),
screenlock_icon_source);
}

// Start locking on ash side.
Expand Down Expand Up @@ -479,6 +479,9 @@ void ScreenLocker::ShutDownClass() {
DCHECK(g_screen_lock_observer);
delete g_screen_lock_observer;
g_screen_lock_observer = nullptr;

// Delete |screen_locker_| if it is being shown.
ScheduleDeletion();
}

// static
Expand Down
27 changes: 21 additions & 6 deletions chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <memory>

#include "ash/public/cpp/ash_switches.h"
#include "ash/wm/window_state.h"
#include "base/command_line.h"
#include "base/macros.h"
Expand Down Expand Up @@ -117,6 +118,10 @@ class ScreenLockerTest : public InProcessBrowserTest {
->notify_lock_screen_dismissed_call_count();
}

void SetUpCommandLine(base::CommandLine* command_line) override {
command_line->AppendSwitchASCII(switches::kLoginProfile, "user");
}

private:
void SetUpInProcessBrowserTestFixture() override {
fake_session_manager_client_ = new FakeSessionManagerClient;
Expand All @@ -127,16 +132,26 @@ class ScreenLockerTest : public InProcessBrowserTest {
ui::ScopedAnimationDurationScaleMode::ZERO_DURATION));
}

void SetUpCommandLine(base::CommandLine* command_line) override {
command_line->AppendSwitchASCII(switches::kLoginProfile, "user");
}

std::unique_ptr<ui::ScopedAnimationDurationScaleMode> zero_duration_mode_;

DISALLOW_COPY_AND_ASSIGN(ScreenLockerTest);
};

IN_PROC_BROWSER_TEST_F(ScreenLockerTest, TestBasic) {
class WebUiScreenLockerTest : public ScreenLockerTest {
public:
WebUiScreenLockerTest() = default;
~WebUiScreenLockerTest() override = default;

void SetUpCommandLine(base::CommandLine* command_line) override {
ScreenLockerTest::SetUpCommandLine(command_line);
command_line->AppendSwitch(ash::switches::kShowWebUiLock);
}

private:
DISALLOW_COPY_AND_ASSIGN(WebUiScreenLockerTest);
};

IN_PROC_BROWSER_TEST_F(WebUiScreenLockerTest, TestBasic) {
ScreenLocker::Show();
std::unique_ptr<test::ScreenLockerTester> tester(ScreenLocker::GetTester());
tester->EmulateWindowManagerReady();
Expand Down Expand Up @@ -184,7 +199,7 @@ IN_PROC_BROWSER_TEST_F(ScreenLockerTest, LockScreenWhileAddingUser) {
}

// Test how locking the screen affects an active fullscreen window.
IN_PROC_BROWSER_TEST_F(ScreenLockerTest, TestFullscreenExit) {
IN_PROC_BROWSER_TEST_F(WebUiScreenLockerTest, TestFullscreenExit) {
// 1) If the active browser window is in fullscreen and the fullscreen window
// does not have all the pixels (e.g. the shelf is auto hidden instead of
// hidden), locking the screen should not exit fullscreen. The shelf is
Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/chromeos/shutdown_policy_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <string>

#include "ash/login_status.h"
#include "ash/public/cpp/ash_switches.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/system/tiles/tiles_default_view.h"
Expand Down Expand Up @@ -214,6 +215,12 @@ class ShutdownPolicyLockerTest : public ShutdownPolicyBaseTest {
ShutdownPolicyLockerTest() : fake_session_manager_client_(nullptr) {}
~ShutdownPolicyLockerTest() override {}

void SetUp() override {
base::CommandLine::ForCurrentProcess()->AppendSwitch(
ash::switches::kShowWebUiLock);
ShutdownPolicyBaseTest::SetUp();
}

void SetUpInProcessBrowserTestFixture() override {
fake_session_manager_client_ = new FakeSessionManagerClient;
DBusThreadManager::GetSetterForTesting()->SetSessionManagerClient(
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/webui/chromeos/login/oobe_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,8 @@ void OobeUI::GetLocalizedStrings(base::DictionaryValue* localized_strings) {
chromeos::switches::kDisableMdErrorScreen)
? "off"
: "on");
localized_strings->SetString("showMdLogin",
ash::switches::IsUsingMdLogin() ? "on" : "off");
localized_strings->SetString(
"showMdLogin", ash::switches::IsUsingWebUiLock() ? "off" : "on");
}

void OobeUI::AddWebUIHandler(std::unique_ptr<BaseWebUIHandler> handler) {
Expand Down
2 changes: 2 additions & 0 deletions testing/buildbot/filters/ash_unittests_mash.filter
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@
-WindowSelectorTest.RemoveDisplayWithAnimation
-WindowTreeHostManagerTest.UpdateMouseLocationAfterDisplayChange_PrimaryDisconnected
-WindowTreeHostManagerTest.UpdateMouseLocationAfterDisplayChange_SwapPrimary
# TODO: Panel focus is broken. http://crbug.com/780936
-WorkspaceControllerTest.WindowEdgeMouseHitTestPanel
# TODO: fix this. It might fail because GL_OES_texture_npot is not supported.
# http://crbug.com/775067.
-WorkspaceLayoutManagerBackdropTest.BackdropTest
Expand Down

0 comments on commit a309db5

Please sign in to comment.