Windows: Fix focus/activation handling and keyboard input (issue #1700)

libcef/browser/browser_host_impl_win.cc

@@ -864,10 +864,34 @@ void CefBrowserHostImpl::PlatformSetFocus(bool focus) {
   }
 
   if (window_widget_) {
-    // Give native focus to the DesktopNativeWidgetAura for the root window.
+    // Give native focus to the DesktopWindowTreeHostWin associated with the
-    // Needs to be done via the HWND so that keyboard focus is assigned
+    // root window.
-    // correctly. DesktopNativeWidgetAura will update focus state on the
+    //
-    // aura::Window when WM_SETFOCUS and WM_KILLFOCUS are received.
+    // The DesktopWindowTreeHostWin HandleNativeFocus/HandleNativeBlur methods
+    // are called in response to WM_SETFOCUS/WM_KILLFOCUS respectively. The
+    // implementation has been patched to call HandleActivationChanged which
+    // results in the following behaviors:
+    // 1. Update focus/activation state of the aura::Window indirectly via
+    //    wm::FocusController. This allows focus-related behaviors (e.g. focus
+    //    rings, flashing caret, onFocus/onBlur JS events, etc.) to work as
+    //    expected (see issue #1677).
+    // 2. Update focus state of the ui::InputMethod. If this does not occur
+    //    then InputMethodBase::GetTextInputClient will return NULL and
+    //    InputMethodWin::OnChar will fail to sent character events to the
+    //    renderer (see issue #1700).
+    //
+    // This differs from activation in Chrome which is handled via
+    // HWNDMessageHandler::PostProcessActivateMessage (Widget::Show indirectly
+    // calls HWNDMessageHandler::Activate which calls ::SetForegroundWindow
+    // resulting in a WM_ACTIVATE message being sent to the window). The Chrome
+    // code path doesn't work for CEF because IsTopLevelWindow in
+    // hwnd_message_handler.cc will return false and consequently
+    // HWNDMessageHandler::PostProcessActivateMessage will not be called.
+    //
+    // Activation events are usually reserved for the top-level window so
+    // triggering activation based on focus events may be incorrect in some
+    // circumstances. Revisit this implementation if additional problems are
+    // discovered.
     ::SetFocus(HWNDForWidget(window_widget_));
   }
 }

libcef/browser/window_delegate_view.cc

@@ -41,6 +41,13 @@ void CefWindowDelegateView::Init(
   params.opacity = views::Widget::InitParams::OPAQUE_WINDOW;
   // Tell Aura not to draw the window frame on resize.
   params.remove_standard_frame = true;
+#if defined(OS_WIN)
+  // Cause WidgetDelegate::CanActivate to return true. See comments in
+  // CefBrowserHostImpl::PlatformSetFocus.
+  // TODO(cef): Do we need similar logic on Linux for proper activation/focus
+  // handling?
+  params.activatable = views::Widget::InitParams::ACTIVATABLE_YES;
+#endif
 
   // Results in a call to InitContent().
   widget->Init(params);
@@ -49,6 +56,10 @@ void CefWindowDelegateView::Init(
   DCHECK_EQ(widget, GetWidget());
   // |widget| must be top-level for focus handling to work correctly.
   DCHECK(widget->is_top_level());
+#if defined(OS_WIN)
+  // |widget| must be activatable for focus handling to work correctly.
+  DCHECK(widget->widget_delegate()->CanActivate());
+#endif
 }
 
 void CefWindowDelegateView::InitContent() {

patch/patch.cfg

@@ -53,8 +53,9 @@ patches = [
   {
     # Allow specification of a parent window handle for Widget creation.
     # https://code.google.com/p/chromiumembedded/issues/detail?id=180
-    # Fix focus/blur handling on Windows.
+    # Fix focus/activation handling and keyboard input on Windows.
     # https://bitbucket.org/chromiumembedded/cef/issues/1677
+    # https://bitbucket.org/chromiumembedded/cef/issues/1700
     'name': 'views_widget_180_1677',
     'path': '../ui/views/widget/',
   },
@@ -182,13 +183,6 @@ patches = [
     'name': 'hwnd_message_handler_1481',
     'path': '../ui/views/win/',
   },
-  {
-    # Fix focus/blur handling on Windows.
-    # https://bitbucket.org/chromiumembedded/cef/issues/1677
-    # https://codereview.chromium.org/1135063002 (reverted)
-    'name': 'aura_window_1677',
-    'path': '../ui/aura/',
-  },
   {
     # Fix rendering of the PDF extension with OSR when the device scale factor
     # is not 1.

patch/patches/aura_window_1677.patch

@@ -1,32 +0,0 @@
-diff --git window.cc window.cc
-index 6cde8c2..bdd5093 100644
---- window.cc
-+++ window.cc
-@@ -522,6 +522,12 @@ void Window::Focus() {
-   client->FocusWindow(this);
- }
- 
-+void Window::Blur() {
-+  client::FocusClient* client = client::GetFocusClient(this);
-+  DCHECK(client);
-+  client->FocusWindow(NULL);
-+}
-+
- bool Window::HasFocus() const {
-   client::FocusClient* client = client::GetFocusClient(this);
-   return client && client->GetFocusedWindow() == this;
-diff --git window.h window.h
-index 5586044..53bfd2e 100644
---- window.h
-+++ window.h
-@@ -262,8 +262,9 @@ class AURA_EXPORT Window : public ui::LayerDelegate,
-   // that has a delegate set).  The toplevel window may be |this|.
-   Window* GetToplevelWindow();
- 
--  // Claims focus.
-+  // Claims or relinquishes the claim to focus.
-   void Focus();
-+  void Blur();
- 
-   // Returns true if the Window is currently the focused window.
-   bool HasFocus() const;

patch/patches/views_widget_180_1677.patch

@@ -12,7 +12,7 @@ index a8e088c..838b6a0 100644
    return host ? host->GetAcceleratedWidget() : NULL;
  }
 diff --git desktop_aura/desktop_window_tree_host_win.cc desktop_aura/desktop_window_tree_host_win.cc
-index 5bee7fd..53e6c22 100644
+index 5bee7fd..3d89853 100644
 --- desktop_aura/desktop_window_tree_host_win.cc
 +++ desktop_aura/desktop_window_tree_host_win.cc
 @@ -131,7 +131,9 @@ void DesktopWindowTreeHostWin::Init(aura::Window* content_window,
@@ -26,16 +26,21 @@ index 5bee7fd..53e6c22 100644
      parent_hwnd = params.parent->GetHost()->GetAcceleratedWidget();
  
    message_handler_->set_remove_standard_frame(params.remove_standard_frame);
-@@ -793,10 +795,12 @@ void DesktopWindowTreeHostWin::HandleFrameChanged() {
+@@ -792,11 +794,15 @@ void DesktopWindowTreeHostWin::HandleFrameChanged() {
+ }
  
  void DesktopWindowTreeHostWin::HandleNativeFocus(HWND last_focused_window) {
-   // TODO(beng): inform the native_widget_delegate_.
+-  // TODO(beng): inform the native_widget_delegate_.
-+  GetWidget()->GetNativeWindow()->Focus();
++  // See comments in CefBrowserHostImpl::PlatformSetFocus.
++  if (CanActivate())
++    HandleActivationChanged(true);
  }
  
  void DesktopWindowTreeHostWin::HandleNativeBlur(HWND focused_window) {
-   // TODO(beng): inform the native_widget_delegate_.
+-  // TODO(beng): inform the native_widget_delegate_.
-+  GetWidget()->GetNativeWindow()->Blur();
++  // See comments in CefBrowserHostImpl::PlatformSetFocus.
++  if (CanActivate())
++    HandleActivationChanged(false);
  }
  
  bool DesktopWindowTreeHostWin::HandleMouseEvent(const ui::MouseEvent& event) {