Make chrome hide keyboard when opening a new window

Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The CL
https://codereview.chromium.org/2290133002 moves the hidding logic from
onViewFocusChanged() to onViewDetachedFromWindow() and onAttach(). But
the view will not be active when it is detached from window, so the keyboard
will not be hidden.

To fix that, this CL brings back the hidding logic to onViewFocusChanged(). Thus,
we are able to hide keyboard before the view becomes inactive. Since we don’t
want to change the current behavior of Webview, i.e., not hiding keyboard when
losing focus, so we pass a parameter to distinguish that. To avoid possible
regressions, this CL keeps the previous hiding logic in onViewDetachedFromWindow()
and onAttach().

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method. 

BUG=677646
Review-Url: https://codereview.chromium.org/2709633002
Cr-Commit-Position: refs/heads/master@{#451950}
Committed: https://chromium.googlesource.com/chromium/src/+/44b7e0d7c92d708fdc64cb2357bbb2a712551b94

[yabinh, 2017-02-21 03:29:53]

The CQ bit was checked by yabinh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 03:30:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yabinh, 2017-02-21 04:31:35]

Description was changed from

==========
Make chrome hide keyboard when opening a new window

BUG=677646
==========

to

==========
Make chrome hide keyboard when opening a new window

We should hide the keyboard when opening a new window in Chrome. The old 
implementation tried to do that in onViewDetachedFromWindow(). But the view 
will not be active by that time, so the keyboard will not be hidden. To fix 
that, this CL hides keyboard at an earlier stage (in onViewFocusChanged() ) when
the view is still active. Note that we don’t want to change the current behavior

of Webview, i.e., not hiding keyboard when losing focus, so we pass a parameter
to distinguish that. Also, this CL keeps the previous hiding logic in 
onViewDetachedFromWindow() and onAttach() to ensure that it will not cause
regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling.

BUG=677646
==========

[yabinh, 2017-02-21 04:32:07]

Description was changed from

==========
Make chrome hide keyboard when opening a new window

We should hide the keyboard when opening a new window in Chrome. The old 
implementation tried to do that in onViewDetachedFromWindow(). But the view 
will not be active by that time, so the keyboard will not be hidden. To fix 
that, this CL hides keyboard at an earlier stage (in onViewFocusChanged() ) when
the view is still active. Note that we don’t want to change the current behavior

of Webview, i.e., not hiding keyboard when losing focus, so we pass a parameter
to distinguish that. Also, this CL keeps the previous hiding logic in 
onViewDetachedFromWindow() and onAttach() to ensure that it will not cause
regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling.

BUG=677646
==========

to

==========
Make chrome hide keyboard when opening a new window

We should hide the keyboard when opening a new window in Chrome. The old 
implementation tried to do that in onViewDetachedFromWindow(). But the view 
will not be active by that time, so the keyboard will not be hidden. To fix 
that, this CL hides keyboard at an earlier stage (in onViewFocusChanged() ) when
the view is still active. Note that we don’t want to change the current behavior

of Webview, i.e., not hiding keyboard when losing focus, so we pass a parameter
to distinguish that. Also, this CL keeps the previous hiding logic in 
onViewDetachedFromWindow() and onAttach() to ensure that it will not cause 
regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling.

BUG=677646
==========

[yabinh, 2017-02-21 04:34:01]

Description was changed from

==========
Make chrome hide keyboard when opening a new window

We should hide the keyboard when opening a new window in Chrome. The old 
implementation tried to do that in onViewDetachedFromWindow(). But the view 
will not be active by that time, so the keyboard will not be hidden. To fix 
that, this CL hides keyboard at an earlier stage (in onViewFocusChanged() ) when
the view is still active. Note that we don’t want to change the current behavior

of Webview, i.e., not hiding keyboard when losing focus, so we pass a parameter
to distinguish that. Also, this CL keeps the previous hiding logic in 
onViewDetachedFromWindow() and onAttach() to ensure that it will not cause 
regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling.

BUG=677646
==========

to

==========
Make chrome hide keyboard when opening a new window

We should hide the keyboard when opening a new window in Chrome. The old 
implementation tried to do that in onViewDetachedFromWindow(). But the view 
will not be active by that time, so the keyboard will not be hidden.

To fix that, this CL hides keyboard at an earlier stage (in onViewFocusChanged()
) when
the view is still active. Note that we don’t want to change the current behavior

of Webview, i.e., not hiding keyboard when losing focus, so we pass a parameter
to distinguish that. Also, this CL keeps the previous hiding logic in 
onViewDetachedFromWindow() and onAttach() to ensure that it will not cause 
regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling.

BUG=677646
==========

[yabinh, 2017-02-21 04:35:10]

Description was changed from

==========
Make chrome hide keyboard when opening a new window

We should hide the keyboard when opening a new window in Chrome. The old 
implementation tried to do that in onViewDetachedFromWindow(). But the view 
will not be active by that time, so the keyboard will not be hidden.

To fix that, this CL hides keyboard at an earlier stage (in onViewFocusChanged()
) when
the view is still active. Note that we don’t want to change the current behavior

of Webview, i.e., not hiding keyboard when losing focus, so we pass a parameter
to distinguish that. Also, this CL keeps the previous hiding logic in 
onViewDetachedFromWindow() and onAttach() to ensure that it will not cause 
regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling.

BUG=677646
==========

to

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The old 
implementation tried to do that in onViewDetachedFromWindow(). But the view 
will not be active by that time, so the keyboard will not be hidden.

To fix that, this CL hides keyboard at an earlier stage (in onViewFocusChanged()
)
when the view is still active. Note that we don’t want to change the current
behavior 
of Webview, i.e., not hiding keyboard when losing focus, so we pass a parameter
to distinguish that. Also, this CL keeps the previous hiding logic in 
onViewDetachedFromWindow() and onAttach() to ensure that it will not cause 
regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling.

BUG=677646
==========

[yabinh, 2017-02-21 04:37:46]

Description was changed from

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The old 
implementation tried to do that in onViewDetachedFromWindow(). But the view 
will not be active by that time, so the keyboard will not be hidden.

To fix that, this CL hides keyboard at an earlier stage (in onViewFocusChanged()
)
when the view is still active. Note that we don’t want to change the current
behavior 
of Webview, i.e., not hiding keyboard when losing focus, so we pass a parameter
to distinguish that. Also, this CL keeps the previous hiding logic in 
onViewDetachedFromWindow() and onAttach() to ensure that it will not cause 
regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling.

BUG=677646
==========

to

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The old 
implementation tried to do that in onViewDetachedFromWindow(). But the view 
will not be active by that time, so the keyboard will not be hidden.

To fix that, this CL hides keyboard at an earlier stage (in onViewFocusChanged()
)
when the view is still active. Note that we don’t want to change the current
behavior 
of Webview, i.e., not hiding keyboard when losing focus, so we pass a parameter
to distinguish that. Also, this CL keeps the previous hiding logic in 
onViewDetachedFromWindow() and onAttach() to ensure that it will not cause 
regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method.

BUG=677646
==========

[yabinh, 2017-02-21 04:39:14]

yabinh@chromium.org changed reviewers:
+ changwan@chromium.org

[yabinh, 2017-02-21 04:39:15]

changwan@, PTAL, especially the description.

[Changwan Ryu, 2017-02-21 05:16:39]

lgtm w/ nits

Could you also refer to the old CL in the CL description? 
Without looking at the old CL, it is difficult to understand what 'previous'
behavior it had and how this is going back.

https://codereview.chromium.org/2709633002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/ContentView.java
(right):

https://codereview.chromium.org/2709633002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/ContentView.java:154:
mContentViewCore.onFocusChanged(gainFocus, true);
true /* hideKeyboardOnBlur */

https://codereview.chromium.org/2709633002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
(right):

https://codereview.chromium.org/2709633002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java:440:
* @param hideKeyboardIfNeeded True if we should hide soft keyboard when losing
focus.
how about renaming this to hideKeyboardOnBlur?

[commit-bot: I haz the power, 2017-02-21 05:29:49]

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

[commit-bot: I haz the power, 2017-02-21 05:29:50]

Dry run: This issue passed the CQ dry run.

[yabinh, 2017-02-21 06:00:43]

The CQ bit was checked by yabinh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 06:00:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yabinh, 2017-02-21 06:14:27]

Description was changed from

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The old 
implementation tried to do that in onViewDetachedFromWindow(). But the view 
will not be active by that time, so the keyboard will not be hidden.

To fix that, this CL hides keyboard at an earlier stage (in onViewFocusChanged()
)
when the view is still active. Note that we don’t want to change the current
behavior 
of Webview, i.e., not hiding keyboard when losing focus, so we pass a parameter
to distinguish that. Also, this CL keeps the previous hiding logic in 
onViewDetachedFromWindow() and onAttach() to ensure that it will not cause 
regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method.

BUG=677646
==========

to

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The CL
https://codereview.chromium.org/2290133002 moves the hidding logic from
onViewFocusChanged() to onViewDetachedFromWindow() and onAttach(). But
the view will not be active when it is detached from window, so the keyboard
will not be hidden.

To fix that, this CL brings back the hidding logic to onViewFocusChanged().
Thus,
we are able to hide keyboard before the view becomes inactive. Since we don’t
want to change the current behavior of Webview, i.e., not hiding keyboard when
losing focus, so we pass a parameter to distinguish that. Note that this CL
keeps
the previous hiding logic in onViewDetachedFromWindow() and onAttach() to
ensure no regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method.

BUG=677646
==========

[yabinh, 2017-02-21 06:16:57]

Description was changed from

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The CL
https://codereview.chromium.org/2290133002 moves the hidding logic from
onViewFocusChanged() to onViewDetachedFromWindow() and onAttach(). But
the view will not be active when it is detached from window, so the keyboard
will not be hidden.

To fix that, this CL brings back the hidding logic to onViewFocusChanged().
Thus,
we are able to hide keyboard before the view becomes inactive. Since we don’t
want to change the current behavior of Webview, i.e., not hiding keyboard when
losing focus, so we pass a parameter to distinguish that. Note that this CL
keeps
the previous hiding logic in onViewDetachedFromWindow() and onAttach() to
ensure no regressions. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method.

BUG=677646
==========

to

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The CL
https://codereview.chromium.org/2290133002 moves the hidding logic from
onViewFocusChanged() to onViewDetachedFromWindow() and onAttach(). But
the view will not be active when it is detached from window, so the keyboard
will not be hidden.

To fix that, this CL brings back the hidding logic to onViewFocusChanged().
Thus,
we are able to hide keyboard before the view becomes inactive. Since we don’t
want to change the current behavior of Webview, i.e., not hiding keyboard when
losing focus, so we pass a parameter to distinguish that. To avoid possible
regressions, this CL keeps the previous hiding logic in
onViewDetachedFromWindow()
and onAttach().

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method.

BUG=677646
==========

[yabinh, 2017-02-21 06:17:34]

Description was changed from

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The CL
https://codereview.chromium.org/2290133002 moves the hidding logic from
onViewFocusChanged() to onViewDetachedFromWindow() and onAttach(). But
the view will not be active when it is detached from window, so the keyboard
will not be hidden.

To fix that, this CL brings back the hidding logic to onViewFocusChanged().
Thus,
we are able to hide keyboard before the view becomes inactive. Since we don’t
want to change the current behavior of Webview, i.e., not hiding keyboard when
losing focus, so we pass a parameter to distinguish that. To avoid possible
regressions, this CL keeps the previous hiding logic in
onViewDetachedFromWindow()
and onAttach().

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method. 

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method.

BUG=677646
==========

to

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The CL
https://codereview.chromium.org/2290133002 moves the hidding logic from
onViewFocusChanged() to onViewDetachedFromWindow() and onAttach(). But
the view will not be active when it is detached from window, so the keyboard
will not be hidden.

To fix that, this CL brings back the hidding logic to onViewFocusChanged().
Thus,
we are able to hide keyboard before the view becomes inactive. Since we don’t
want to change the current behavior of Webview, i.e., not hiding keyboard when
losing focus, so we pass a parameter to distinguish that. To avoid possible
regressions, this CL keeps the previous hiding logic in
onViewDetachedFromWindow()
and onAttach().

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method. 

BUG=677646
==========

[yabinh, 2017-02-21 06:18:09]

On 2017/02/21 05:16:39, Changwan Ryu wrote:
> lgtm w/ nits
> 
> Could you also refer to the old CL in the CL description? 
> Without looking at the old CL, it is difficult to understand what 'previous'
> behavior it had and how this is going back.

Done.

[yabinh, 2017-02-21 06:18:31]

https://codereview.chromium.org/2709633002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/ContentView.java
(right):

https://codereview.chromium.org/2709633002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/ContentView.java:154:
mContentViewCore.onFocusChanged(gainFocus, true);
On 2017/02/21 05:16:39, Changwan Ryu wrote:
> true /* hideKeyboardOnBlur */

Done.

https://codereview.chromium.org/2709633002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
(right):

https://codereview.chromium.org/2709633002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java:440:
* @param hideKeyboardIfNeeded True if we should hide soft keyboard when losing
focus.
On 2017/02/21 05:16:39, Changwan Ryu wrote:
> how about renaming this to hideKeyboardOnBlur?

Done.

[yabinh, 2017-02-21 06:21:17]

yabinh@chromium.org changed reviewers:
+ aelias@chromium.org

[yabinh, 2017-02-21 06:21:18]

aelias@,PTAL. Thank!

[commit-bot: I haz the power, 2017-02-21 08:22:51]

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

[commit-bot: I haz the power, 2017-02-21 08:22:52]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[yabinh, 2017-02-21 09:03:32]

FYI, I just confirmed that this CL won't change the behavior of Webview.

[aelias_OOO_until_Jul13, 2017-02-22 01:52:41]

aelias@chromium.org changed reviewers:
+ boliu@chromium.org, dtrainor@chromium.org

[aelias_OOO_until_Jul13, 2017-02-22 01:52:42]

lgtm, adding boliu@ and dtrainor@ for OWNERS.

[boliu, 2017-02-22 02:45:50]

lgtm % comment

https://codereview.chromium.org/2709633002/diff/20001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/2709633002/diff/20001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:3235:
mContentViewCore.onFocusChanged(focused, false);
document parameter name

[yabinh, 2017-02-22 03:38:14]

The CQ bit was checked by yabinh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 03:39:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yabinh, 2017-02-22 03:40:05]

Thanks!
dtrainor@, PTAL.

https://codereview.chromium.org/2709633002/diff/20001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/2709633002/diff/20001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:3235:
mContentViewCore.onFocusChanged(focused, false);
On 2017/02/22 02:45:50, boliu wrote:
> document parameter name

Done.

[David Trainor- moved to gerrit, 2017-02-22 05:05:19]

lgtm!

[yabinh, 2017-02-22 05:28:31]

The CQ bit was unchecked by yabinh@chromium.org

[yabinh, 2017-02-22 05:28:31]

The CQ bit was checked by yabinh@chromium.org

[yabinh, 2017-02-22 05:28:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
changwan@chromium.org, aelias@chromium.org, boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2709633002/#ps40001
(title: "Add a comment")

[commit-bot: I haz the power, 2017-02-22 05:28:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 09:14:45]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487741311420130,
"parent_rev": "6eb028403e170e62a0c63670a4bcaac80bc4a8a3", "commit_rev":
"44b7e0d7c92d708fdc64cb2357bbb2a712551b94"}

[commit-bot: I haz the power, 2017-02-22 09:15:27]

Description was changed from

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The CL
https://codereview.chromium.org/2290133002 moves the hidding logic from
onViewFocusChanged() to onViewDetachedFromWindow() and onAttach(). But
the view will not be active when it is detached from window, so the keyboard
will not be hidden.

To fix that, this CL brings back the hidding logic to onViewFocusChanged().
Thus,
we are able to hide keyboard before the view becomes inactive. Since we don’t
want to change the current behavior of Webview, i.e., not hiding keyboard when
losing focus, so we pass a parameter to distinguish that. To avoid possible
regressions, this CL keeps the previous hiding logic in
onViewDetachedFromWindow()
and onAttach().

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method. 

BUG=677646
==========

to

==========
Make chrome hide keyboard when opening a new window

We should hide keyboard when opening a new window in Chrome. The CL
https://codereview.chromium.org/2290133002 moves the hidding logic from
onViewFocusChanged() to onViewDetachedFromWindow() and onAttach(). But
the view will not be active when it is detached from window, so the keyboard
will not be hidden.

To fix that, this CL brings back the hidding logic to onViewFocusChanged().
Thus,
we are able to hide keyboard before the view becomes inactive. Since we don’t
want to change the current behavior of Webview, i.e., not hiding keyboard when
losing focus, so we pass a parameter to distinguish that. To avoid possible
regressions, this CL keeps the previous hiding logic in
onViewDetachedFromWindow()
and onAttach().

Alternatively, we can use a tab observer to determine when to hide keyboard, 
but it will increase code coupling, so we stick to the above method. 

BUG=677646

Review-Url: https://codereview.chromium.org/2709633002
Cr-Commit-Position: refs/heads/master@{#451950}
Committed:
https://chromium.googlesource.com/chromium/src/+/44b7e0d7c92d708fdc64cb2357bb...
==========

[commit-bot: I haz the power, 2017-02-22 09:15:28]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/44b7e0d7c92d708fdc64cb2357bb...