Fix the flaky full screen video tests by providing our own clickView;

Fix the flaky full screen video tests by providing our own clickView;


BUG=224923
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192536

[mkosiba (inactive), 2013-04-03 17:44:51]

thanks for following through!

https://codereview.chromium.org/13510002/diff/7001/android_webview/javatests/...
File
android_webview/javatests/src/org/chromium/android_webview/test/util/AwTouchUtils.java
(right):

https://codereview.chromium.org/13510002/diff/7001/android_webview/javatests/...
android_webview/javatests/src/org/chromium/android_webview/test/util/AwTouchUtils.java:25:
public static void clickView(InstrumentationTestCase test, View v) {
this looks like reimplementing a bunch of stuff already done in
org.chromium.content.browser.test.util.TouchCommon
How hard would it be to reuse TouchCommon to implement AwTouchUtils?

[michaelbai, 2013-04-03 19:15:39]

https://codereview.chromium.org/13510002/diff/7001/android_webview/javatests/...
File
android_webview/javatests/src/org/chromium/android_webview/test/util/AwTouchUtils.java
(right):

https://codereview.chromium.org/13510002/diff/7001/android_webview/javatests/...
android_webview/javatests/src/org/chromium/android_webview/test/util/AwTouchUtils.java:25:
public static void clickView(InstrumentationTestCase test, View v) {

I am not aware there is TouchCommon, took advantage of it now.


On 2013/04/03 17:44:51, Martin Kosiba wrote:
> this looks like reimplementing a bunch of stuff already done in
> org.chromium.content.browser.test.util.TouchCommon
> How hard would it be to reuse TouchCommon to implement AwTouchUtils?

[mkosiba (inactive), 2013-04-04 08:56:44]

lgtm

https://codereview.chromium.org/13510002/diff/15001/android_webview/javatests...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientGetVideoLoadingProgressViewTest.java
(right):

https://codereview.chromium.org/13510002/diff/15001/android_webview/javatests...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientGetVideoLoadingProgressViewTest.java:70:
Thread.sleep(5 * 1000);
I know the sleep was here before, but why do we need it? Maybe add a comment?

https://codereview.chromium.org/13510002/diff/15001/android_webview/javatests...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientGetVideoLoadingProgressViewTest.java:71:
TouchCommon touchCommon = new TouchCommon(this);
nit: move the instantiation out of the try block (to line 64 for example)

[michaelbai, 2013-04-04 16:57:10]

https://codereview.chromium.org/13510002/diff/15001/android_webview/javatests...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientGetVideoLoadingProgressViewTest.java
(right):

https://codereview.chromium.org/13510002/diff/15001/android_webview/javatests...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientGetVideoLoadingProgressViewTest.java:70:
Thread.sleep(5 * 1000);
It is used to wait the UI show up.

On 2013/04/04 08:56:44, Martin Kosiba wrote:
> I know the sleep was here before, but why do we need it? Maybe add a comment?

https://codereview.chromium.org/13510002/diff/15001/android_webview/javatests...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientGetVideoLoadingProgressViewTest.java:71:
TouchCommon touchCommon = new TouchCommon(this);
since it is only use once, I want to declare it at where it actually used.

On 2013/04/04 08:56:44, Martin Kosiba wrote:
> nit: move the instantiation out of the try block (to line 64 for example)

[michaelbai, 2013-04-04 16:59:16]

Yaron@ for your owner approval of content/public/test/android/

[Yaron, 2013-04-04 17:58:32]

lgtm

https://codereview.chromium.org/13510002/diff/15001/content/public/test/andro...
File
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchCommon.java
(right):

https://codereview.chromium.org/13510002/diff/15001/content/public/test/andro...
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchCommon.java:123:
public void singleClickView(View v) {
Nit: rename singleClickViewInCenter

https://codereview.chromium.org/13510002/diff/15001/content/public/test/andro...
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchCommon.java:124:
singleClickView(v, v.getWidth()/2, v.getHeight()/2);
add spaces around "/"

[michaelbai, 2013-04-04 18:16:41]

https://codereview.chromium.org/13510002/diff/15001/content/public/test/andro...
File
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchCommon.java
(right):

https://codereview.chromium.org/13510002/diff/15001/content/public/test/andro...
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchCommon.java:123:
public void singleClickView(View v) {
I think it might not necessary, if the position can not be assigned, the center
of view is straightforward choice.

In another hand, it is not important for this method to click the center of view
or not.
On 2013/04/04 17:58:32, Yaron wrote:
> Nit: rename singleClickViewInCenter

https://codereview.chromium.org/13510002/diff/15001/content/public/test/andro...
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchCommon.java:124:
singleClickView(v, v.getWidth()/2, v.getHeight()/2);
On 2013/04/04 17:58:32, Yaron wrote:
> add spaces around "/"

Done.

[commit-bot: I haz the power, 2013-04-05 07:05:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelbai@chromium.org/13510002/26001

[commit-bot: I haz the power, 2013-04-05 09:55:39]

Change committed as 192536