BookmarkBarView tests fixes

Relanding:
BookmarkBarView tests fixes
 * Add BOOKMARK_CONTEXT_MENU_SHOWN notification. On Linux, event handling is asynchronous, but the showing context menu is blocking,
    so we can't use RunAllPending().
 * Convert gtk keycode to Win when checking accelerator.

BUG=39736
TEST=none (interactive_ui_tests should pass once enabled
it'll be enabled by http://codereview.chromium.org/1576008

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=43699

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=43700

[sky, 2010-04-02 17:31:09]

http://codereview.chromium.org/1545011/diff/1/2
File chrome/browser/views/bookmark_bar_view_test.cc (right):

http://codereview.chromium.org/1545011/diff/1/2#newcode327
chrome/browser/views/bookmark_bar_view_test.cc:327:
EXPECT_TRUE(menu->GetSubmenu()->IsShowing());
These should be asserts, as otherwise if the menu isn't showing mousemove is
going to try to and move to a view that isn't on screen, which likely completely
fails.

http://codereview.chromium.org/1545011/diff/1/2#newcode385
chrome/browser/views/bookmark_bar_view_test.cc:385:
ContextMenuNotificationObserver(Task* task) : task_(task) {
explicit

http://codereview.chromium.org/1545011/diff/1/2#newcode794
chrome/browser/views/bookmark_bar_view_test.cc:794: NewRunnableMethod(this,
&ViewEventTestBase::Done), 100);
Is there something we can wait for? Using 100 like this is likely to cause
problems on the bots.

http://codereview.chromium.org/1545011/diff/1/4
File chrome/common/notification_type.h (right):

http://codereview.chromium.org/1545011/diff/1/4#newcode928
chrome/common/notification_type.h:928: // tests that the context menu has been
created and shown.
Document the source/details.

http://codereview.chromium.org/1545011/diff/1/5
File views/controls/menu/menu_controller.cc (right):

http://codereview.chromium.org/1545011/diff/1/5#newcode726
views/controls/menu/menu_controller.cc:726: DLOG(WARNING) << "OnKeyDown: " <<
key_code;
Nuke this.

[oshima, 2010-04-02 22:01:55]

http://codereview.chromium.org/1545011/diff/1/2
File chrome/browser/views/bookmark_bar_view_test.cc (right):

http://codereview.chromium.org/1545011/diff/1/2#newcode327
chrome/browser/views/bookmark_bar_view_test.cc:327:
EXPECT_TRUE(menu->GetSubmenu()->IsShowing());
ok, i thought it was just cut&past thing. Reverted.  Is it still okay to use
expect_eq for the last check?

On 2010/04/02 17:31:10, sky wrote:
> These should be asserts, as otherwise if the menu isn't showing mousemove is
> going to try to and move to a view that isn't on screen, which likely
completely
> fails.

http://codereview.chromium.org/1545011/diff/1/2#newcode385
chrome/browser/views/bookmark_bar_view_test.cc:385:
ContextMenuNotificationObserver(Task* task) : task_(task) {
On 2010/04/02 17:31:10, sky wrote:
> explicit

Done.

http://codereview.chromium.org/1545011/diff/1/2#newcode794
chrome/browser/views/bookmark_bar_view_test.cc:794: NewRunnableMethod(this,
&ViewEventTestBase::Done), 100);
This wasn't actually necessary, so removed.

On 2010/04/02 17:31:10, sky wrote:
> Is there something we can wait for? Using 100 like this is likely to cause
> problems on the bots.

This test already has several DelayedTask actually. Looks like it's waiting for
synthesized events to be handled?  If that's the case, we may be able to
eliminate them with RunAllPending?

[sky, 2010-04-02 22:15:53]

LGTM