Skip to content

Commit

Permalink
chromeos: Remove some IME methods from ash::SystemTrayDelegate
Browse files Browse the repository at this point in the history
For the mustash project we're trying to eliminate delegates that call
from ash back into chrome browser.

The IME InputMethodManager interfaces live in //ui/base/ime/chromeos, and
//ash already uses them, so convert some of the system tray code to use
InputMethodManager directly.

* Remove InputMethodSwitchRecorder and move histogram recording to //ash
* Use InputMethodManager directly in the ash keyboard accelerator code
* Remove ash::ImeControlDelegate and chrome's ImeController.
* Use MockInputMethodManager in all AcceleratorControllerTests

BUG=724305
TEST=ash_unittests, manually switch IMEs when running chrome

Review-Url: https://codereview.chromium.org/2891263002
Cr-Commit-Position: refs/heads/master@{#474380}
  • Loading branch information
jamescook authored and Commit bot committed May 24, 2017
1 parent 526c7eb commit 1290776
Show file tree
Hide file tree
Showing 21 changed files with 169 additions and 358 deletions.
2 changes: 1 addition & 1 deletion ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ component("ash") {
"host/root_window_transformer.h",
"host/transformer_helper.cc",
"host/transformer_helper.h",
"ime_control_delegate.h",
"ime/ime_switch_type.h",
"key_event_watcher.cc",
"key_event_watcher.h",
"keyboard/keyboard_observer_register.cc",
Expand Down
76 changes: 45 additions & 31 deletions ash/accelerators/accelerator_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include "ash/accessibility_delegate.h"
#include "ash/accessibility_types.h"
#include "ash/focus_cycler.h"
#include "ash/ime_control_delegate.h"
#include "ash/ime/ime_switch_type.h"
#include "ash/media_controller.h"
#include "ash/multi_profile_uma.h"
#include "ash/new_window_controller.h"
Expand Down Expand Up @@ -68,6 +68,7 @@ namespace ash {
namespace {

using base::UserMetricsAction;
using chromeos::input_method::InputMethodManager;
using message_center::Notification;

// Identifier for the high contrast toggle accelerator notification.
Expand Down Expand Up @@ -171,6 +172,11 @@ void RecordUmaHistogram(const char* histogram_name,
histogram->Add(sample);
}

void RecordImeSwitchByAccelerator() {
UMA_HISTOGRAM_ENUMERATION("InputMethod.ImeSwitch",
ImeSwitchType::kAccelerator, ImeSwitchType::kCount);
}

void HandleCycleBackwardMRU(const ui::Accelerator& accelerator) {
if (accelerator.key_code() == ui::VKEY_TAB)
base::RecordAction(base::UserMetricsAction("Accel_PrevWindow_Tab"));
Expand Down Expand Up @@ -251,8 +257,15 @@ void HandleNewWindow() {
Shell::Get()->new_window_controller()->NewWindow(false /* is_incognito */);
}

bool CanHandleNextIme(ImeControlDelegate* ime_control_delegate) {
return ime_control_delegate && ime_control_delegate->CanCycleIme();
bool CanCycleInputMethod() {
InputMethodManager* manager = InputMethodManager::Get();
DCHECK(manager);
if (!manager->GetActiveIMEState()) {
LOG(WARNING) << "Cannot cycle through input methods as they are not "
"initialized yet.";
return false;
}
return manager->GetActiveIMEState()->CanCycleInputMethod();
}

bool CanHandleCycleMru(const ui::Accelerator& accelerator) {
Expand All @@ -267,25 +280,25 @@ bool CanHandleCycleMru(const ui::Accelerator& accelerator) {
return !(keyboard_controller && keyboard_controller->keyboard_visible());
}

void HandleNextIme(ImeControlDelegate* ime_control_delegate) {
void HandleNextIme() {
base::RecordAction(UserMetricsAction("Accel_Next_Ime"));
ime_control_delegate->HandleNextIme();
RecordImeSwitchByAccelerator();
InputMethodManager::Get()->GetActiveIMEState()->SwitchToNextInputMethod();
}

void HandleOpenFeedbackPage() {
base::RecordAction(UserMetricsAction("Accel_Open_Feedback_Page"));
Shell::Get()->new_window_controller()->OpenFeedbackPage();
}

bool CanHandlePreviousIme(ImeControlDelegate* ime_control_delegate) {
return ime_control_delegate && ime_control_delegate->CanCycleIme();
}

void HandlePreviousIme(ImeControlDelegate* ime_control_delegate,
const ui::Accelerator& accelerator) {
void HandlePreviousIme(const ui::Accelerator& accelerator) {
base::RecordAction(UserMetricsAction("Accel_Previous_Ime"));
if (accelerator.key_state() == ui::Accelerator::KeyState::PRESSED)
ime_control_delegate->HandlePreviousIme();
if (accelerator.key_state() == ui::Accelerator::KeyState::PRESSED) {
RecordImeSwitchByAccelerator();
InputMethodManager::Get()
->GetActiveIMEState()
->SwitchToPreviousInputMethod();
}
// Else: consume the Ctrl+Space ET_KEY_RELEASED event but do not do anything.
}

Expand Down Expand Up @@ -352,16 +365,22 @@ void HandleShowTaskManager() {
Shell::Get()->new_window_controller()->ShowTaskManager();
}

bool CanHandleSwitchIme(ImeControlDelegate* ime_control_delegate,
const ui::Accelerator& accelerator) {
return ime_control_delegate &&
ime_control_delegate->CanSwitchIme(accelerator);
bool CanHandleSwitchIme(const ui::Accelerator& accelerator) {
InputMethodManager* manager = InputMethodManager::Get();
DCHECK(manager);
if (!manager->GetActiveIMEState()) {
LOG(WARNING) << "Cannot switch input methods as they are not "
"initialized yet.";
return false;
}
return manager->GetActiveIMEState()->CanSwitchInputMethod(accelerator);
}

void HandleSwitchIme(ImeControlDelegate* ime_control_delegate,
const ui::Accelerator& accelerator) {
void HandleSwitchIme(const ui::Accelerator& accelerator) {
base::RecordAction(UserMetricsAction("Accel_Switch_Ime"));
ime_control_delegate->HandleSwitchIme(accelerator);
RecordImeSwitchByAccelerator();
InputMethodManager::Get()->GetActiveIMEState()->SwitchInputMethod(
accelerator);
}

bool CanHandleToggleAppList(const ui::Accelerator& accelerator,
Expand Down Expand Up @@ -735,11 +754,6 @@ AcceleratorController::GetCurrentAcceleratorRestriction() {
return GetAcceleratorProcessingRestriction(-1);
}

void AcceleratorController::SetImeControlDelegate(
std::unique_ptr<ImeControlDelegate> ime_control_delegate) {
ime_control_delegate_ = std::move(ime_control_delegate);
}

bool AcceleratorController::ShouldCloseMenuAndRepostAccelerator(
const ui::Accelerator& accelerator) const {
auto itr = accelerators_.find(accelerator);
Expand Down Expand Up @@ -912,15 +926,15 @@ bool AcceleratorController::CanPerformAction(
case NEW_INCOGNITO_WINDOW:
return CanHandleNewIncognitoWindow();
case NEXT_IME:
return CanHandleNextIme(ime_control_delegate_.get());
return CanCycleInputMethod();
case PREVIOUS_IME:
return CanHandlePreviousIme(ime_control_delegate_.get());
return CanCycleInputMethod();
case SHOW_MESSAGE_CENTER_BUBBLE:
return CanHandleShowMessageCenterBubble();
case SHOW_STYLUS_TOOLS:
return CanHandleShowStylusTools();
case SWITCH_IME:
return CanHandleSwitchIme(ime_control_delegate_.get(), accelerator);
return CanHandleSwitchIme(accelerator);
case SWITCH_TO_PREVIOUS_USER:
case SWITCH_TO_NEXT_USER:
return CanHandleCycleUser();
Expand Down Expand Up @@ -1111,7 +1125,7 @@ void AcceleratorController::PerformAction(AcceleratorAction action,
HandleNewWindow();
break;
case NEXT_IME:
HandleNextIme(ime_control_delegate_.get());
HandleNextIme();
break;
case OPEN_CROSH:
HandleCrosh();
Expand All @@ -1126,7 +1140,7 @@ void AcceleratorController::PerformAction(AcceleratorAction action,
HandleGetHelp();
break;
case PREVIOUS_IME:
HandlePreviousIme(ime_control_delegate_.get(), accelerator);
HandlePreviousIme(accelerator);
break;
case PRINT_UI_HIERARCHIES:
debug::PrintUIHierarchies();
Expand Down Expand Up @@ -1159,7 +1173,7 @@ void AcceleratorController::PerformAction(AcceleratorAction action,
HandleSuspend();
break;
case SWITCH_IME:
HandleSwitchIme(ime_control_delegate_.get(), accelerator);
HandleSwitchIme(accelerator);
break;
case SWITCH_TO_NEXT_USER:
HandleCycleUser(CycleUserDirection::NEXT);
Expand Down
6 changes: 0 additions & 6 deletions ash/accelerators/accelerator_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ namespace ash {
struct AcceleratorData;
class AcceleratorControllerDelegate;
class ExitWarningHandler;
class ImeControlDelegate;

// AcceleratorController provides functions for registering or unregistering
// global keyboard accelerators, which are handled earlier than any windows. It
Expand Down Expand Up @@ -108,9 +107,6 @@ class ASH_EXPORT AcceleratorController
// Returns the restriction for the current context.
AcceleratorProcessingRestriction GetCurrentAcceleratorRestriction();

void SetImeControlDelegate(
std::unique_ptr<ImeControlDelegate> ime_control_delegate);

// Provides access to the ExitWarningHandler for testing.
ExitWarningHandler* GetExitWarningHandlerForTest() {
return &exit_warning_handler_;
Expand Down Expand Up @@ -187,8 +183,6 @@ class ASH_EXPORT AcceleratorController
// A tracker for the current and previous accelerators.
std::unique_ptr<ui::AcceleratorHistory> accelerator_history_;

std::unique_ptr<ImeControlDelegate> ime_control_delegate_;

// Handles the exit accelerator which requires a double press to exit and
// shows a popup with an explanation.
ExitWarningHandler exit_warning_handler_;
Expand Down
Loading

0 comments on commit 1290776

Please sign in to comment.