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

Revert 113183 - 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
Reason for un-revert: The test succeeded before the original revert, looks like it's just flaky.

TBR=oshima@chromium.org
Review URL: http://codereview.chromium.org/8821012

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

[Jói, 2011-12-06 12:25:02]

Oshima-san, looks like the test failure was just a flake (even though
there were two in a row), so I'm re-landing this for you.

Cheers,
Jói


On Tue, Dec 6, 2011 at 12:23 PM,  <joi@chromium.org> wrote:
> Reviewers: Jói,
>
> Description:
> Revert 113183 - 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...
> Reason for un-revert: The test succeeded before the original revert, looks
> like
> it's just flaky.
>
> TBR=oshima@chromium.org
> Review URL: http://codereview.chromium.org/8821012
>
> TBR=joi@chromium.org
>
> Please review this at http://codereview.chromium.org/8820010/
>
> 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 113187)
> +++ chrome/browser/automation/ui_controls.h     (working copy)
> @@ -97,6 +97,11 @@
>     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 113187)
> +++ chrome/browser/automation/ui_controls_aurawin.cc    (working copy)
> @@ -5,6 +5,7 @@
>  #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"
> @@ -69,4 +70,9 @@
>   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 113187)
> +++ chrome/browser/automation/ui_controls_aurax11.cc    (working copy)
> @@ -70,22 +70,6 @@
>       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 {
> @@ -133,9 +117,7 @@
>     SetMaskAndKeycodeThenSend(&xevent, ShiftMask, XK_Shift_L);
>   if (alt)
>     SetMaskAndKeycodeThenSend(&xevent, Mod1Mask, XK_Alt_L);
> -  xevent.xkey.keycode =
> -      XKeysymToKeycode(base::MessagePumpX::GetDefaultXDisplay(),
> -                       ui::XKeysymForWindowsKeyCode(key, shift));
> +  xevent.xkey.keycode = ui::XKeysymForWindowsKeyCode(key, shift);
>   aura::Desktop::GetInstance()->PostNativeEvent(&xevent);
>
>   // Send key release events.
> @@ -148,7 +130,7 @@
>   if (control)
>     SetKeycodeAndSendThenUnmask(&xevent, ControlMask, XK_Control_L);
>   DCHECK(!xevent.xkey.state);
> -  RunClosureAfterEvents(closure);
> +  RunClosureAfterAllPendingUIEvents(closure);
>   return true;
>  }
>
> @@ -165,7 +147,7 @@
>   xmotion->same_screen = True;
>   // Desktop will take care of other necessary fields.
>   aura::Desktop::GetInstance()->PostNativeEvent(&xevent);
> -  RunClosureAfterEvents(closure);
> +  RunClosureAfterAllPendingUIEvents(closure);
>   return false;
>  }
>
> @@ -208,7 +190,7 @@
>     xevent.xbutton.type = ButtonRelease;
>     desktop->PostNativeEvent(&xevent);
>   }
> -  RunClosureAfterEvents(closure);
> +  RunClosureAfterAllPendingUIEvents(closure);
>   return false;
>  }
>
> @@ -226,4 +208,20 @@
>   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 113187)
> +++ chrome/browser/automation/ui_controls_gtk.cc        (working copy)
> @@ -310,4 +310,13 @@
>  }
>  #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 113187)
> +++ chrome/browser/automation/ui_controls_internal.h    (working copy)
> @@ -27,6 +27,7 @@
>  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 113187)
> +++ chrome/browser/automation/ui_controls_win.cc        (working copy)
> @@ -5,6 +5,7 @@
>  #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"
> @@ -65,4 +66,9 @@
>   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 113187)
> +++ chrome/test/base/view_event_test_base.cc    (working copy)
> @@ -70,9 +70,11 @@
>   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.
> -  MessageLoop::current()->PostTask(FROM_HERE, 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. Finish all
> +  // pending UI events before posting closure because events it may be
> +  // executed before UI events are executed.
> +
>  ui_controls::RunClosureAfterAllPendingUIEvents(MessageLoop::QuitClosure());
>  }
>
>  void ViewEventTestBase::SetUp() {
>
>