Commit 47a732b0 authored by Giovanni Ortuño Urquidi's avatar Giovanni Ortuño Urquidi Committed by Commit Bot

Revert "Support feedback keyboard shortcut in views login screen."

This reverts commit b0278053.

Reason for revert: UserAddingScreenTest.ScreenVisibility has been
flaky since this landed:

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=UserAddingScreenTest.ScreenVisibility

[28936:28936:0617/213745.316026:FATAL:accelerator_manager.cc(32)] Check failed: !base::ContainsValue(targets, target). Registering the same target multiple times
#0 0x000004980f5c base::debug::StackTrace::StackTrace()
#1 0x000004902b4b logging::LogMessage::~LogMessage()
#2 0x000005857d49 ui::AcceleratorManager::Register()
#3 0x000001ca3442 chromeos::WebUILoginView::WebUILoginView()
#4 0x000001c9a51c chromeos::LoginDisplayHostWebUI::InitLoginWindowAndView()
#5 0x000001c98cc5 chromeos::LoginDisplayHostWebUI::OnStartUserAdding()
#6 0x000001ca20c7 chromeos::(anonymous namespace)::UserAddingScreenImpl::Start()
#7 0x0000015dfd1c chromeos::UserAddingScreenTest_ScreenVisibility_Test::RunTestOnMainThread()
#8 0x000004f449b7 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()

Original change's description:
> Support feedback keyboard shortcut in views login screen.
>
> This CL supports alt+shift+i to open the feedback dialog in views login
> screen.
>
> Bug: 852242
> Change-Id: I9a285c6929fd2302d22f90fbefbb32bcd814359d
> Reviewed-on: https://chromium-review.googlesource.com/1102035
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: Jacob Dufault <jdufault@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Commit-Queue: Xiaoyin Hu <xiaoyinh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#567829}

TBR=xiyuan@chromium.org,tsepez@chromium.org,jdufault@chromium.org,xiaoyinh@chromium.org

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

Bug: 852242, 853445
Change-Id: I21afadf31af3c9ae8e04a30f6021b928efde43f8
Reviewed-on: https://chromium-review.googlesource.com/1103957
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: default avatarGiovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567935}
parent da3a9ac2
...@@ -219,12 +219,6 @@ void LoginScreenController::RequestPublicSessionKeyboardLayouts( ...@@ -219,12 +219,6 @@ void LoginScreenController::RequestPublicSessionKeyboardLayouts(
login_screen_client_->RequestPublicSessionKeyboardLayouts(account_id, locale); login_screen_client_->RequestPublicSessionKeyboardLayouts(account_id, locale);
} }
void LoginScreenController::ShowFeedback() {
if (!login_screen_client_)
return;
login_screen_client_->ShowFeedback();
}
void LoginScreenController::AddObserver( void LoginScreenController::AddObserver(
LoginScreenControllerObserver* observer) { LoginScreenControllerObserver* observer) {
observers_.AddObserver(observer); observers_.AddObserver(observer);
......
...@@ -77,7 +77,6 @@ class ASH_EXPORT LoginScreenController : public mojom::LoginScreen { ...@@ -77,7 +77,6 @@ class ASH_EXPORT LoginScreenController : public mojom::LoginScreen {
const std::string& input_method); const std::string& input_method);
void RequestPublicSessionKeyboardLayouts(const AccountId& account_id, void RequestPublicSessionKeyboardLayouts(const AccountId& account_id,
const std::string& locale); const std::string& locale);
void ShowFeedback();
// Add or remove an observer. // Add or remove an observer.
void AddObserver(LoginScreenControllerObserver* observer); void AddObserver(LoginScreenControllerObserver* observer);
......
...@@ -66,7 +66,6 @@ class MockLoginScreenClient : public mojom::LoginScreenClient { ...@@ -66,7 +66,6 @@ class MockLoginScreenClient : public mojom::LoginScreenClient {
const std::string& input_method)); const std::string& input_method));
MOCK_METHOD2(RequestPublicSessionKeyboardLayouts, MOCK_METHOD2(RequestPublicSessionKeyboardLayouts,
void(const AccountId& account_id, const std::string& locale)); void(const AccountId& account_id, const std::string& locale));
MOCK_METHOD0(ShowFeedback, void());
private: private:
bool authenticate_user_callback_result_ = true; bool authenticate_user_callback_result_ = true;
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include <memory> #include <memory>
#include <utility> #include <utility>
#include "ash/accelerators/accelerator_controller.h"
#include "ash/detachable_base/detachable_base_pairing_status.h" #include "ash/detachable_base/detachable_base_pairing_status.h"
#include "ash/focus_cycler.h" #include "ash/focus_cycler.h"
#include "ash/ime/ime_controller.h" #include "ash/ime/ime_controller.h"
...@@ -77,7 +76,7 @@ constexpr int kMediumDensityMarginLeftOfAuthUserPortraitDp = 0; ...@@ -77,7 +76,7 @@ constexpr int kMediumDensityMarginLeftOfAuthUserPortraitDp = 0;
constexpr int kMediumDensityDistanceBetweenAuthUserAndUsersLandscapeDp = 220; constexpr int kMediumDensityDistanceBetweenAuthUserAndUsersLandscapeDp = 220;
constexpr int kMediumDensityDistanceBetweenAuthUserAndUsersPortraitDp = 84; constexpr int kMediumDensityDistanceBetweenAuthUserAndUsersPortraitDp = 84;
constexpr char kLockContentsViewName[] = "LockContentsView"; constexpr const char kLockContentsViewName[] = "LockContentsView";
// A view which stores two preferred sizes. The embedder can control which one // A view which stores two preferred sizes. The embedder can control which one
// is used. // is used.
...@@ -304,7 +303,6 @@ LockContentsView::LockContentsView( ...@@ -304,7 +303,6 @@ LockContentsView::LockContentsView(
OnLockScreenNoteStateChanged(initial_note_action_state); OnLockScreenNoteStateChanged(initial_note_action_state);
chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver( chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(
this); this);
RegisterAccelerators();
} }
LockContentsView::~LockContentsView() { LockContentsView::~LockContentsView() {
...@@ -375,15 +373,6 @@ void LockContentsView::GetAccessibleNodeData(ui::AXNodeData* node_data) { ...@@ -375,15 +373,6 @@ void LockContentsView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->SetNameExplicitlyEmpty(); node_data->SetNameExplicitlyEmpty();
} }
bool LockContentsView::AcceleratorPressed(const ui::Accelerator& accelerator) {
auto entry = accel_map_.find(accelerator);
if (entry == accel_map_.end())
return false;
PerformAction(entry->second);
return true;
}
void LockContentsView::OnUsersChanged( void LockContentsView::OnUsersChanged(
const std::vector<mojom::LoginUserInfoPtr>& users) { const std::vector<mojom::LoginUserInfoPtr>& users) {
// The debug view will potentially call this method many times. Make sure to // The debug view will potentially call this method many times. Make sure to
...@@ -1333,24 +1322,4 @@ void LockContentsView::DisableLockScreenNote() { ...@@ -1333,24 +1322,4 @@ void LockContentsView::DisableLockScreenNote() {
OnLockScreenNoteStateChanged(mojom::TrayActionState::kNotAvailable); OnLockScreenNoteStateChanged(mojom::TrayActionState::kNotAvailable);
} }
void LockContentsView::RegisterAccelerators() {
// TODO: Add more accelerators that are applicable to login screen.
accel_map_[ui::Accelerator(ui::VKEY_I, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN)] =
AcceleratorAction::kShowFeedback;
AcceleratorController* controller = Shell::Get()->accelerator_controller();
for (const auto& item : accel_map_)
controller->Register({item.first}, this);
}
void LockContentsView::PerformAction(AcceleratorAction action) {
switch (action) {
case AcceleratorAction::kShowFeedback:
Shell::Get()->login_screen_controller()->ShowFeedback();
return;
default:
NOTREACHED();
}
}
} // namespace ash } // namespace ash
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#ifndef ASH_LOGIN_UI_LOCK_CONTENTS_VIEW_H_ #ifndef ASH_LOGIN_UI_LOCK_CONTENTS_VIEW_H_
#define ASH_LOGIN_UI_LOCK_CONTENTS_VIEW_H_ #define ASH_LOGIN_UI_LOCK_CONTENTS_VIEW_H_
#include <map>
#include <memory> #include <memory>
#include <string> #include <string>
#include <vector> #include <vector>
...@@ -100,10 +99,6 @@ class ASH_EXPORT LockContentsView ...@@ -100,10 +99,6 @@ class ASH_EXPORT LockContentsView
kExclusivePublicAccountExpandedView, kExclusivePublicAccountExpandedView,
}; };
enum class AcceleratorAction {
kShowFeedback,
};
// Number of login attempts before a login dialog is shown. For example, if // Number of login attempts before a login dialog is shown. For example, if
// this value is 4 then the user can submit their password 4 times, and on the // this value is 4 then the user can submit their password 4 times, and on the
// 4th bad attempt the login dialog is shown. This only applies to the login // 4th bad attempt the login dialog is shown. This only applies to the login
...@@ -123,7 +118,6 @@ class ASH_EXPORT LockContentsView ...@@ -123,7 +118,6 @@ class ASH_EXPORT LockContentsView
void OnFocus() override; void OnFocus() override;
void AboutToRequestFocusFromTabTraversal(bool reverse) override; void AboutToRequestFocusFromTabTraversal(bool reverse) override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override; void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
// LoginScreenController::Observer: // LoginScreenController::Observer:
void SetAvatarForUser(const AccountId& account_id, void SetAvatarForUser(const AccountId& account_id,
...@@ -337,12 +331,6 @@ class ASH_EXPORT LockContentsView ...@@ -337,12 +331,6 @@ class ASH_EXPORT LockContentsView
// All the subsequent calls of |OnLockScreenNoteStateChanged| will be ignored. // All the subsequent calls of |OnLockScreenNoteStateChanged| will be ignored.
void DisableLockScreenNote(); void DisableLockScreenNote();
// Register accelerators used in login screen.
void RegisterAccelerators();
// Performs the specified accelerator action.
void PerformAction(AcceleratorAction action);
const LockScreen::ScreenType screen_type_; const LockScreen::ScreenType screen_type_;
std::vector<UserState> users_; std::vector<UserState> users_;
...@@ -404,9 +392,6 @@ class ASH_EXPORT LockContentsView ...@@ -404,9 +392,6 @@ class ASH_EXPORT LockContentsView
// KeyboardControllerState::HIDDEN. // KeyboardControllerState::HIDDEN.
bool keyboard_shown_ = false; bool keyboard_shown_ = false;
// Accelerators handled by login screen.
std::map<ui::Accelerator, AcceleratorAction> accel_map_;
DISALLOW_COPY_AND_ASSIGN(LockContentsView); DISALLOW_COPY_AND_ASSIGN(LockContentsView);
}; };
......
...@@ -213,7 +213,4 @@ interface LoginScreenClient { ...@@ -213,7 +213,4 @@ interface LoginScreenClient {
// locale. // locale.
RequestPublicSessionKeyboardLayouts(signin.mojom.AccountId account_id, RequestPublicSessionKeyboardLayouts(signin.mojom.AccountId account_id,
string locale); string locale);
// Request to show a feedback report dialog in chrome.
ShowFeedback();
}; };
...@@ -133,6 +133,4 @@ void FakeLoginDisplayHost::MigrateUserData(const std::string& old_password) {} ...@@ -133,6 +133,4 @@ void FakeLoginDisplayHost::MigrateUserData(const std::string& old_password) {}
void FakeLoginDisplayHost::ResyncUserData() {} void FakeLoginDisplayHost::ResyncUserData() {}
void FakeLoginDisplayHost::ShowFeedback() {}
} // namespace chromeos } // namespace chromeos
...@@ -62,7 +62,6 @@ class FakeLoginDisplayHost : public LoginDisplayHost { ...@@ -62,7 +62,6 @@ class FakeLoginDisplayHost : public LoginDisplayHost {
void CancelPasswordChangedFlow() override; void CancelPasswordChangedFlow() override;
void MigrateUserData(const std::string& old_password) override; void MigrateUserData(const std::string& old_password) override;
void ResyncUserData() override; void ResyncUserData() override;
void ShowFeedback() override;
private: private:
class FakeBaseScreen; class FakeBaseScreen;
......
...@@ -180,9 +180,6 @@ class LoginDisplayHost { ...@@ -180,9 +180,6 @@ class LoginDisplayHost {
// user data. // user data.
virtual void ResyncUserData() = 0; virtual void ResyncUserData() = 0;
// Shows a feedback report dialog.
virtual void ShowFeedback() = 0;
protected: protected:
LoginDisplayHost(); LoginDisplayHost();
virtual ~LoginDisplayHost(); virtual ~LoginDisplayHost();
......
...@@ -24,10 +24,7 @@ ...@@ -24,10 +24,7 @@
namespace chromeos { namespace chromeos {
namespace { namespace {
constexpr char kLoginDisplay[] = "login"; constexpr char kLoginDisplay[] = "login";
constexpr char kAccelSendFeedback[] = "send_feedback";
} // namespace } // namespace
LoginDisplayHostMojo::LoginDisplayHostMojo() LoginDisplayHostMojo::LoginDisplayHostMojo()
...@@ -248,12 +245,6 @@ const user_manager::UserList LoginDisplayHostMojo::GetUsers() { ...@@ -248,12 +245,6 @@ const user_manager::UserList LoginDisplayHostMojo::GetUsers() {
return users_; return users_;
} }
void LoginDisplayHostMojo::ShowFeedback() {
DCHECK(GetOobeUI());
GetOobeUI()->web_ui()->CallJavascriptFunctionUnsafe(
"cr.ui.Oobe.handleAccelerator", base::Value(kAccelSendFeedback));
}
void LoginDisplayHostMojo::CancelPasswordChangedFlow() { void LoginDisplayHostMojo::CancelPasswordChangedFlow() {
// Close the Oobe UI dialog. // Close the Oobe UI dialog.
UpdateGaiaDialogVisibility(false /*visible*/, true /*can_close*/, UpdateGaiaDialogVisibility(false /*visible*/, true /*can_close*/,
......
...@@ -88,7 +88,6 @@ class LoginDisplayHostMojo : public LoginDisplayHostCommon, ...@@ -88,7 +88,6 @@ class LoginDisplayHostMojo : public LoginDisplayHostCommon,
void UpdateGaiaDialogSize(int width, int height) override; void UpdateGaiaDialogSize(int width, int height) override;
const user_manager::UserList GetUsers() override; const user_manager::UserList GetUsers() override;
void CancelPasswordChangedFlow() override; void CancelPasswordChangedFlow() override;
void ShowFeedback() override;
// LoginScreenClient::Delegate: // LoginScreenClient::Delegate:
void HandleAuthenticateUser(const AccountId& account_id, void HandleAuthenticateUser(const AccountId& account_id,
......
...@@ -1164,10 +1164,6 @@ const user_manager::UserList LoginDisplayHostWebUI::GetUsers() { ...@@ -1164,10 +1164,6 @@ const user_manager::UserList LoginDisplayHostWebUI::GetUsers() {
return user_manager::UserList(); return user_manager::UserList();
} }
void LoginDisplayHostWebUI::ShowFeedback() {
NOTREACHED();
}
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// external // external
......
...@@ -77,7 +77,6 @@ class LoginDisplayHostWebUI : public LoginDisplayHostCommon, ...@@ -77,7 +77,6 @@ class LoginDisplayHostWebUI : public LoginDisplayHostCommon,
const base::Optional<AccountId>& prefilled_account) override; const base::Optional<AccountId>& prefilled_account) override;
void UpdateGaiaDialogSize(int width, int height) override; void UpdateGaiaDialogSize(int width, int height) override;
const user_manager::UserList GetUsers() override; const user_manager::UserList GetUsers() override;
void ShowFeedback() override;
// Creates WizardController instance. // Creates WizardController instance.
WizardController* CreateWizardController(); WizardController* CreateWizardController();
......
...@@ -69,7 +69,6 @@ class MockLoginDisplayHost : public LoginDisplayHost { ...@@ -69,7 +69,6 @@ class MockLoginDisplayHost : public LoginDisplayHost {
MOCK_METHOD0(CancelPasswordChangedFlow, void()); MOCK_METHOD0(CancelPasswordChangedFlow, void());
MOCK_METHOD1(MigrateUserData, void(const std::string&)); MOCK_METHOD1(MigrateUserData, void(const std::string&));
MOCK_METHOD0(ResyncUserData, void()); MOCK_METHOD0(ResyncUserData, void());
MOCK_METHOD0(ShowFeedback, void());
private: private:
DISALLOW_COPY_AND_ASSIGN(MockLoginDisplayHost); DISALLOW_COPY_AND_ASSIGN(MockLoginDisplayHost);
......
...@@ -146,11 +146,6 @@ void LoginScreenClient::RequestPublicSessionKeyboardLayouts( ...@@ -146,11 +146,6 @@ void LoginScreenClient::RequestPublicSessionKeyboardLayouts(
locale); locale);
} }
void LoginScreenClient::ShowFeedback() {
if (chromeos::LoginDisplayHost::default_host())
chromeos::LoginDisplayHost::default_host()->ShowFeedback();
}
void LoginScreenClient::LoadWallpaper(const AccountId& account_id) { void LoginScreenClient::LoadWallpaper(const AccountId& account_id) {
WallpaperControllerClient::Get()->ShowUserWallpaper(account_id); WallpaperControllerClient::Get()->ShowUserWallpaper(account_id);
} }
......
...@@ -82,7 +82,6 @@ class LoginScreenClient : public ash::mojom::LoginScreenClient { ...@@ -82,7 +82,6 @@ class LoginScreenClient : public ash::mojom::LoginScreenClient {
const std::string& input_method) override; const std::string& input_method) override;
void RequestPublicSessionKeyboardLayouts(const AccountId& account_id, void RequestPublicSessionKeyboardLayouts(const AccountId& account_id,
const std::string& locale) override; const std::string& locale) override;
void ShowFeedback() override;
private: private:
void SetPublicSessionKeyboardLayout( void SetPublicSessionKeyboardLayout(
......
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