Refactor ContentViewClient (6/6)

Refactor ContentViewClient (6/6)

Moves the single method left in ContentViewClient
|getSystemWindowInsetBottom| to |ViewAndroidDelegate|
and deletes the interface entirely.

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

[Jinsuk Kim, 2017-04-06 11:59:58]

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

[commit-bot: I haz the power, 2017-04-06 12:00:11]

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

[Jinsuk Kim, 2017-04-06 12:54:41]

jinsukkim@chromium.org changed reviewers:
+ boliu@chromium.org, halliwell@chromium.org, tedchoc@chromium.org

[Jinsuk Kim, 2017-04-06 12:54:42]

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

boliu@chromium.org: Please review changes in chromecast/

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

[commit-bot: I haz the power, 2017-04-06 13:04:17]

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

[commit-bot: I haz the power, 2017-04-06 13:04:18]

Dry run: This issue passed the CQ dry run.

[halliwell, 2017-04-06 14:36:31]

On 2017/04/06 12:54:42, Jinsuk Kim wrote:
> mailto:halliwell@chromium.org: Please review changes in content/, ui/
> 
> mailto:boliu@chromium.org: Please review changes in chromecast/
> 
> mailto:tedchoc@chromium.org: Please review changes in chrome/

chromecast/ lgtm

[boliu, 2017-04-06 18:39:03]

lgtm

[Ted C, 2017-04-06 19:07:55]

https://codereview.chromium.org/2804953003/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/OSKOverscrollTest.java
(right):

https://codereview.chromium.org/2804953003/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/OSKOverscrollTest.java:156:
TabViewAndroidDelegate tvad = new TabViewAndroidDelegate(
this doesn't seem to be in the spirit of the test.  We are creating a new object
that may or may not be sending the same values as those received by the
renderer.

I think we should expose the ViewAndroidDelegate from CVC or find some why to
query this from the view layer instead of like this (ideally, this test wouldn't
know anything about TabViewAndroidDelegate)

[Jinsuk Kim, 2017-04-06 22:08:46]

https://codereview.chromium.org/2804953003/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/OSKOverscrollTest.java
(right):

https://codereview.chromium.org/2804953003/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/OSKOverscrollTest.java:156:
TabViewAndroidDelegate tvad = new TabViewAndroidDelegate(
On 2017/04/06 19:07:55, Ted C wrote:
> this doesn't seem to be in the spirit of the test.  We are creating a new
object
> that may or may not be sending the same values as those received by the
> renderer.
> 
> I think we should expose the ViewAndroidDelegate from CVC or find some why to
> query this from the view layer instead of like this (ideally, this test
wouldn't
> know anything about TabViewAndroidDelegate)

Point taken. In fact this test defeats the goal of the refactoring because I'm
trying to avoid the flow for inset value going through CVC. Though a bit higher
than view level, tapping into current tab so that we can ensure the same value
as we expect will be provided will work too, I think. PTAL.

[Ted C, 2017-04-10 16:31:45]

On 2017/04/06 22:08:46, Jinsuk Kim wrote:
>
https://codereview.chromium.org/2804953003/diff/1/chrome/android/javatests/sr...
> File
>
chrome/android/javatests/src/org/chromium/chrome/browser/OSKOverscrollTest.java
> (right):
> 
>
https://codereview.chromium.org/2804953003/diff/1/chrome/android/javatests/sr...
>
chrome/android/javatests/src/org/chromium/chrome/browser/OSKOverscrollTest.java:156:
> TabViewAndroidDelegate tvad = new TabViewAndroidDelegate(
> On 2017/04/06 19:07:55, Ted C wrote:
> > this doesn't seem to be in the spirit of the test.  We are creating a new
> object
> > that may or may not be sending the same values as those received by the
> > renderer.
> > 
> > I think we should expose the ViewAndroidDelegate from CVC or find some why
to
> > query this from the view layer instead of like this (ideally, this test
> wouldn't
> > know anything about TabViewAndroidDelegate)
> 
> Point taken. In fact this test defeats the goal of the refactoring because I'm
> trying to avoid the flow for inset value going through CVC. Though a bit
higher
> than view level, tapping into current tab so that we can ensure the same value
> as we expect will be provided will work too, I think. PTAL.

thank you, chrome/ lgtm

[Jinsuk Kim, 2017-04-12 09:00:03]

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

[commit-bot: I haz the power, 2017-04-12 09:00:24]

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

[Jinsuk Kim, 2017-04-12 09:28:35]

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

[commit-bot: I haz the power, 2017-04-12 09:28:50]

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-04-12 11:39:27]

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

[commit-bot: I haz the power, 2017-04-12 11:39:28]

Dry run: This issue passed the CQ dry run.

[Jinsuk Kim, 2017-04-12 12:22:55]

The CQ bit was checked by jinsukkim@chromium.org

[Jinsuk Kim, 2017-04-12 12:22:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
halliwell@chromium.org, boliu@chromium.org, tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2804953003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-12 12:23:08]

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-12 12:30:30]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491999775691990,
"parent_rev": "6c31098c891b599a7592a9af49f30d600b62d354", "commit_rev":
"a52e8dc5d782827ce7e426ebde5537949ef4cb01"}

[commit-bot: I haz the power, 2017-04-12 12:31:27]

Description was changed from

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

Moves the single method left in ContentViewClient
|getSystemWindowInsetBottom| to |ViewAndroidDelegate|
and deletes the interface entirely.

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

to

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

Moves the single method left in ContentViewClient
|getSystemWindowInsetBottom| to |ViewAndroidDelegate|
and deletes the interface entirely.

BUG=620172

Review-Url: https://codereview.chromium.org/2804953003
Cr-Commit-Position: refs/heads/master@{#464003}
Committed:
https://chromium.googlesource.com/chromium/src/+/a52e8dc5d782827ce7e426ebde55...
==========

[commit-bot: I haz the power, 2017-04-12 12:31:28]

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