Refactor ContentViewClient (5/6)

Refactor ContentViewClient (5/6)

|shouldPropagetKeyEvent| was removed from the interface,
replaced with a static utility method.

BUG=620172
Review-Url: https://codereview.chromium.org/2756813002
Cr-Commit-Position: refs/heads/master@{#459385}
Committed: https://chromium.googlesource.com/chromium/src/+/219a962acb6807d9c694e16659d9388772eae457

[Jinsuk Kim, 2017-03-17 01:04:35]

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

[Jinsuk Kim, 2017-03-17 01:04:54]

The CQ bit was unchecked by jinsukkim@chromium.org

[Jinsuk Kim, 2017-03-20 00:31:19]

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

[commit-bot: I haz the power, 2017-03-20 00:31:30]

Dry run: 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-03-20 01:44:19]

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

[commit-bot: I haz the power, 2017-03-20 01:44:21]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Jinsuk Kim, 2017-03-20 07:00:00]

Patchset #3 (id:40001) has been deleted

[Jinsuk Kim, 2017-03-20 07:01:12]

Description was changed from

==========
Refactor ContentViewClient (5/6)

Moves the interface method |getSystemWindowInsetBottom| to
ViewAndroidDelegate and deletes some ContentViewCore APIs
that become unused.

BUG=620172
==========

to

==========
Refactor ContentViewClient (5/6)

Moves the interface method |getSystemWindowInsetBottom| to
ViewAndroidDelegate and deletes some ContentViewCore APIs
that become unused.

|shouldPropagetKeyEvent| was removed from the interface,
replaced with a static utility method.

BUG=620172
==========

[Jinsuk Kim, 2017-03-22 00:49:17]

jinsukkim@chromium.org changed reviewers:
+ boliu@chromium.org, nyquist@chromium.org

[Jinsuk Kim, 2017-03-22 00:49:18]

nyquist@chromium.org: Please review changes in chrome/

boliu@chromium.org: Please review changes in content/ ui/

[boliu, 2017-03-22 15:50:41]

this CL is two independent refactors afaict

split them up

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

https://codereview.chromium.org/2756813002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:3145:
boolean shouldOverride = KeyUtils.shouldOverrideKeyEvent(event);
why does webview need to check this, just check mContentsClient here, and CVC
can keep checking its own version, and result would be the same?

[Jinsuk Kim, 2017-03-23 01:47:17]

Description was changed from

==========
Refactor ContentViewClient (5/6)

Moves the interface method |getSystemWindowInsetBottom| to
ViewAndroidDelegate and deletes some ContentViewCore APIs
that become unused.

|shouldPropagetKeyEvent| was removed from the interface,
replaced with a static utility method.

BUG=620172
==========

to

==========
Refactor ContentViewClient (5/6)

|shouldPropagetKeyEvent| was removed from the interface,
replaced with a static utility method.

BUG=620172
==========

[Jinsuk Kim, 2017-03-23 01:47:17]

jinsukkim@chromium.org changed reviewers:
- nyquist@chromium.org

[Jinsuk Kim, 2017-03-23 01:47:32]

PTAL

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

https://codereview.chromium.org/2756813002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:3145:
boolean shouldOverride = KeyUtils.shouldOverrideKeyEvent(event);
On 2017/03/22 15:50:41, boliu wrote:
> why does webview need to check this, just check mContentsClient here, and CVC
> can keep checking its own version, and result would be the same?

Done. Put the line here because it is used in both cases but could have been
simpler + dup'ed less by just returning false as updated.

[boliu, 2017-03-23 03:59:37]

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

https://codereview.chromium.org/2756813002/diff/80001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:3148:
return KeyUtils.shouldOverrideKeyEvent(event)
you don't need to call KeyUtils here anymore, right? then KeyUntils don't need
to be public either?

[Jinsuk Kim, 2017-03-23 04:30:17]

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

https://codereview.chromium.org/2756813002/diff/80001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:3148:
return KeyUtils.shouldOverrideKeyEvent(event)
On 2017/03/23 03:59:37, boliu wrote:
> you don't need to call KeyUtils here anymore, right? then KeyUntils don't need
> to be public either?

Right - it will eventually be checked in CVC.dispatchKeyEvent. Removed.

[boliu, 2017-03-23 04:34:44]

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

https://codereview.chromium.org/2756813002/diff/80001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:3148:
return KeyUtils.shouldOverrideKeyEvent(event)
On 2017/03/23 04:30:17, Jinsuk Kim wrote:
> On 2017/03/23 03:59:37, boliu wrote:
> > you don't need to call KeyUtils here anymore, right? then KeyUntils don't
need
> > to be public either?
> 
> Right - it will eventually be checked in CVC.dispatchKeyEvent. Removed.

then KeyUntils don't need to be public in content anymore? the code can just say
in CVC

[Jinsuk Kim, 2017-03-23 05:00:25]

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

https://codereview.chromium.org/2756813002/diff/80001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:3148:
return KeyUtils.shouldOverrideKeyEvent(event)
On 2017/03/23 04:34:43, boliu wrote:
> On 2017/03/23 04:30:17, Jinsuk Kim wrote:
> > On 2017/03/23 03:59:37, boliu wrote:
> > > you don't need to call KeyUtils here anymore, right? then KeyUntils don't
> need
> > > to be public either?
> > 
> > Right - it will eventually be checked in CVC.dispatchKeyEvent. Removed.
> 
> then KeyUntils don't need to be public in content anymore? the code can just
say
> in CVC

Removed KeyUtils

[boliu, 2017-03-23 05:09:56]

lgtm

[Jinsuk Kim, 2017-03-23 07:30:18]

jinsukkim@chromium.org changed reviewers:
+ scottmg@chromium.org

[Jinsuk Kim, 2017-03-23 07:30:19]

scottmg@ could you give lgtm for the DEPS file with the new dependency on
+device/gamepad/android/java?

[scottmg, 2017-03-23 15:00:10]

deps lgtm

[Jinsuk Kim, 2017-03-24 02:48:28]

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

[commit-bot: I haz the power, 2017-03-24 02:48:48]

Dry run: 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-03-24 03:33:57]

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

[commit-bot: I haz the power, 2017-03-24 03:33:58]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Jinsuk Kim, 2017-03-24 06:46:31]

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

[Jinsuk Kim, 2017-03-24 06:46:46]

Patchset #8 (id:160001) has been deleted

[commit-bot: I haz the power, 2017-03-24 06:46:47]

Dry run: 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-03-24 08:26:01]

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

[commit-bot: I haz the power, 2017-03-24 08:26:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Jinsuk Kim, 2017-03-24 10:11:02]

The CQ bit was checked by jinsukkim@chromium.org

[Jinsuk Kim, 2017-03-24 10:11:02]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org,
scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/2756813002/#ps180001
(title: "fix bug/tests")

[commit-bot: I haz the power, 2017-03-24 10:11:23]

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-03-24 11:00:39]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1490350262375960,
"parent_rev": "b874c08258d7ffb293ecf1f779cdf2c61d03e23f", "commit_rev":
"219a962acb6807d9c694e16659d9388772eae457"}

[commit-bot: I haz the power, 2017-03-24 11:01:51]

Description was changed from

==========
Refactor ContentViewClient (5/6)

|shouldPropagetKeyEvent| was removed from the interface,
replaced with a static utility method.

BUG=620172
==========

to

==========
Refactor ContentViewClient (5/6)

|shouldPropagetKeyEvent| was removed from the interface,
replaced with a static utility method.

BUG=620172

Review-Url: https://codereview.chromium.org/2756813002
Cr-Commit-Position: refs/heads/master@{#459385}
Committed:
https://chromium.googlesource.com/chromium/src/+/219a962acb6807d9c694e16659d9...
==========

[commit-bot: I haz the power, 2017-03-24 11:01:52]

Committed patchset #8 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/219a962acb6807d9c694e16659d9...