[Android] Perform haptic feedback after long press when appropriate

[Android] Perform haptic feedback after long press when appropriate

Tentatively trigger haptic feedback when ContentViewCore is directed to show the
the paste popup. While there are no guarantees that such a commend was triggered
by a long press gesture, it should still be a usability improvement. Also
trigger feedback when the selection is updated via a repeated long press gesture.

BUG=150151, 399619
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290107

[jdduke (slow), 2014-08-13 15:57:32]

cjhopman@: What do you think?

[jdduke (slow), 2014-08-13 18:37:10]

On 2014/08/13 15:57:32, jdduke wrote:
> cjhopman@: What do you think?

Hmm, I went ahead and folded in the case where the selection is updated via
repeated long press gestures.

[cjhopman, 2014-08-14 00:12:28]

https://codereview.chromium.org/467303002/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/touch_selection_controller.cc (right):

https://codereview.chromium.org/467303002/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/touch_selection_controller.cc:332: if
(!is_selection_active_ || last_input_event_type_ == LONG_PRESS) {
Maybe there's a better name for last_input_event_type_, maybe
pending_input_event_type_ or response_pending_input_event_type_. 

Should this be reset to NONE on other events that might signify that it is done?


Should we have a more robust way of determining that a particular event (or type
of event) caused a selection change?

https://codereview.chromium.org/467303002/diff/40001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/467303002/diff/40001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:2420:
private void showPastePopup(int xDip, int yDip) {
This naming would make me expect that showPastePopup will always show the paste
popup (but showPastePopupIfAppropriate only shows it sometimes).

Maybe change to showPastePopupWithFeedback and showPastePopup or some such.

[jdduke (slow), 2014-08-14 17:57:19]

https://codereview.chromium.org/467303002/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/touch_selection_controller.cc (right):

https://codereview.chromium.org/467303002/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/touch_selection_controller.cc:332: if
(!is_selection_active_ || last_input_event_type_ == LONG_PRESS) {
On 2014/08/14 00:12:28, cjhopman wrote:
> Maybe there's a better name for last_input_event_type_, maybe
> pending_input_event_type_ or response_pending_input_event_type_. 
> 

Yeah, I'll tweak it.

> Should this be reset to NONE on other events that might signify that it is
done?
> 

Yes.

> 
> Should we have a more robust way of determining that a particular event (or
type
> of event) caused a selection change?

Yeah, the current attempt is both lazy and imprecise.  I've put in a hard reset
of the event any time the selection changes.  That still doesn't guarantee the
change came from a gesture; that would probably involve wiring the causal change
from Blink to be truly accurate.

https://codereview.chromium.org/467303002/diff/40001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/467303002/diff/40001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:2420:
private void showPastePopup(int xDip, int yDip) {
On 2014/08/14 00:12:28, cjhopman wrote:
> This naming would make me expect that showPastePopup will always show the
paste
> popup (but showPastePopupIfAppropriate only shows it sometimes).
> 
> Maybe change to showPastePopupWithFeedback and showPastePopup or some such.

Done.

[cjhopman, 2014-08-15 22:14:33]

lgtm

[jdduke (slow), 2014-08-15 22:15:07]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-08-15 22:16:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/467303002/100001

[commit-bot: I haz the power, 2014-08-16 06:17:23]

Committed patchset #4 (100001) as 290107