Attaching a video of flash

Deleted: ch_omnibox_dismiss.mp4 413 KB

	ch_omnibox_dismiss.mp4 413 KB View Download

I believe that this can happen any time in the following scenario:

<Layout android:focusable=true>
  <EditText android:focusable=true\>
</Layout>

editText.clearFocus() will move focus from EditText to Layout (a focusable wrapper to take away focus). This will cause onStartInput() with null InputConnection and therefore see this type of flicker.

But on Android,
1) One view should always be focused.
2) Keyboard gets automatically hidden only when you type 'enter' or view is removed from view hierarchy.

Therefore, if you're moving focus away from EditText, you have to either
1) Have a UI that requires EditText to be removed or user types enter.
2) Or hide soft input and move focus to a wrapper.

I think this is a bit ugly, and maybe Android can do a better job by considering 'editability' (e.g. onCheckIsTextEditor()) in the future, but For now I think 2) can work. I'll upload a CL soon.

Another workaround would be to emulate the original EditorInfo of the EditText in the focus placeholder. Technically the wrapper Layout can return null InputConnection while returning TYPE_TEXT as inputType.

But there will still be some caveat - if keyboard was showing some suggestion, and the focus change removed the suggestion in a flashy manner, then the user may still observe a flash. I think this is unavoidable if you intend not to hide keyboard immediately anyways.

Attached video shows such a flicker mentioned in #4 caused by suggestion change on Samsung keyboard.

Note that in this scenario the small suggestion near the 'GO' changes from 'www.' to '.com' to 'www.' again. I suspect that this is caused by another selection update before the second onCreateInputConnection() is called. (not sure if it's from TextView or AutocompleteController).

Deleted: a.mp4 1.2 MB

	a.mp4 1.2 MB View Download

Here's the video for the approach mentioned in #3. I've only tested against high end phones, so not sure if we still have the jankiness when two animations are triggered at the same time.

tedchoc@, ktam@ - WDYT?

Deleted: a.mp4 1.3 MB

	a.mp4 1.3 MB View Download

@Changwan, thanks for prototyping some solutions! I think the video in #5 is already a much better improvement so landing that first would be ideal if possible. (especially if we can get it in for the next dev push).

Is it possible to get a one-off APK for #6 so we can try it on lower end phones?

I'm not entirely sure if the UI improvement in #5 can justify the added code complexity and suggestion flickering.

Regarding #6 I measured performance on Android One in the case of this bug - focus on omnibox and then press downloads:

previously, we draw ToolbarPhone 30.7 times, and after #6 we draw ToolbarPhone 29.6 times in approximately 0.7 second.

Attached are the raw files.

Deleted: draw_current.txt 24.8 KB

draw_current.txt 24.8 KB View Download

Deleted: draw_hide.txt 24.1 KB

draw_hide.txt 24.1 KB View Download

Not sure I understand. Are you saying there is no visible performance jankiness with the #6 fix?

And #5 still looks significantly better as we're not bumping the whole UI up as we transition out. That said, if we think #6 won't have any visible issues I'm good with that too.

There will be some frame drop: (30.7-29.6)/0.7 = 1.6 fps loss, so the animation may look very slightly choppier on low-end phones when we hide keyboard and apply our animation at the same time. Ted and I chatted about this today, and we tend to think that perfecting #5 (removing suggestion flicker) is almost impossible because of how Android framework works. It's going to be a brittle and hacky approach at best. In this sense, I prefer #6 as long as performance degradation is minimal.

Sure - I think #6 sounds good to me. Is this something that we can either get an APK for to test on lower-end devices or just land a CL for if we are comfortable with the change?

#8 was tested on Android One which is a svelte device. At this point I think we should try to land it and get it reverted in case of adversarial evidence.

Sure - sounds good thanks! Just out of curiosity this means that we'd be animating the keyboard at the same time as everything else?

Correct.

Sounds good. Thanks! Would this affect animations in as well or just our fade out animation?

This should only affect keyboard hiding situations, so mostly fade out animations, I guess?

Got it. Yeah let's start there.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/657ba4ddf2ab939ce918666bcb89328e9eae93bc

commit 657ba4ddf2ab939ce918666bcb89328e9eae93bc
Author: Changwan Ryu <changwan@chromium.org>
Date: Sat Jun 03 01:56:03 2017

Avoid flicker when unfocusing omnibox in ChromeHome

clearFocus() moves focus from UrlBar to any focusable View which is in
this case ToolbarPhone. But ToolbarPhone itself is not editable, and
keyboard app flashes as a result of this type change until it gets
evicted from view hierarchy. The flashing behavior can be prevented by
hiding soft input before calling clearFocus().

BUG= 726442 

Change-Id: Ibfaf60e1d23457c1f5c174fbbf132379f583b492
Reviewed-on: https://chromium-review.googlesource.com/520712
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476854}
[modify] https://crrev.com/657ba4ddf2ab939ce918666bcb89328e9eae93bc/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java

requesting merge of #18 to m60

Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/285251422ee2794e936eb03949fe7377e7aa98aa

commit 285251422ee2794e936eb03949fe7377e7aa98aa
Author: Changwan Ryu <changwan@chromium.org>
Date: Mon Jun 05 19:42:30 2017

Avoid flicker when unfocusing omnibox in ChromeHome

clearFocus() moves focus from UrlBar to any focusable View which is in
this case ToolbarPhone. But ToolbarPhone itself is not editable, and
keyboard app flashes as a result of this type change until it gets
evicted from view hierarchy. The flashing behavior can be prevented by
hiding soft input before calling clearFocus().

BUG= 726442 

(cherry picked from commit 657ba4ddf2ab939ce918666bcb89328e9eae93bc)

Change-Id: Ibfaf60e1d23457c1f5c174fbbf132379f583b492
Reviewed-on: https://chromium-review.googlesource.com/520712
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#476854}
Reviewed-on: https://chromium-review.googlesource.com/524290
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3112@{#167}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/285251422ee2794e936eb03949fe7377e7aa98aa/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java