Avoid blocking while mapping an X11 window

chrome/browser/extensions/extension_commands_global_registry_apitest.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "build/build_config.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/window_controller.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/test/result_catcher.h"
 #include "ui/base/base_window.h"
 #include "ui/base/test/ui_controls.h"
 
 #if defined(OS_LINUX) && defined(USE_X11)
 #include <X11/Xlib.h>
 #include <X11/extensions/XTest.h>
 #include <X11/keysym.h>
-
+#include "ui/aura/window.h"
 #include "ui/events/keycodes/keyboard_code_conversion_x.h"
 #include "ui/gfx/x/x11_types.h"
+#include "ui/views/widget/desktop_aura/desktop_window_tree_host_observer_x11.h"
+#include "ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h"
 #endif
 
 #if defined(OS_MACOSX)
 #include <Carbon/Carbon.h>
 
 #include "base/mac/scoped_cftyperef.h"
 #endif
 
 namespace extensions {
 
@@ skipping 75 matching lines @@
   CGEventPost(event_tap_location, shift_down);
   CGEventPost(event_tap_location, key_down);
   CGEventPost(event_tap_location, key_up);
   CGEventPost(event_tap_location, shift_up);
   CGEventPost(event_tap_location, command_up);
 
   CFRelease(event_source);
 }
 #endif
 
+#if defined(OS_LINUX) && defined(USE_X11)
+class GlobalCommandTreeHostObserver
+  : public views::DesktopWindowTreeHostObserverX11 {
+  public:
+  void OnWindowMapped(unsigned long xid) override {
+    // On Linux, our infrastructure for sending keys just synthesize keyboard
+    // event and send them directly to the specified window, without notifying
+    // the X root window. It didn't work while testing global shortcut because
+    // the stuff of global shortcut on Linux need to be notified when KeyPress
+    // event is happening on X root window. So we simulate the keyboard input
+    // here.
+    SendNativeKeyEventToXDisplay(ui::VKEY_1, true, true, false);
+    SendNativeKeyEventToXDisplay(ui::VKEY_A, true, true, false);
+    SendNativeKeyEventToXDisplay(ui::VKEY_8, true, true, false);
+  }
+
+  void OnWindowUnmapped(unsigned long xid) override {}
+};
+#endif
+
 // Test the basics of global commands and make sure they work when Chrome
 // doesn't have focus. Also test that non-global commands are not treated as
 // global and that keys beyond Ctrl+Shift+[0..9] cannot be auto-assigned by an
 // extension.
 //
 // Doesn't work in GN CrOS ozone builds yet, http://crbug.com/619784
 #if defined(OS_CHROMEOS) && defined(USE_OZONE)
 #define MAYBE_GlobalCommand DISABLED_GlobalCommand
 #else
 #define MAYBE_GlobalCommand GlobalCommand
@@ skipping 20 matching lines @@
   ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
       incognito_browser, ui::VKEY_1, true, true, false, false));
   ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
       incognito_browser, ui::VKEY_A, true, true, false, false));
 
   // Activate the shortcut (Ctrl+Shift+8). This should have an effect.
   ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
       incognito_browser, ui::VKEY_8, true, true, false, false));
 #elif defined(OS_LINUX) && defined(USE_X11)
   // Create an incognito browser to capture the focus.
-  CreateIncognitoBrowser();
+  Browser* incognito_browser = CreateIncognitoBrowser();
 
-  // On Linux, our infrastructure for sending keys just synthesize keyboard
-  // event and send them directly to the specified window, without notifying the
-  // X root window. It didn't work while testing global shortcut because the
-  // stuff of global shortcut on Linux need to be notified when KeyPress event
-  // is happening on X root window. So we simulate the keyboard input here.
-  SendNativeKeyEventToXDisplay(ui::VKEY_1, true, true, false);
-  SendNativeKeyEventToXDisplay(ui::VKEY_A, true, true, false);
-  SendNativeKeyEventToXDisplay(ui::VKEY_8, true, true, false);
+  views::DesktopWindowTreeHostX11* host =

[sky, 2017/02/06 22:25:28]

Your description makes the cases for production code, but in test code can't we
block until mapped?

[Tom (Use chromium acct), 2017/02/06 22:31:08]

We could.  Where would we add such code?

+    static_cast<views::DesktopWindowTreeHostX11*>(
+        incognito_browser->window()->GetNativeWindow()->GetHost());
+
+  auto observer = base::MakeUnique<GlobalCommandTreeHostObserver>();

[danakj, 2017/03/09 20:45:34]

nit: can u pass a lambda (or closure) to the observer that has the actions u
want it to do (the key events) so that code stays down here?

[Tom (Use chromium acct), 2017/03/10 01:29:57]

Done (sort of, I couldn't figure out how to use the lambda).

+  // The observer sends the commands after window mapping
+  host->AddObserver(observer.get());
+
 #elif defined(OS_MACOSX)
   // Create an incognito browser to capture the focus.
   CreateIncognitoBrowser();
 
   // Send some native mac key events.
   SendNativeCommandShift(kVK_ANSI_1);
   SendNativeCommandShift(kVK_ANSI_A);
   SendNativeCommandShift(kVK_ANSI_8);
 #endif
 
   // If this fails, it might be because the global shortcut failed to work,
   // but it might also be because the non-global shortcuts unexpectedly
   // worked.
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(USE_X11)
+  host->RemoveObserver(observer.get());
+#endif
 }
 
 #if defined(OS_WIN)
 // Feature only fully implemented on Windows, other platforms coming.
 // TODO(smus): On mac, SendKeyPress must first support media keys.
 // Test occasionally times out on Windows. http://crbug.com/428813
 #define MAYBE_GlobalDuplicatedMediaKey DISABLED_GlobalDuplicatedMediaKey
 #else
 #define MAYBE_GlobalDuplicatedMediaKey DISABLED_GlobalDuplicatedMediaKey
 #endif
@@ skipping 15 matching lines @@
                             false,
                             false,
                             false);
 
   // We should get two success results.
   ASSERT_TRUE(catcher.GetNextResult());
   ASSERT_TRUE(catcher.GetNextResult());
 }
 
 }  // namespace extensions