exo: WMHelperMus: Add focus and activation support.

components/exo/wm_helper_mus.cc

Index: components/exo/wm_helper_mus.cc
diff --git a/components/exo/wm_helper_mus.cc b/components/exo/wm_helper_mus.cc
index 315a70c4781bca4cb25adf8bb92b36a54a6012f7..bee8c971ce7edd342e97ad8f56d76789a977775c 100644
--- a/components/exo/wm_helper_mus.cc
+++ b/components/exo/wm_helper_mus.cc
@@ -5,15 +5,40 @@
 #include "components/exo/wm_helper_mus.h"
 
 #include "ash/common/display/display_info.h"
+#include "services/ui/public/cpp/window_tree_client.h"
+#include "ui/aura/client/focus_client.h"
+#include "ui/views/mus/native_widget_mus.h"
+#include "ui/views/mus/window_manager_connection.h"
+#include "ui/views/widget/widget.h"
 
 namespace exo {
+namespace {
+
+aura::Window* GetToplevelAuraWindow(ui::Window* window) {
+  if (!window)

[reveman, 2016/08/22 19:28:38]

nit: I prefer if this check was moved to the caller as I find it surprising that
it's valid behavior to call this with a null input

[Peng, 2016/08/22 20:25:50]

Done.

+    return nullptr;
+  // We never create child ui::Window, so window->parent() should be null.
+  DCHECK(!window->parent());
+  auto* widget = views::NativeWidgetMus::GetWidgetForWindow(window);

[reveman, 2016/08/22 19:28:38]

nit: I prefer to avoid "auto" unless iterator types or similar where it's clear
what the type is and writing the full name provides little value and typically
just makes the code harder to read.

[Peng, 2016/08/22 20:25:50]

Done.

+  if (!widget)
+    return nullptr;
+  return widget->GetNativeWindow();
+}
+}
 
 ////////////////////////////////////////////////////////////////////////////////
 // WMHelperMus, public:
 
-WMHelperMus::WMHelperMus() {}
+WMHelperMus::WMHelperMus()
+    : active_window_(GetActiveWindow()), focused_window_(GetFocusedWindow()) {

[reveman, 2016/08/22 19:28:38]

hm, can we avoid calling virtual functions in the ctor? while the behavior might
be well defined it's not always what the person reading the code expects.

[Peng, 2016/08/22 20:25:50]

Done.

+  views::WindowManagerConnection::Get()->client()->AddObserver(this);
+}
 
-WMHelperMus::~WMHelperMus() {}
+WMHelperMus::~WMHelperMus() {
+  auto* connection = views::WindowManagerConnection::Get();

[reveman, 2016/08/22 19:28:38]

nit: I prefer not using auto here

[Peng, 2016/08/22 20:25:50]

Done.

+  if (connection)

[reveman, 2016/08/22 19:28:38]

do we need this check?

[Peng, 2016/08/22 20:25:50]

Removed it. Done

+    connection->client()->RemoveObserver(this);
+}
 
 ////////////////////////////////////////////////////////////////////////////////
 // WMHelperMus, private:
@@ -29,13 +54,17 @@ aura::Window* WMHelperMus::GetContainer(int container_id) {
 }
 
 aura::Window* WMHelperMus::GetActiveWindow() const {
-  NOTIMPLEMENTED();
-  return nullptr;
+  ui::Window* window =
+      views::WindowManagerConnection::Get()->client()->GetFocusedWindow();
+  return GetToplevelAuraWindow(window);
 }
 
 aura::Window* WMHelperMus::GetFocusedWindow() const {
-  NOTIMPLEMENTED();
-  return nullptr;
+  auto* active_window = GetActiveWindow();

[reveman, 2016/08/22 19:28:38]

nit: I prefer not using auto here

[Peng, 2016/08/22 20:25:50]

Done.

+  if (!active_window)
+    return nullptr;
+  auto* focus_client = aura::client::GetFocusClient(active_window);

[reveman, 2016/08/22 19:28:38]

nit: I prefer not using auto here

[Peng, 2016/08/22 20:25:50]

Done.

+  return focus_client->GetFocusedWindow();
 }
 
 ui::CursorSetType WMHelperMus::GetCursorSet() const {
@@ -68,4 +97,37 @@ bool WMHelperMus::IsMaximizeModeWindowManagerEnabled() const {
   return false;
 }
 
+void WMHelperMus::OnWindowTreeFocusChanged(ui::Window* gained_focus,
+                                           ui::Window* lost_focus) {
+  aura::Window* gained_active = GetToplevelAuraWindow(gained_focus);
+  aura::Window* lost_active = GetToplevelAuraWindow(lost_focus);
+
+  OnWindowFocused(nullptr, focused_window_);

[reveman, 2016/08/22 19:28:38]

why is this called twice with null as temporarily focus instead of just one time
with old and new focus window?

[Peng, 2016/08/22 20:25:50]

Done.

+
+  // Because NativeWidgetMus uses separate FocusClient for every toplevel
+  // window, we have to stop observering the FocusClient of the |lost_active|
+  // and start observering the FocusClient of the |gained_active|.
+  if (active_window_) {
+    auto* focus_client = aura::client::GetFocusClient(active_window_);

[reveman, 2016/08/22 19:28:38]

nit: I prefer not using auto here

[Peng, 2016/08/22 20:25:50]

Done.

+    focus_client->RemoveObserver(this);
+  }
+
+  active_window_ = gained_active;
+  NotifyWindowActivated(gained_active, lost_active);
+
+  if (active_window_) {
+    auto* focus_client = aura::client::GetFocusClient(active_window_);

[reveman, 2016/08/22 19:28:38]

nit: I prefer not using auto here

[Peng, 2016/08/22 20:25:50]

Done.

+    focus_client->AddObserver(this);
+    OnWindowFocused(focus_client->GetFocusedWindow(), nullptr);
+  }
+}
+
+void WMHelperMus::OnWindowFocused(aura::Window* gained_focus,
+                                  aura::Window* lost_focus) {
+  if (focused_window_ != gained_focus) {
+    focused_window_ = gained_focus;
+    NotifyWindowFocused(gained_focus, lost_focus);
+  }
+}
+
 }  // namespace exo