GTK: First cut at tab dragging automation....

GTK: First cut at tab dragging automation.

Also make tab dragging slightly more robust.

I tried really hard to avoid hackiness, but after many hours of wrestling with gtk and X, this is the best I could do. The main point of contention is that GTK (and our tab dragging code in particular) seems to be able to get X into a state where gdk_display_warp_pointer() doesn't send back any events (although it does move the X pointer). I tried to fix our code directly, but decided it was GTK that was broken. So I faked some mouse motion events to prod the tab dragging into working. This approach does not appear to be flaky, and is actually closer to the event stream that occurs when a user drags a tab than the obvious approach would be. (The tests themselves are somewhat flaky, but only due to WaitForURLDisplayedForTab() flakiness, which is a separate issue I'll look at later. The tests aren't on any buildbot for now so I'd like to leave them enabled.)

BUG=22182
TEST=--gtest_filter=AutomatedUITest.Drag*


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

[Paweł Hajdan Jr., 2009-09-24 15:38:11]

Drive-by.

http://codereview.chromium.org/218017/diff/1/4
File chrome/browser/automation/automation_provider_gtk.cc (right):

http://codereview.chromium.org/218017/diff/1/4#newcode112
Line 112: DCHECK(reply_message_ != NULL);
Can this DCHECK be moved to the ctor? Let's fail as early as possible. DCHECKing
the provider won't hurt as well.

http://codereview.chromium.org/218017/diff/1/2
File chrome/test/automated_ui_tests/automated_ui_test_interactive_test.cc
(right):

http://codereview.chromium.org/218017/diff/1/2#newcode10
Line 10: #if defined(OS_WINDOWS)
Looks like this MAYBE macro can be now removed.

[sky, 2009-09-24 16:49:13]

LGTM with the following style nits fixed

http://codereview.chromium.org/218017/diff/1/4
File chrome/browser/automation/automation_provider_gtk.cc (right):

http://codereview.chromium.org/218017/diff/1/4#newcode108
Line 108: : provider_(provider), reply_message_(reply_message) {}
nit: style guide says if you wrap the member initializer list onto a new line as
you've done, then you must put each field on a separate line.

http://codereview.chromium.org/218017/diff/1/4#newcode141
Line 141: ui_controls::MouseButtonState state_;
nit: DISALLOW_COPY_AND_ASSIGN

http://codereview.chromium.org/218017/diff/1/4#newcode172
Line 172: if (window && (drag_path.size() > 1)) {
!drag_page.empty()

http://codereview.chromium.org/218017/diff/1/5
File chrome/browser/automation/ui_controls_linux.cc (right):

http://codereview.chromium.org/218017/diff/1/5#newcode281
Line 281: reinterpret_cast<GdkModifierType*>(&event->button.state));
nit: line > 80 chars

[Evan Stade, 2009-09-24 18:02:27]

http://codereview.chromium.org/218017/diff/1/4
File chrome/browser/automation/automation_provider_gtk.cc (right):

http://codereview.chromium.org/218017/diff/1/4#newcode108
Line 108: : provider_(provider), reply_message_(reply_message) {}
On 2009/09/24 16:49:13, sky wrote:
> nit: style guide says if you wrap the member initializer list onto a new line
as
> you've done, then you must put each field on a separate line.

Done.

http://codereview.chromium.org/218017/diff/1/4#newcode112
Line 112: DCHECK(reply_message_ != NULL);
On 2009/09/24 15:38:12, Paweł Hajdan Jr. wrote:
> Can this DCHECK be moved to the ctor? Let's fail as early as possible.
DCHECKing
> the provider won't hurt as well.

Done.

http://codereview.chromium.org/218017/diff/1/4#newcode141
Line 141: ui_controls::MouseButtonState state_;
On 2009/09/24 16:49:13, sky wrote:
> nit: DISALLOW_COPY_AND_ASSIGN

Done.

http://codereview.chromium.org/218017/diff/1/4#newcode172
Line 172: if (window && (drag_path.size() > 1)) {
On 2009/09/24 16:49:13, sky wrote:
> !drag_page.empty()

drag_path.size() > 1, not drag_path.size() > 0

http://codereview.chromium.org/218017/diff/1/5
File chrome/browser/automation/ui_controls_linux.cc (right):

http://codereview.chromium.org/218017/diff/1/5#newcode281
Line 281: reinterpret_cast<GdkModifierType*>(&event->button.state));
On 2009/09/24 16:49:13, sky wrote:
> nit: line > 80 chars

Done.

http://codereview.chromium.org/218017/diff/1/2
File chrome/test/automated_ui_tests/automated_ui_test_interactive_test.cc
(right):

http://codereview.chromium.org/218017/diff/1/2#newcode10
Line 10: #if defined(OS_WINDOWS)
On 2009/09/24 15:38:12, Paweł Hajdan Jr. wrote:
> Looks like this MAYBE macro can be now removed.

Done.

[Paweł Hajdan Jr., 2009-09-24 18:04:58]

http://codereview.chromium.org/218017/diff/1002/1005
File chrome/browser/automation/automation_provider_gtk.cc (right):

http://codereview.chromium.org/218017/diff/1002/1005#newcode111
Line 111: DCHECK(provider);
very-small-nit: Please be consistent and check things in the same order as they
come to the function and are initialized, and either check both member
variables, or both parameters. Currently it's mixed (reply_message_ - member and
provider - parameter).

[sky, 2009-09-24 18:19:02]

> http://codereview.chromium.org/218017/diff/1/4#newcode172
> Line 172: if (window && (drag_path.size() > 1)) {
> On 2009/09/24 16:49:13, sky wrote:
>>
>> !drag_page.empty()
>
> drag_path.size() > 1, not drag_path.size() > 0

My mistake.
Sorry,

  -Scott