Fix problem with input fields not gaining focus

Fix problem with input fields not gaining focus

Brings the onFocusChanged handling exactly inline between ContentView and
AwContents.

NOTRY=true
BUG=285582
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221482

[joth, 2013-09-05 02:59:01]

+aurimas@ who pointed me in the right line for this.

[Nico, 2013-09-05 03:11:46]

Is it possible to test this?

[joth, 2013-09-05 03:28:55]

On 2013/09/05 03:11:46, Nico wrote:
> Is it possible to test this?

Hmm great question.
My gut feel is any obvious test is going to degenerate into effectively testing
that a boolean holds true after we set it to true (via some numerous plumbing)
because-
- the real scenario under test involves apps with multiple activities, which is
awkward to orchestrate in a test.
- the end state we're looking for is that the form can receive user input, and
injecting key presses is very awkward to orchestrate under test (in a non-flaky
way)

Copied in mkosiba as he often has good suggestions on view level testing...

[Nico, 2013-09-05 03:41:57]

On 2013/09/05 03:28:55, joth wrote:
> On 2013/09/05 03:11:46, Nico wrote:
> > Is it possible to test this?
> 
> Hmm great question.
> My gut feel is any obvious test is going to degenerate into effectively
testing
> that a boolean holds true after we set it to true (via some numerous plumbing)
> because-
> - the real scenario under test involves apps with multiple activities, which
is
> awkward to orchestrate in a test.
> - the end state we're looking for is that the form can receive user input, and
> injecting key presses is very awkward to orchestrate under test (in a
non-flaky
> way)
> 
> Copied in mkosiba as he often has good suggestions on view level testing...

I recently learned about (google-internal) go/espresso-test . That doesn't look
too bad to me?

[boliu, 2013-09-05 03:45:47]

lgtm % nit

Yeah I agree that testing explicitly for this failure case is difficult as the
reasons stated. But I think it's worth to have a more general instrumentation
test that covers this:

* Load page with an <input type=text>, maybe 100% width and height
* Fake a click with onTouchEvent to focus the input field
* Maybe detech keyboard shows up (I'm not even sure how to do this)
* Fake keyboard input with dispatchKeyEvent
* Use javascript to check that the text is in the input field

Could do this in a separate CL though

https://codereview.chromium.org/23623020/diff/1/android_webview/java/src/org/...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/23623020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1493:
mWindowFocused = hasWindowFocus;
nit: mWindowFocused is no longer used for anything. Get rid of it?

[joth, 2013-09-05 03:49:49]

https://codereview.chromium.org/23623020/diff/1/android_webview/java/src/org/...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/23623020/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1493:
mWindowFocused = hasWindowFocus;
On 2013/09/05 03:45:47, boliu wrote:
> nit: mWindowFocused is no longer used for anything. Get rid of it?

well... it's used to ensure we correctly restore the onWindowFocusChanged()
state on a popup, and I guarantee anyone adding code in here in future would
forget to deal with that (unless I put a big warning here i guess) so as I'd
already coded this bit I thought I might as well keep it.
I'm not super attached to it though...

[commit-bot: I haz the power, 2013-09-05 03:56:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joth@chromium.org/23623020/11001

[commit-bot: I haz the power, 2013-09-05 04:35:54]

Retried try job too often on ios_dbg_simulator for step(s) ui_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[joth, 2013-09-05 18:56:45]

On 2013/09/05 04:35:54, I haz the power (commit-bot) wrote:
> Retried try job too often on ios_dbg_simulator for step(s) ui_unittests
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

All failing builds & tests are in configs that don't even build the changed file
here so plumping for "NOTRY=true"

[commit-bot: I haz the power, 2013-09-05 18:58:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joth@chromium.org/23623020/11001

[commit-bot: I haz the power, 2013-09-05 19:00:06]

Change committed as 221482