Revert 113177 - process all ui events before posting closure because menu depends on ui events to...

Revert 113177 - process all ui events before posting closure because menu depends on ui events to quit.
 Add new RunClosureAfterAllPendingUIEvents for toolkit views.

BUG=104359
TEST=BookmarkBarViewTest in interactive_ui_tests will pass


Review URL: http://codereview.chromium.org/8799020

Reason for revert: http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromeOS%20Aura/builds/1514

TBR=oshima@chromium.org
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113183

[Jói, 2011-12-06 11:36:07]

Oshima-san, the change was causing a test failure on ChromeOS Aura,
see
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromeOS%20A....

Please re-land once it passes the linux_chromeos_aura trybot.

Cheers,
Jói


On Tue, Dec 6, 2011 at 11:31 AM,  <joi@chromium.org> wrote:
> Reviewers: oshima,
>
> Description:
> Revert 113177 - process all ui events before posting closure because menu
> depends on ui events to quit.
>  Add new RunClosureAfterAllPendingUIEvents for toolkit views.
>
> BUG=104359
> TEST=BookmarkBarViewTest in interactive_ui_tests will pass
>
>
> Review URL: http://codereview.chromium.org/8799020
>
> Reason for revert:
>
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromeOS%20A...
>
> TBR=oshima@chromium.org
>
> Please review this at http://codereview.chromium.org/8821012/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     chrome/browser/automation/ui_controls.h
>  M     chrome/browser/automation/ui_controls_aurawin.cc
>  M     chrome/browser/automation/ui_controls_aurax11.cc
>  M     chrome/browser/automation/ui_controls_gtk.cc
>  M     chrome/browser/automation/ui_controls_internal.h
>  M     chrome/browser/automation/ui_controls_win.cc
>  M     chrome/test/base/view_event_test_base.cc
>
>
> Index: chrome/browser/automation/ui_controls.h
> ===================================================================
> --- chrome/browser/automation/ui_controls.h     (revision 113182)
> +++ chrome/browser/automation/ui_controls.h     (working copy)
> @@ -97,11 +97,6 @@
>     int state,
>     const base::Closure& task);
>
> -#if defined(TOOLKIT_VIEWS)
> -// Runs |closure| after processing all pending ui events.
> -void RunClosureAfterAllPendingUIEvents(const base::Closure& closure);
> -#endif
> -
>  }  // ui_controls
>
>  #endif  // CHROME_BROWSER_AUTOMATION_UI_CONTROLS_H_
> Index: chrome/browser/automation/ui_controls_aurawin.cc
> ===================================================================
> --- chrome/browser/automation/ui_controls_aurawin.cc    (revision 113182)
> +++ chrome/browser/automation/ui_controls_aurawin.cc    (working copy)
> @@ -5,7 +5,6 @@
>  #include "chrome/browser/automation/ui_controls.h"
>
>  #include "base/logging.h"
> -#include "base/message_loop.h"
>  #include "chrome/browser/automation/ui_controls_internal.h"
>  #include "ui/aura/desktop.h"
>  #include "ui/views/view.h"
> @@ -70,9 +69,4 @@
>   SendMouseEventsNotifyWhenDone(button, state, task);
>  }
>
> -void RunClosureAfterAllPendingUIEvents(const base::Closure& task) {
> -  // On windows, posting UI events is synchronous so just post the closure.
> -  MessageLoopForUI::current()->PostTask(FROM_HERE, task);
> -}
> -
>  }  // namespace ui_controls
> Index: chrome/browser/automation/ui_controls_aurax11.cc
> ===================================================================
> --- chrome/browser/automation/ui_controls_aurax11.cc    (revision 113182)
> +++ chrome/browser/automation/ui_controls_aurax11.cc    (working copy)
> @@ -70,6 +70,22 @@
>       event->xclient.message_type == MarkerEventAtom();
>  }
>
> +void RunClosureAfterEvents(const base::Closure closure) {
> +  if (closure.is_null())
> +    return;
> +  static XEvent* marker_event = NULL;
> +  if (!marker_event) {
> +    marker_event = new XEvent();
> +    marker_event->xclient.type = ClientMessage;
> +    marker_event->xclient.display = NULL;
> +    marker_event->xclient.window = None;
> +    marker_event->xclient.format = 8;
> +  }
> +  marker_event->xclient.message_type = MarkerEventAtom();
> +  aura::Desktop::GetInstance()->PostNativeEvent(marker_event);
> +  new EventWaiter(closure, &Matcher);
> +}
> +
>  }  // namespace
>
>  namespace ui_controls {
> @@ -117,7 +133,9 @@
>     SetMaskAndKeycodeThenSend(&xevent, ShiftMask, XK_Shift_L);
>   if (alt)
>     SetMaskAndKeycodeThenSend(&xevent, Mod1Mask, XK_Alt_L);
> -  xevent.xkey.keycode = ui::XKeysymForWindowsKeyCode(key, shift);
> +  xevent.xkey.keycode =
> +      XKeysymToKeycode(base::MessagePumpX::GetDefaultXDisplay(),
> +                       ui::XKeysymForWindowsKeyCode(key, shift));
>   aura::Desktop::GetInstance()->PostNativeEvent(&xevent);
>
>   // Send key release events.
> @@ -130,7 +148,7 @@
>   if (control)
>     SetKeycodeAndSendThenUnmask(&xevent, ControlMask, XK_Control_L);
>   DCHECK(!xevent.xkey.state);
> -  RunClosureAfterAllPendingUIEvents(closure);
> +  RunClosureAfterEvents(closure);
>   return true;
>  }
>
> @@ -147,7 +165,7 @@
>   xmotion->same_screen = True;
>   // Desktop will take care of other necessary fields.
>   aura::Desktop::GetInstance()->PostNativeEvent(&xevent);
> -  RunClosureAfterAllPendingUIEvents(closure);
> +  RunClosureAfterEvents(closure);
>   return false;
>  }
>
> @@ -190,7 +208,7 @@
>     xevent.xbutton.type = ButtonRelease;
>     desktop->PostNativeEvent(&xevent);
>   }
> -  RunClosureAfterAllPendingUIEvents(closure);
> +  RunClosureAfterEvents(closure);
>   return false;
>  }
>
> @@ -208,20 +226,4 @@
>   SendMouseEventsNotifyWhenDone(button, state, closure);
>  }
>
> -void RunClosureAfterAllPendingUIEvents(const base::Closure& closure) {
> -  if (closure.is_null())
> -    return;
> -  static XEvent* marker_event = NULL;
> -  if (!marker_event) {
> -    marker_event = new XEvent();
> -    marker_event->xclient.type = ClientMessage;
> -    marker_event->xclient.display = NULL;
> -    marker_event->xclient.window = None;
> -    marker_event->xclient.format = 8;
> -  }
> -  marker_event->xclient.message_type = MarkerEventAtom();
> -  aura::Desktop::GetInstance()->PostNativeEvent(marker_event);
> -  new EventWaiter(closure, &Matcher);
> -}
> -
>  }  // namespace ui_controls
> Index: chrome/browser/automation/ui_controls_gtk.cc
> ===================================================================
> --- chrome/browser/automation/ui_controls_gtk.cc        (revision 113182)
> +++ chrome/browser/automation/ui_controls_gtk.cc        (working copy)
> @@ -310,13 +310,4 @@
>  }
>  #endif
>
> -#if defined(TOOLKIT_VIEWS)
> -void RunClosureAfterAllPendingUIEvents(const base::Closure& task) {
> -  // Send noop event and run task.
> -  int x, y;
> -  gdk_window_at_pointer(&x, &y);
> -  SendMouseMoveNotifyWhenDone(x, y, task);
> -}
> -#endif
> -
>  }  // namespace ui_controls
> Index: chrome/browser/automation/ui_controls_internal.h
> ===================================================================
> --- chrome/browser/automation/ui_controls_internal.h    (revision 113182)
> +++ chrome/browser/automation/ui_controls_internal.h    (working copy)
> @@ -27,7 +27,6 @@
>  bool SendMouseEventsImpl(MouseButton type,
>                          int state,
>                          const base::Closure& task);
> -void RunClosureAfterAllPendingUITasksImpl(const base::Closure& task);
>  #endif
>
>  }  // namespace internal
> Index: chrome/browser/automation/ui_controls_win.cc
> ===================================================================
> --- chrome/browser/automation/ui_controls_win.cc        (revision 113182)
> +++ chrome/browser/automation/ui_controls_win.cc        (working copy)
> @@ -5,7 +5,6 @@
>  #include "chrome/browser/automation/ui_controls.h"
>
>  #include "base/callback.h"
> -#include "base/message_loop.h"
>  #include "chrome/browser/automation/ui_controls_internal.h"
>  #include "ui/gfx/point.h"
>  #include "ui/views/view.h"
> @@ -66,9 +65,4 @@
>   SendMouseEventsNotifyWhenDone(button, state, task);
>  }
>
> -void RunClosureAfterAllPendingUIEvents(const base::Closure& closure) {
> -  // On windows, posting UI events is synchronous so just post the closure.
> -  MessageLoopForUI::current()->PostTask(FROM_HERE, closure);
> -}
> -
>  }  // ui_controls
> Index: chrome/test/base/view_event_test_base.cc
> ===================================================================
> --- chrome/test/base/view_event_test_base.cc    (revision 113182)
> +++ chrome/test/base/view_event_test_base.cc    (working copy)
> @@ -70,11 +70,9 @@
>   PostMessage(window_->GetNativeWindow(), WM_USER, 0, 0);
>  #endif
>
> -  // If we're in a nested message loop, as is the case with menus, we
> -  // need to quit twice. The second quit does that for us. Finish all
> -  // pending UI events before posting closure because events it may be
> -  // executed before UI events are executed.
> -
>  ui_controls::RunClosureAfterAllPendingUIEvents(MessageLoop::QuitClosure());
> +  // If we're in a nested message loop, as is the case with menus, we need
> +  // to quit twice. The second quit does that for us.
> +  MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
>  }
>
>  void ViewEventTestBase::SetUp() {
>
>