Fix more corner cases for empty touch selection.

Fix more corner cases for empty touch selection.

1) In r373606 I fixed the case where selection would select whitespace,
but that still left a broken case -- when the hit test intersects with
the empty space at the end of a textbox (which doesn't have whitespace
characters per se).  Add an explicit comparison to lastPositionInNode to
cover this.

2) The same empty selection conventions apply to double-tap as well as
long-press, so extend the enum enabling this behavior to them.  (Double
taps enter SelectionController as synthetic mouse events with
clickCount=2 and fromTouch=true.)

BUG=415933
Committed: https://crrev.com/18c145f35f2dbbb10b4ff7168a115b9119fef6fc
Cr-Commit-Position: refs/heads/master@{#401155}

[aelias_OOO_until_Jul13, 2016-06-21 04:45:23]

aelias@chromium.org changed reviewers:
+ yosin@chromium.org

[aelias_OOO_until_Jul13, 2016-06-21 04:45:32]

Hi yosin@, PTAL.

[yosin_UTC9, 2016-06-21 07:50:45]

https://codereview.chromium.org/2074093003/diff/60001/content/public/android/...
File
content/public/android/javatests/src/org/chromium/content/browser/ContentViewCoreSelectionTest.java
(right):

https://codereview.chromium.org/2074093003/diff/60001/content/public/android/...
content/public/android/javatests/src/org/chromium/content/browser/ContentViewCoreSelectionTest.java:38:
+ "<br/><input id=\"password\" type=\"password\" value=\"SamplePassword\"
size=\"10\"/>"
Why do you change id to "password"?

https://codereview.chromium.org/2074093003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebViewTest.cpp (right):

https://codereview.chromium.org/2074093003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1769: EXPECT_EQ("",
std::string(frame->selectionAsText().utf8().data()));
nit: I think you don't need to use |std::string()| here.
How about
EXPECT_TRUE(frame->selectionAsText().isEmpty());

https://codereview.chromium.org/2074093003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1780: EXPECT_EQ("",
std::string(frame->selectionAsText().utf8().data()));
nit: I think you don't need to use |std::string()| here.
How about
EXPECT_TRUE(frame->selectionAsText().isEmpty());

https://codereview.chromium.org/2074093003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1787: EXPECT_EQ("",
std::string(frame->selectionAsText().utf8().data()));
nit: I think you don't need to use |std::string()| here.
How about
EXPECT_TRUE(frame->selectionAsText().isEmpty());

https://codereview.chromium.org/2074093003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1791: EXPECT_EQ("",
std::string(frame->selectionAsText().utf8().data()));
nit: I think you don't need to use |std::string()| here.
How about
EXPECT_TRUE(frame->selectionAsText().isEmpty());

[aelias_OOO_until_Jul13, 2016-06-21 18:29:03]

aelias@chromium.org changed reviewers:
+ dtrainor@chromium.org

[aelias_OOO_until_Jul13, 2016-06-21 18:30:41]

+dtrainor for OWNERS of small test data changes in content/public/android/. 
(The tests relied on the broken selection behavior that I fixed in this patch,
so I needed to change the textbox size so that the longpress directly hits the
text it it intended to select.)

https://codereview.chromium.org/2074093003/diff/60001/content/public/android/...
File
content/public/android/javatests/src/org/chromium/content/browser/ContentViewCoreSelectionTest.java
(right):

https://codereview.chromium.org/2074093003/diff/60001/content/public/android/...
content/public/android/javatests/src/org/chromium/content/browser/ContentViewCoreSelectionTest.java:38:
+ "<br/><input id=\"password\" type=\"password\" value=\"SamplePassword\"
size=\"10\"/>"
On 2016/06/21 at 07:50:45, Yosi_UTC9 wrote:
> Why do you change id to "password"?

To fit in Java 100-character line limit.  It was the cleanest way I could think
of.

https://codereview.chromium.org/2074093003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebViewTest.cpp (right):

https://codereview.chromium.org/2074093003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1769: EXPECT_EQ("",
std::string(frame->selectionAsText().utf8().data()));
On 2016/06/21 at 07:50:45, Yosi_UTC9 wrote:
> nit: I think you don't need to use |std::string()| here.
> How about
> EXPECT_TRUE(frame->selectionAsText().isEmpty());

OK, all 4 changed.

[David Trainor- moved to gerrit, 2016-06-21 23:13:42]

content/public/android lgtm thanks!

[yosin_UTC9, 2016-06-22 01:19:25]

lgtm

Thanks for explanation of about Java changes.

[aelias_OOO_until_Jul13, 2016-06-22 01:20:20]

The CQ bit was checked by aelias@chromium.org

[commit-bot: I haz the power, 2016-06-22 01:20:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2074093003/80001

[commit-bot: I haz the power, 2016-06-22 01:26:05]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-22 01:28:03]

Description was changed from

==========
Fix more corner cases for empty touch selection.

1) In r373606 I fixed the case where selection would select whitespace,
but that still left a broken case -- when the hit test intersects with
the empty space at the end of a textbox (which doesn't have whitespace
characters per se).  Add an explicit comparison to lastPositionInNode to
cover this.

2) The same empty selection conventions apply to double-tap as well as
long-press, so extend the enum enabling this behavior to them.  (Double
taps enter SelectionController as synthetic mouse events with
clickCount=2 and fromTouch=true.)

BUG=415933
==========

to

==========
Fix more corner cases for empty touch selection.

1) In r373606 I fixed the case where selection would select whitespace,
but that still left a broken case -- when the hit test intersects with
the empty space at the end of a textbox (which doesn't have whitespace
characters per se).  Add an explicit comparison to lastPositionInNode to
cover this.

2) The same empty selection conventions apply to double-tap as well as
long-press, so extend the enum enabling this behavior to them.  (Double
taps enter SelectionController as synthetic mouse events with
clickCount=2 and fromTouch=true.)

BUG=415933

Committed: https://crrev.com/18c145f35f2dbbb10b4ff7168a115b9119fef6fc
Cr-Commit-Position: refs/heads/master@{#401155}
==========

[commit-bot: I haz the power, 2016-06-22 01:28:04]

Patchset 5 (id:??) landed as
https://crrev.com/18c145f35f2dbbb10b4ff7168a115b9119fef6fc
Cr-Commit-Position: refs/heads/master@{#401155}