[WebView] Hide PopupWindow-based selection handles on scroll

[WebView] Hide PopupWindow-based selection handles on scroll

Restore the previous behavior of hiding the PopupWindow-based text
selection handles on scroll. Note that such hiding applies when the
content is scrolling as well as when the containing View is moving.

Also avoid sync barrier issues by deferring invalidations relating to
visibility and position updates of the PopupWindow.

BUG=423120
Committed: https://crrev.com/4a73ea3d91b615fefd45b730409b5f4bcc83720f
Cr-Commit-Position: refs/heads/master@{#300114}

[jdduke (slow), 2014-10-14 18:22:07]

jdduke@chromium.org changed reviewers:
+ benm@chromium.org

[jdduke (slow), 2014-10-14 18:22:08]

benm@: PTAL, thanks. This isn't really an ideal solution, but it's more or less
what we had previously.

[benm (inactive), 2014-10-16 19:56:32]

Thanks Jared, this seems to fix the issue, mod showing/hiding rather than fade
in/out.

https://codereview.chromium.org/657593003/diff/20001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/input/PopupTouchHandleDrawable.java
(right):

https://codereview.chromium.org/657593003/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/input/PopupTouchHandleDrawable.java:110:
mContainer.setAnimationStyle(0);
should this be a fade animation? I tried patching this into M38, however the
handles just seem to show/hide rather than fade.

[jdduke (slow), 2014-10-16 20:00:39]

https://codereview.chromium.org/657593003/diff/20001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/input/PopupTouchHandleDrawable.java
(right):

https://codereview.chromium.org/657593003/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/input/PopupTouchHandleDrawable.java:110:
mContainer.setAnimationStyle(0);
On 2014/10/16 19:56:31, benm wrote:
> should this be a fade animation? I tried patching this into M38, however the
> handles just seem to show/hide rather than fade.

I believe we used |setAnimationStyle(0)| on the old handles, but we did have a
fade-in animation (I think the PopupWindow animation only applies to fade out).
I can restore both animations if you like (at the possible expense of jank when
fading out).

[benm (inactive), 2014-10-16 20:10:12]

On 2014/10/16 20:00:39, jdduke wrote:
>
https://codereview.chromium.org/657593003/diff/20001/content/public/android/j...
> File
>
content/public/android/java/src/org/chromium/content/browser/input/PopupTouchHandleDrawable.java
> (right):
> 
>
https://codereview.chromium.org/657593003/diff/20001/content/public/android/j...
>
content/public/android/java/src/org/chromium/content/browser/input/PopupTouchHandleDrawable.java:110:
> mContainer.setAnimationStyle(0);
> On 2014/10/16 19:56:31, benm wrote:
> > should this be a fade animation? I tried patching this into M38, however the
> > handles just seem to show/hide rather than fade.
> 
> I believe we used |setAnimationStyle(0)| on the old handles, but we did have a
> fade-in animation (I think the PopupWindow animation only applies to fade
out).
> I can restore both animations if you like (at the possible expense of jank
when
> fading out).

Thanks - just compared with the old implementation and indeed it looks like we
only have an animation on fade in after they've been temporarily hidden. Can we
restore to that behaviour?

[jdduke (slow), 2014-10-16 20:21:00]

On 2014/10/16 20:10:12, benm wrote:
> Thanks - just compared with the old implementation and indeed it looks like we
> only have an animation on fade in after they've been temporarily hidden. Can
we
> restore to that behaviour?

Yup, I actually had that in the original version I was playing with, but ripped
it out because I wasn't sure how good it looked. In any case, I'll restore it
(fade in), shouldn't be too much trouble.

[jdduke (slow), 2014-10-16 22:23:47]

OK, fade in animation restored. If you're happy with this, feel free to land
(I'll be OOO for a few days).

[benm (inactive), 2014-10-17 15:52:35]

lgtm

[benm (inactive), 2014-10-17 15:52:40]

The CQ bit was checked by benm@chromium.org

[commit-bot: I haz the power, 2014-10-17 15:52:53]

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

[commit-bot: I haz the power, 2014-10-17 16:26:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-17 16:27:15]

Patchset 3 (id:??) landed as
https://crrev.com/4a73ea3d91b615fefd45b730409b5f4bcc83720f
Cr-Commit-Position: refs/heads/master@{#300114}