Refactor ZoomControlsDelegate out of ContentViewCore

Refactor ZoomControlsDelegate out of ContentViewCore

We can keep this logic entirely in AwContents, it doesn't need to be in
ContentViewCore. I'm using the existing GestureStateListener interface to signal
AwZoomControls of the relevant events, but I did have to add a few more events
to that interface. Hopefully they're OK and will work with our future plans to
refactor more input-handling stuff out of ContentViewCore.

Note on testing this: the Android WebView shell works for testing but I had to
knock out the "&& mAwContents.getSettings().shouldDisplayZoomControls()" check
in getZoomController() to enable the zoom controls. Once you do this they appear
when scrolling.

BUG=664339
Committed: https://crrev.com/6ea15211ce9bda01f3ef0a014d30997b5fe06769
Cr-Commit-Position: refs/heads/master@{#438045}

[rlanday, 2016-12-08 01:49:10]

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

[commit-bot: I haz the power, 2016-12-08 01:50:03]

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

[rlanday, 2016-12-08 01:51:41]

rlanday@chromium.org changed reviewers:
+ aelias@chromium.org, boliu@chromium.org, jinsukkim@chromium.org

[rlanday, 2016-12-08 01:51:41]

rlanday@chromium.org changed required reviewers:
+ boliu@chromium.org

[rlanday, 2016-12-08 01:52:25]

I think this works, still waiting for the trybots to run but the zoom controls
worked fine on my test device

[rlanday, 2016-12-08 02:11:05]

On 2016/12/08 at 01:52:25, rlanday wrote:
> I think this works, still waiting for the trybots to run but the zoom controls
worked fine on my test device

aelias suggested that we should look at the other GestureEventListener instances
and try to figure out if GestureEventListener is something we want to keep
around or if it too can be refactored into something cleaner

[commit-bot: I haz the power, 2016-12-08 03:14:00]

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

[commit-bot: I haz the power, 2016-12-08 03:14:01]

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_...)

[boliu, 2016-12-08 06:08:05]

https://codereview.chromium.org/2561463004/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content_public/browser/GestureStateListener.java
(right):

https://codereview.chromium.org/2561463004/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content_public/browser/GestureStateListener.java:81:
public void onVisibilityChanged(int visibility) {}
visibility and detach are signals *from* the embedder, in webview's case, from
AwContents, there is no need to tell embdder about these

[rlanday, 2016-12-08 18:32:01]

https://codereview.chromium.org/2561463004/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content_public/browser/GestureStateListener.java
(right):

https://codereview.chromium.org/2561463004/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content_public/browser/GestureStateListener.java:81:
public void onVisibilityChanged(int visibility) {}
On 2016/12/08 at 06:08:05, boliu wrote:
> visibility and detach are signals *from* the embedder, in webview's case, from
AwContents, there is no need to tell embdder about these

Oh cool, let me clean this up...

[rlanday, 2016-12-08 20:17:26]

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

[commit-bot: I haz the power, 2016-12-08 20:18:06]

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

[rlanday, 2016-12-08 20:19:12]

I cleaned this up as requested, the ScreenOrientationListenerTest that was
failing on the trybot is passing on my test device

[boliu, 2016-12-08 21:29:07]

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

https://codereview.chromium.org/2561463004/diff/20001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:3247: if
(!viewVisible) {
I was gonna suggest move this to where we call CVC.onVisibilityChanged, but
webview never actually call that method! it was dead code!

so just remove this as well

https://codereview.chromium.org/2561463004/diff/20001/content/public/android/...
File
content/public/android/java/src/org/chromium/content_public/browser/GestureStateListener.java
(right):

https://codereview.chromium.org/2561463004/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content_public/browser/GestureStateListener.java:78:
* Called when the gesture source (ContentViewCore)'s containing view's
remove these two?

[rlanday, 2016-12-08 22:18:59]

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

[commit-bot: I haz the power, 2016-12-08 22:19:31]

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

[rlanday, 2016-12-08 22:19:34]

Updated with requested changes

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

https://codereview.chromium.org/2561463004/diff/20001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:3247: if
(!viewVisible) {
On 2016/12/08 at 21:29:07, boliu wrote:
> I was gonna suggest move this to where we call CVC.onVisibilityChanged, but
webview never actually call that method! it was dead code!
> 
> so just remove this as well

Oh interesting

https://codereview.chromium.org/2561463004/diff/20001/content/public/android/...
File
content/public/android/java/src/org/chromium/content_public/browser/GestureStateListener.java
(right):

https://codereview.chromium.org/2561463004/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content_public/browser/GestureStateListener.java:78:
* Called when the gesture source (ContentViewCore)'s containing view's
On 2016/12/08 at 21:29:07, boliu wrote:
> remove these two?

oops

[commit-bot: I haz the power, 2016-12-08 22:53:49]

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

[commit-bot: I haz the power, 2016-12-08 22:53:49]

Dry run: This issue passed the CQ dry run.

[boliu, 2016-12-10 22:11:59]

lgtm

think I meant to do that 2 days ago..

[rlanday, 2016-12-12 19:00:27]

The CQ bit was checked by rlanday@chromium.org

[rlanday, 2016-12-12 19:00:27]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2561463004/#ps60001
(title: "Rebase (no actual changes needed)")

[commit-bot: I haz the power, 2016-12-12 19:01: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, 2016-12-12 21:05:19]

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

[commit-bot: I haz the power, 2016-12-12 21:05:21]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[rlanday, 2016-12-12 22:11:13]

The CQ bit was checked by rlanday@chromium.org

[rlanday, 2016-12-12 22:11:57]

I tried some of the failing tests on master, they're failing there too,
hopefully they'll start working soonish...

[commit-bot: I haz the power, 2016-12-12 22:12:19]

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, 2016-12-13 01:05:15]

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

[commit-bot: I haz the power, 2016-12-13 01:05:16]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[rlanday, 2016-12-13 03:04:30]

The CQ bit was checked by rlanday@chromium.org

[commit-bot: I haz the power, 2016-12-13 03:04:59]

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, 2016-12-13 03:52:29]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481598270497380,
"parent_rev": "82e4fd0520cd68c843125b868f5e4fbaa876ef50", "commit_rev":
"fabfe51366d0ed75649495135316a2437888ec74"}

[commit-bot: I haz the power, 2016-12-13 03:52:56]

Description was changed from

==========
Refactor ZoomControlsDelegate out of ContentViewCore

We can keep this logic entirely in AwContents, it doesn't need to be in
ContentViewCore. I'm using the existing GestureStateListener interface to signal
AwZoomControls of the relevant events, but I did have to add a few more events
to that interface. Hopefully they're OK and will work with our future plans to
refactor more input-handling stuff out of ContentViewCore.

Note on testing this: the Android WebView shell works for testing but I had to
knock out the "&& mAwContents.getSettings().shouldDisplayZoomControls()" check
in getZoomController() to enable the zoom controls. Once you do this they appear
when scrolling.

BUG=664339
==========

to

==========
Refactor ZoomControlsDelegate out of ContentViewCore

We can keep this logic entirely in AwContents, it doesn't need to be in
ContentViewCore. I'm using the existing GestureStateListener interface to signal
AwZoomControls of the relevant events, but I did have to add a few more events
to that interface. Hopefully they're OK and will work with our future plans to
refactor more input-handling stuff out of ContentViewCore.

Note on testing this: the Android WebView shell works for testing but I had to
knock out the "&& mAwContents.getSettings().shouldDisplayZoomControls()" check
in getZoomController() to enable the zoom controls. Once you do this they appear
when scrolling.

BUG=664339

Review-Url: https://codereview.chromium.org/2561463004
==========

[commit-bot: I haz the power, 2016-12-13 03:52:57]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-13 03:55:00]

Description was changed from

==========
Refactor ZoomControlsDelegate out of ContentViewCore

We can keep this logic entirely in AwContents, it doesn't need to be in
ContentViewCore. I'm using the existing GestureStateListener interface to signal
AwZoomControls of the relevant events, but I did have to add a few more events
to that interface. Hopefully they're OK and will work with our future plans to
refactor more input-handling stuff out of ContentViewCore.

Note on testing this: the Android WebView shell works for testing but I had to
knock out the "&& mAwContents.getSettings().shouldDisplayZoomControls()" check
in getZoomController() to enable the zoom controls. Once you do this they appear
when scrolling.

BUG=664339

Review-Url: https://codereview.chromium.org/2561463004
==========

to

==========
Refactor ZoomControlsDelegate out of ContentViewCore

We can keep this logic entirely in AwContents, it doesn't need to be in
ContentViewCore. I'm using the existing GestureStateListener interface to signal
AwZoomControls of the relevant events, but I did have to add a few more events
to that interface. Hopefully they're OK and will work with our future plans to
refactor more input-handling stuff out of ContentViewCore.

Note on testing this: the Android WebView shell works for testing but I had to
knock out the "&& mAwContents.getSettings().shouldDisplayZoomControls()" check
in getZoomController() to enable the zoom controls. Once you do this they appear
when scrolling.

BUG=664339

Committed: https://crrev.com/6ea15211ce9bda01f3ef0a014d30997b5fe06769
Cr-Commit-Position: refs/heads/master@{#438045}
==========

[commit-bot: I haz the power, 2016-12-13 03:55:01]

Patchset 4 (id:??) landed as
https://crrev.com/6ea15211ce9bda01f3ef0a014d30997b5fe06769
Cr-Commit-Position: refs/heads/master@{#438045}