Ensure initial focus is set properly when creating a ContentViewCore

Ensure initial focus is set properly when creating a ContentViewCore

BUG=708733
Review-Url: https://codereview.chromium.org/2805503002
Cr-Commit-Position: refs/heads/master@{#462235}
Committed: https://chromium.googlesource.com/chromium/src/+/8b258db9ec45e08a723cdb46a67b2cf356adda3f

[mthiesse, 2017-04-05 19:57:14]

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

[commit-bot: I haz the power, 2017-04-05 19:57:51]

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

[mthiesse, 2017-04-05 20:05:50]

The CQ bit was unchecked by mthiesse@chromium.org

[mthiesse, 2017-04-05 20:06:25]

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

[mthiesse, 2017-04-05 20:06:35]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-04-05 20:06:52]

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

[mthiesse, 2017-04-05 20:08:20]

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

[mthiesse, 2017-04-05 20:08:20]

WDYT aelias? I don't know of any bugs on ToT that this fixes, but we were
definitely not setting initial state correctly, so hopefully this prevents other
running into the same headaches I did.

[aelias_OOO_until_Jul13, 2017-04-05 20:24:42]

Thanks for the cleanup.

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

https://codereview.chromium.org/2805503002/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ContentView.java:167:
if (mHasWindowFocus != null) return mHasWindowFocus;
I'm not entirely comfortable with adding this state and logic here when it
doesn't solve any known bug (and there may be some logic to
View#hasWindowFocus's "incorrect" values that we don't yet understand).  How
about restricting this patch only to View onFocusChanged for now?

https://codereview.chromium.org/2805503002/diff/20001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/2805503002/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:364:
private boolean mHasViewFocus;
These are unused in this patch, please remove.

[mthiesse, 2017-04-05 20:49:31]

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

https://codereview.chromium.org/2805503002/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ContentView.java:167:
if (mHasWindowFocus != null) return mHasWindowFocus;
On 2017/04/05 20:24:41, aelias wrote:
> I'm not entirely comfortable with adding this state and logic here when it
> doesn't solve any known bug (and there may be some logic to
> View#hasWindowFocus's "incorrect" values that we don't yet understand).  How
> about restricting this patch only to View onFocusChanged for now?

Okay, I reverted these changes. However, I think we should still call
onWindowFocusChanged(mContainerView.hasWindowFocus()) when the containerView
changes, because not calling it is definitely incorrect.

https://codereview.chromium.org/2805503002/diff/20001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/2805503002/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:364:
private boolean mHasViewFocus;
On 2017/04/05 20:24:41, aelias wrote:
> These are unused in this patch, please remove.

Done.

[aelias_OOO_until_Jul13, 2017-04-05 21:01:11]

lgtm, thanks!

[mthiesse, 2017-04-05 21:04:49]

The CQ bit was checked by mthiesse@chromium.org

[commit-bot: I haz the power, 2017-04-05 21:05:33]

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-04-05 21:59:33]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491426289505050,
"parent_rev": "12af4f53a6107c5c80252666c804fb5a262b01bf", "commit_rev":
"8b258db9ec45e08a723cdb46a67b2cf356adda3f"}

[commit-bot: I haz the power, 2017-04-05 22:00:26]

Description was changed from

==========
Ensure initial focus is set properly when creating a ContentViewCore

BUG=708733
==========

to

==========
Ensure initial focus is set properly when creating a ContentViewCore

BUG=708733

Review-Url: https://codereview.chromium.org/2805503002
Cr-Commit-Position: refs/heads/master@{#462235}
Committed:
https://chromium.googlesource.com/chromium/src/+/8b258db9ec45e08a723cdb46a67b...
==========

[commit-bot: I haz the power, 2017-04-05 22:00:28]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/8b258db9ec45e08a723cdb46a67b...