Add insertion handle tests

Add insertion handle tests

These tests check that an insertion handle properly appears when
clicking on an editable text area, that dragging the handle works, and that 'paste' works.

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

[Yaron, 2012-12-04 21:10:30]

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
File
content/public/android/javatests/src/org/chromium/content/browser/ContentDetectionTestBase.java
(right):

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/ContentDetectionTestBase.java:72:
return getContentView().getContentViewCore();
if this change is going to stick just use mContentView directly

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
File
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java
(right):

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java:19:
public class InsertionHandleTest extends ContentDetectionTestBase {
This isn't a ContentDetection test. Why not just use ContentShellTestBase?

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java:26:
private static final int DRAG_TO_X = 287;
Can you add a brief comment about these magic constants.

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java:158:
int positionX = -1;
nit: rename mPositionX and mPositionY

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java:188:

nit: stray ws

https://codereview.chromium.org/11348361/diff/4001/content/public/test/androi...
File
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchUtils.java
(right):

https://codereview.chromium.org/11348361/diff/4001/content/public/test/androi...
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchUtils.java:85:
public static void sleepForDoubleTapTimeout(Instrumentation instrumentation) {
nit: brief javadoc

https://codereview.chromium.org/11348361/diff/4001/content/public/test/androi...
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchUtils.java:86:
SystemClock.sleep((long)(ViewConfiguration.getDoubleTapTimeout() * 1.5));
WHy 1.5? Just guaranteeing that you don't fall into that interval?

[cjhopman, 2012-12-04 23:07:17]

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
File
content/public/android/javatests/src/org/chromium/content/browser/ContentDetectionTestBase.java
(right):

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/ContentDetectionTestBase.java:72:
return getContentView().getContentViewCore();
On 2012/12/04 21:10:30, Yaron wrote:
> if this change is going to stick just use mContentView directly

Done.

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
File
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java
(right):

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java:19:
public class InsertionHandleTest extends ContentDetectionTestBase {
On 2012/12/04 21:10:30, Yaron wrote:
> This isn't a ContentDetection test. Why not just use ContentShellTestBase?
Hm. There are some useful things in ContentDetectionTestBase that I am using
(startActivityWithTestUrl, getContentView, getContentViewCore), but really these
things should just be in ContentShellTestBase.

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java:26:
private static final int DRAG_TO_X = 287;
On 2012/12/04 21:10:30, Yaron wrote:
> Can you add a brief comment about these magic constants.

Done.

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java:158:
int positionX = -1;
On 2012/12/04 21:10:30, Yaron wrote:
> nit: rename mPositionX and mPositionY

Done.

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java:188:

On 2012/12/04 21:10:30, Yaron wrote:
> nit: stray ws

Done.

https://codereview.chromium.org/11348361/diff/4001/content/public/test/androi...
File
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchUtils.java
(right):

https://codereview.chromium.org/11348361/diff/4001/content/public/test/androi...
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchUtils.java:85:
public static void sleepForDoubleTapTimeout(Instrumentation instrumentation) {
On 2012/12/04 21:10:30, Yaron wrote:
> nit: brief javadoc

Done.

https://codereview.chromium.org/11348361/diff/4001/content/public/test/androi...
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchUtils.java:86:
SystemClock.sleep((long)(ViewConfiguration.getDoubleTapTimeout() * 1.5));
On 2012/12/04 21:10:30, Yaron wrote:
> WHy 1.5? Just guaranteeing that you don't fall into that interval?
For long click below, we do the same thing (wait for longPressTimeout * 1.5).
Thought I'd do the same here. I can't think of any reason for it originally
except to guarantee that the wait is long enough.

[Yaron, 2012-12-05 00:17:21]

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
File
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java
(right):

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java:19:
public class InsertionHandleTest extends ContentDetectionTestBase {
On 2012/12/04 23:07:17, cjhopman wrote:
> On 2012/12/04 21:10:30, Yaron wrote:
> > This isn't a ContentDetection test. Why not just use ContentShellTestBase?
> Hm. There are some useful things in ContentDetectionTestBase that I am using
> (startActivityWithTestUrl, getContentView, getContentViewCore), but really
these
> things should just be in ContentShellTestBase.

Yep, they should be in ContentShellTestBase. So.. please fix :)

[cjhopman, 2012-12-08 00:08:50]

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
File
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java
(right):

https://codereview.chromium.org/11348361/diff/4001/content/public/android/jav...
content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java:19:
public class InsertionHandleTest extends ContentDetectionTestBase {
On 2012/12/05 00:17:21, Yaron wrote:
> On 2012/12/04 23:07:17, cjhopman wrote:
> > On 2012/12/04 21:10:30, Yaron wrote:
> > > This isn't a ContentDetection test. Why not just use ContentShellTestBase?
> > Hm. There are some useful things in ContentDetectionTestBase that I am using
> > (startActivityWithTestUrl, getContentView, getContentViewCore), but really
> these
> > things should just be in ContentShellTestBase.
> 
> Yep, they should be in ContentShellTestBase. So.. please fix :)

Done.

[Yaron, 2012-12-10 18:30:36]

lgtm

https://codereview.chromium.org/11348361/diff/17001/content/test/data/android...
File
content/test/data/android/device_files/content_detection/editable_long_text.html
(right):

https://codereview.chromium.org/11348361/diff/17001/content/test/data/android...
content/test/data/android/device_files/content_detection/editable_long_text.html:1:
<html>
One last thing: this shouldn't be under a "content_detection" folder. Please
move to "insertion_handle" or "selection_handle" folder

[commit-bot: I haz the power, 2012-12-11 20:38:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/11348361/31002

[commit-bot: I haz the power, 2012-12-11 21:08:56]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-12-11 21:12:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/11348361/37002

[commit-bot: I haz the power, 2012-12-11 22:14:44]

Retried try job too often on android_dbg for step(s) build

[commit-bot: I haz the power, 2012-12-11 22:21:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/11348361/43002

[commit-bot: I haz the power, 2012-12-12 01:01:44]

Change committed as 172477