Skip to content

Commit

Permalink
mash: Convert FocusCycler to wm common types and move to //ash/common
Browse files Browse the repository at this point in the history
FocusCycler is a dependency of the status area widget code I'm trying to move.

* Introduce WmWindow::GetWidget().
* Move FocusCycler ownership from ash::Shell to WmShell.
* Eliminate extraneous Shell::RotateFocus() method in favor of calling
FocusCycler directly.

I'm intentionally leaving the unit test in place, as it has dependencies that
haven't migrated yet.

BUG=619636
TEST=ash_unittests

Review-Url: https://codereview.chromium.org/2075923002
Cr-Commit-Position: refs/heads/master@{#400354}
  • Loading branch information
jamescook authored and Commit bot committed Jun 17, 2016
1 parent 5c038c1 commit 26f3209
Show file tree
Hide file tree
Showing 21 changed files with 90 additions and 88 deletions.
19 changes: 8 additions & 11 deletions ash/accelerators/accelerator_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ash/accelerators/accelerator_commands.h"
#include "ash/accelerators/debug_commands.h"
#include "ash/ash_switches.h"
#include "ash/common/focus_cycler.h"
#include "ash/common/session/session_state_delegate.h"
#include "ash/common/shelf/shelf_model.h"
#include "ash/common/shell_window_ids.h"
Expand All @@ -23,7 +24,6 @@
#include "ash/common/wm_shell.h"
#include "ash/debug.h"
#include "ash/display/window_tree_host_manager.h"
#include "ash/focus_cycler.h"
#include "ash/gpu_support.h"
#include "ash/ime_control_delegate.h"
#include "ash/magnifier/magnification_controller.h"
Expand Down Expand Up @@ -204,27 +204,24 @@ void HandleCycleForwardMRU(const ui::Accelerator& accelerator) {
WindowCycleController::FORWARD);
}

void HandleRotatePaneFocus(Shell::Direction direction) {
Shell* shell = Shell::GetInstance();
void HandleRotatePaneFocus(FocusCycler::Direction direction) {
switch (direction) {
// TODO(stevet): Not sure if this is the same as IDC_FOCUS_NEXT_PANE.
case Shell::FORWARD: {
case FocusCycler::FORWARD: {
base::RecordAction(UserMetricsAction("Accel_Focus_Next_Pane"));
shell->focus_cycler()->RotateFocus(FocusCycler::FORWARD);
break;
}
case Shell::BACKWARD: {
case FocusCycler::BACKWARD: {
base::RecordAction(UserMetricsAction("Accel_Focus_Previous_Pane"));
shell->focus_cycler()->RotateFocus(FocusCycler::BACKWARD);
break;
}
}
WmShell::Get()->focus_cycler()->RotateFocus(direction);
}

void HandleFocusShelf() {
Shell* shell = Shell::GetInstance();
base::RecordAction(base::UserMetricsAction("Accel_Focus_Shelf"));
shell->focus_cycler()->FocusWidget(
WmShell::Get()->focus_cycler()->FocusWidget(
Shelf::ForPrimaryDisplay()->shelf_widget());
}

Expand Down Expand Up @@ -1149,10 +1146,10 @@ void AcceleratorController::PerformAction(AcceleratorAction action,
exit_warning_handler_.HandleAccelerator();
break;
case FOCUS_NEXT_PANE:
HandleRotatePaneFocus(Shell::FORWARD);
HandleRotatePaneFocus(FocusCycler::FORWARD);
break;
case FOCUS_PREVIOUS_PANE:
HandleRotatePaneFocus(Shell::BACKWARD);
HandleRotatePaneFocus(FocusCycler::BACKWARD);
break;
case FOCUS_SHELF:
HandleFocusShelf();
Expand Down
4 changes: 2 additions & 2 deletions ash/ash.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@
'common/ash_layout_constants.h',
'common/default_accessibility_delegate.cc',
'common/default_accessibility_delegate.h',
'common/focus_cycler.cc',
'common/focus_cycler.h',
'common/login_status.h',
'common/material_design/material_design_controller.cc',
'common/material_design/material_design_controller.h',
Expand Down Expand Up @@ -324,8 +326,6 @@
'first_run/first_run_helper.h',
'first_run/first_run_helper_impl.cc',
'first_run/first_run_helper_impl.h',
'focus_cycler.cc',
'focus_cycler.h',
'frame/caption_buttons/caption_button_types.h',
'frame/caption_buttons/frame_caption_button.cc',
'frame/caption_buttons/frame_caption_button.h',
Expand Down
8 changes: 6 additions & 2 deletions ash/aura/wm_window_aura.cc
Original file line number Diff line number Diff line change
Expand Up @@ -517,9 +517,13 @@ void WmWindowAura::Show() {
window_->Show();
}

views::Widget* WmWindowAura::GetInternalWidget() {
return views::Widget::GetWidgetForNativeView(window_);
}

void WmWindowAura::CloseWidget() {
DCHECK(views::Widget::GetWidgetForNativeView(window_));
views::Widget::GetWidgetForNativeView(window_)->Close();
DCHECK(GetInternalWidget());
GetInternalWidget()->Close();
}

bool WmWindowAura::IsFocused() const {
Expand Down
1 change: 1 addition & 0 deletions ash/aura/wm_window_aura.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ class ASH_EXPORT WmWindowAura : public WmWindow, public aura::WindowObserver {
bool IsAlwaysOnTop() const override;
void Hide() override;
void Show() override;
views::Widget* GetInternalWidget() override;
void CloseWidget() override;
bool IsFocused() const override;
bool IsActive() const override;
Expand Down
42 changes: 18 additions & 24 deletions ash/focus_cycler.cc → ash/common/focus_cycler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ash/focus_cycler.h"
#include "ash/common/focus_cycler.h"

#include "ash/aura/wm_window_aura.h"
#include "ash/common/wm/mru_window_tracker.h"
#include "ash/common/wm/window_state.h"
#include "ash/shell.h"
#include "ash/wm/window_state_aura.h"
#include "ash/wm/window_util.h"
#include "ui/aura/window.h"
#include "ash/common/wm_shell.h"
#include "ash/common/wm_window.h"
#include "ui/views/accessible_pane_view.h"
#include "ui/views/focus/focus_search.h"
#include "ui/views/widget/widget.h"
Expand All @@ -21,17 +18,14 @@ namespace ash {
namespace {

bool HasFocusableWindow() {
return !ash::Shell::GetInstance()->
mru_window_tracker()->BuildMruWindowList().empty();
return !WmShell::Get()->GetMruWindowTracker()->BuildMruWindowList().empty();
}

} // namespace

FocusCycler::FocusCycler() : widget_activating_(NULL) {
}
FocusCycler::FocusCycler() : widget_activating_(nullptr) {}

FocusCycler::~FocusCycler() {
}
FocusCycler::~FocusCycler() {}

void FocusCycler::AddWidget(views::Widget* widget) {
widgets_.push_back(widget);
Expand All @@ -44,14 +38,15 @@ void FocusCycler::RemoveWidget(views::Widget* widget) {
}

void FocusCycler::RotateFocus(Direction direction) {
aura::Window* window = ash::wm::GetActiveWindow();
WmWindow* window = WmShell::Get()->GetActiveWindow();
if (window) {
views::Widget* widget = views::Widget::GetWidgetForNativeWindow(window);
views::Widget* widget = window->GetInternalWidget();
// First try to rotate focus within the active widget. If that succeeds,
// we're done.
if (widget && widget->GetFocusManager()->RotatePaneFocus(
direction == BACKWARD ?
views::FocusManager::kBackward : views::FocusManager::kForward,
if (widget &&
widget->GetFocusManager()->RotatePaneFocus(
direction == BACKWARD ? views::FocusManager::kBackward
: views::FocusManager::kForward,
views::FocusManager::kNoWrap)) {
return;
}
Expand Down Expand Up @@ -85,21 +80,20 @@ void FocusCycler::RotateFocus(Direction direction) {
if (index == browser_index) {
// Activate the most recently active browser window.
MruWindowTracker::WindowList mru_windows(
Shell::GetInstance()->mru_window_tracker()->BuildMruWindowList());
WmShell::Get()->GetMruWindowTracker()->BuildMruWindowList());
if (mru_windows.empty())
break;
WmWindow* window = mru_windows.front();
window->GetWindowState()->Activate();
views::Widget* widget = views::Widget::GetWidgetForNativeWindow(
WmWindowAura::GetAuraWindow(window));
views::Widget* widget = window->GetInternalWidget();
if (!widget)
break;
views::FocusManager* focus_manager = widget->GetFocusManager();
focus_manager->ClearFocus();
focus_manager->RotatePaneFocus(
direction == BACKWARD ?
views::FocusManager::kBackward : views::FocusManager::kForward,
views::FocusManager::kWrap);
focus_manager->RotatePaneFocus(direction == BACKWARD
? views::FocusManager::kBackward
: views::FocusManager::kForward,
views::FocusManager::kWrap);
break;
} else {
if (FocusWidget(widgets_[index]))
Expand Down
7 changes: 3 additions & 4 deletions ash/focus_cycler.h → ash/common/focus_cycler.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FOCUS_CYCLER_H_
#define FOCUS_CYCLER_H_
#ifndef ASH_COMMON_FOCUS_CYCLER_H_
#define ASH_COMMON_FOCUS_CYCLER_H_

#include <vector>

#include "ash/ash_export.h"
#include "base/compiler_specific.h"
#include "base/macros.h"

namespace views {
Expand Down Expand Up @@ -57,4 +56,4 @@ class ASH_EXPORT FocusCycler {

} // namespace ash

#endif // FOCUS_CYCLER_H_
#endif // ASH_COMMON_FOCUS_CYCLER_H_
5 changes: 4 additions & 1 deletion ash/common/wm_shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "ash/common/wm_shell.h"

#include "ash/common/focus_cycler.h"
#include "ash/common/system/tray/system_tray_delegate.h"
#include "ash/common/system/tray/wm_system_tray_notifier.h"
#include "base/logging.h"
Expand All @@ -23,7 +24,9 @@ WmShell* WmShell::Get() {
return instance_;
}

WmShell::WmShell() : system_tray_notifier_(new WmSystemTrayNotifier) {}
WmShell::WmShell()
: focus_cycler_(new FocusCycler),
system_tray_notifier_(new WmSystemTrayNotifier) {}

WmShell::~WmShell() {}

Expand Down
4 changes: 4 additions & 0 deletions ash/common/wm_shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class Rect;
namespace ash {

class AccessibilityDelegate;
class FocusCycler;
class MruWindowTracker;
class SessionStateDelegate;
class ShellObserver;
Expand All @@ -43,6 +44,8 @@ class ASH_EXPORT WmShell {
static WmShell* Get();
static bool HasInstance() { return instance_ != nullptr; }

FocusCycler* focus_cycler() { return focus_cycler_.get(); }

WmSystemTrayNotifier* system_tray_notifier() {
return system_tray_notifier_.get();
}
Expand Down Expand Up @@ -140,6 +143,7 @@ class ASH_EXPORT WmShell {

static WmShell* instance_;

std::unique_ptr<FocusCycler> focus_cycler_;
std::unique_ptr<WmSystemTrayNotifier> system_tray_notifier_;
std::unique_ptr<SystemTrayDelegate> system_tray_delegate_;
};
Expand Down
9 changes: 9 additions & 0 deletions ash/common/wm_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ namespace ui {
class Layer;
}

namespace views {
class Widget;
}

namespace ash {

class WmLayoutManager;
Expand Down Expand Up @@ -203,6 +207,11 @@ class ASH_EXPORT WmWindow {
virtual void Hide() = 0;
virtual void Show() = 0;

// Returns the widget associated with this window, or null if not associated
// with a widget. Only ash system UI widgets are returned, not widgets created
// by the mus window manager code to show a non-client frame.
virtual views::Widget* GetInternalWidget() = 0;

// Requests the window to close and destroy itself. This is intended to
// forward to an associated widget.
virtual void CloseWidget() = 0;
Expand Down
15 changes: 7 additions & 8 deletions ash/focus_cycler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ash/focus_cycler.h"
#include "ash/common/focus_cycler.h"

#include <memory>

Expand All @@ -11,8 +11,6 @@
#include "ash/common/wm_window.h"
#include "ash/shelf/shelf.h"
#include "ash/shelf/shelf_widget.h"
#include "ash/shell.h"
#include "ash/shell_factory.h"
#include "ash/system/status_area_widget.h"
#include "ash/system/status_area_widget_delegate.h"
#include "ash/system/tray/system_tray.h"
Expand Down Expand Up @@ -61,6 +59,8 @@ class PanedWidgetDelegate : public views::WidgetDelegate {

} // namespace

// TODO(jamescook): Migrate this test to //ash/common after the status area
// widget moves. http://crbug.com/620955
class FocusCyclerTest : public AshTestBase {
public:
FocusCyclerTest() {}
Expand Down Expand Up @@ -387,8 +387,7 @@ TEST_F(FocusCyclerTest, CycleFocusThroughWindowWithPanes) {

// Pressing "Escape" while on the status area should
// deactivate it, and activate the browser window.
aura::Window* root = Shell::GetPrimaryRootWindow();
ui::test::EventGenerator event_generator(root, root);
ui::test::EventGenerator& event_generator = GetEventGenerator();
event_generator.PressKey(ui::VKEY_ESCAPE, 0);
EXPECT_TRUE(wm::IsActiveWindow(browser_window));
EXPECT_EQ(focus_manager->GetFocusedView(), view1);
Expand Down Expand Up @@ -418,14 +417,14 @@ TEST_F(FocusCyclerTest, RemoveWidgetOnDisplayRemoved) {
EXPECT_TRUE(wm::IsActiveWindow(window.get()));

// Cycle focus to the status area.
Shell::GetInstance()->focus_cycler()->RotateFocus(FocusCycler::FORWARD);
WmShell::Get()->focus_cycler()->RotateFocus(FocusCycler::FORWARD);
EXPECT_FALSE(wm::IsActiveWindow(window.get()));

// Cycle focus to the shelf.
Shell::GetInstance()->focus_cycler()->RotateFocus(FocusCycler::FORWARD);
WmShell::Get()->focus_cycler()->RotateFocus(FocusCycler::FORWARD);

// Cycle focus should go back to the browser.
Shell::GetInstance()->focus_cycler()->RotateFocus(FocusCycler::FORWARD);
WmShell::Get()->focus_cycler()->RotateFocus(FocusCycler::FORWARD);
EXPECT_TRUE(wm::IsActiveWindow(window.get()));
}

Expand Down
8 changes: 8 additions & 0 deletions ash/mus/bridge/wm_window_mus.cc
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,14 @@ void WmWindowMus::Show() {
window_->SetVisible(true);
}

views::Widget* WmWindowMus::GetInternalWidget() {
// Don't return the window frame widget for an embedded client window.
if (widget_creation_type_ == WidgetCreationType::FOR_CLIENT)
return nullptr;

return widget_;
}

void WmWindowMus::CloseWidget() {
DCHECK(widget_);
// Allow the client to service the close request for remote widgets.
Expand Down
2 changes: 1 addition & 1 deletion ash/mus/bridge/wm_window_mus.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ class WmWindowMus : public WmWindow, public ::mus::WindowObserver {
// The window manager creates a mus::Window and a views::Widget to show the
// non-client frame decorations. In this case the creation type is
// FOR_CLIENT.

FOR_CLIENT,
};

Expand Down Expand Up @@ -176,6 +175,7 @@ class WmWindowMus : public WmWindow, public ::mus::WindowObserver {
bool IsAlwaysOnTop() const override;
void Hide() override;
void Show() override;
views::Widget* GetInternalWidget() override;
void CloseWidget() override;
bool IsFocused() const override;
bool IsActive() const override;
Expand Down
2 changes: 1 addition & 1 deletion ash/root_window_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ash/aura/wm_shelf_aura.h"
#include "ash/aura/wm_window_aura.h"
#include "ash/common/ash_constants.h"
#include "ash/common/focus_cycler.h"
#include "ash/common/root_window_controller_common.h"
#include "ash/common/session/session_state_delegate.h"
#include "ash/common/shelf/shelf_types.h"
Expand All @@ -33,7 +34,6 @@
#include "ash/desktop_background/desktop_background_widget_controller.h"
#include "ash/desktop_background/user_wallpaper_delegate.h"
#include "ash/display/display_manager.h"
#include "ash/focus_cycler.h"
#include "ash/high_contrast/high_contrast_controller.h"
#include "ash/host/ash_window_tree_host.h"
#include "ash/root_window_settings.h"
Expand Down
1 change: 0 additions & 1 deletion ash/shelf/shelf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "ash/common/shelf/shelf_model.h"
#include "ash/common/shelf/wm_shelf_util.h"
#include "ash/common/shell_window_ids.h"
#include "ash/focus_cycler.h"
#include "ash/root_window_controller.h"
#include "ash/screen_util.h"
#include "ash/shelf/shelf_delegate.h"
Expand Down
Loading

0 comments on commit 26f3209

Please sign in to comment.