Prevent repeated taps from resetting insertion handle position

Prevent repeated taps from resetting insertion handle position

Repeated longpress and tap gestures were resetting the cached handle positions
in anticipation of receiving updated values from the renderer.  However, if the
gesture failed to change the selection position, no update was received, and
subsequent taps on the handle would improperly position the paste popup at the
reset position. Fix this by avoiding cached position resets when the insertion
or selection is already active.

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

[raghu, 2014-07-31 10:25:54]

Hi Jdduke,
This issue is happening because, we call the ResetCachedValues when the
selection is empty. But since selection is empty even in case of caret
selection, I have put a check to ensure that we dont call RestCachedValues while
selection or insertion is active.

The same is applicable for Selection handles also in cases where the UI doesnt
use Select Action bar and instead uses a popup.

PTAL.

[jdduke (slow), 2014-07-31 12:04:45]

https://codereview.chromium.org/434583002/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/input/touch_selection_controller.cc (right):

https://codereview.chromium.org/434583002/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/input/touch_selection_controller.cc:160: if
(!is_insertion_active_ && !is_selection_active_)
Hmm, maybe rename to ResetCachedValuesIfInactive and do the check there? That's
probably what we want in all cases where this method is called.  Also please add
a unit test.

[jdduke (slow), 2014-07-31 16:00:55]

Hmm, I'm going to be unavailable over the weekend, but I wanted to get this
fixed sooner than later. I uploaded a version of the patch here, with test:
https://codereview.chromium.org/432833002.  In the future, for minor issues like
this, it might be more effective to report the bug first before fixing it. 
Otherwise, if you do go ahead and fix it, please make sure that you have as much
test coverage as possible.  Feel free to cherry-pick the patch I uploaded at
https://codereview.chromium.org/432833002, upload it here, and I'll be happy to
approve.

[raghu, 2014-08-01 10:13:32]

On 2014/07/31 16:00:55, jdduke wrote:
> Hmm, I'm going to be unavailable over the weekend, but I wanted to get this
> fixed sooner than later. I uploaded a version of the patch here, with test:
> https://codereview.chromium.org/432833002.  In the future, for minor issues
like
> this, it might be more effective to report the bug first before fixing it. 
> Otherwise, if you do go ahead and fix it, please make sure that you have as
much
> test coverage as possible.  Feel free to cherry-pick the patch I uploaded at
> https://codereview.chromium.org/432833002, upload it here, and I'll be happy
to
> approve.

Hi Jdduke,
Thanks for helping me out with the unit tests, I will remember to write unit
tests for my further patches.
PTAL.

[jdduke (slow), 2014-08-02 14:15:56]

lgtm

[jdduke (slow), 2014-08-02 14:16:00]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-08-02 14:16:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/r.ghatage@samsung.com/434583002/40001

[commit-bot: I haz the power, 2014-08-02 14:58:10]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/36771)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-02 14:59:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-02 14:59:20]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/36773)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)

[jdduke (slow), 2014-08-02 17:45:28]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-08-02 17:46:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/r.ghatage@samsung.com/434583002/40001

[commit-bot: I haz the power, 2014-08-02 17:53:50]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/36788)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-02 17:55:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-02 17:55:01]

Try jobs failed on following builders:
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[raghu, 2014-08-03 08:30:37]

rebased the patch.

[raghu, 2014-08-03 08:30:58]

The CQ bit was checked by r.ghatage@samsung.com

[commit-bot: I haz the power, 2014-08-03 08:31:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/r.ghatage@samsung.com/434583002/60001

[commit-bot: I haz the power, 2014-08-03 11:12:35]

Change committed as 287256