[Android] Use the platform SwipeRefreshLayout

[Android] Use the platform SwipeRefreshLayout

Adopt the compat library's implementation of the pull-to-refresh effect,
removing the old compositor-based approach.

This depends directly on the SwipeRefreshLayout addition in
https://codereview.chromium.org/897663003/.

BUG=428429, 444134
Committed: https://crrev.com/b003775553a0b51cbbd8bd25a5b9c4a27f46550c
Cr-Commit-Position: refs/heads/master@{#325172}

[jdduke (slow), 2015-03-20 17:02:07]

jdduke@chromium.org changed reviewers:
+ aurimas@chromium.org, tedchoc@chromium.org

[jdduke (slow), 2015-03-20 17:02:08]

Here's the fun. If you gentlemen could take a look at the following:

aurimas@: content/browser/android
tedchoc@: chrome/android

I'd also like some guidance on how best to modify the third party content. The
changes to MaterialProgressDrawable and CircleImageView are pretty basic, simply
removing some annotations and some app compat dependencies. The changes to
SwipeRrefreshLayout are a bit more invasive, and I tried to make it obvious
there what changes were Chrome-specific, but perhaps there's a more canonical
way of doing that.

https://codereview.chromium.org/894193005/diff/180001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/SwipeRefreshLayoutContainer.java
(right):

https://codereview.chromium.org/894193005/diff/180001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/SwipeRefreshLayoutContainer.java:42:
public SwipeRefreshLayoutContainer(Context context) {
I considered making this a standalone object that had a native counterpart, and
you could just feed it a ContentViewCore ref when constructing it. That approach
would cut down on the ContentViewCore glue for passing the overscroll calls, but
that leaves styling (R.color.light_active_color) configuration issues if it is
constructed in the content/ layer  (unless we want to bundle more styling into
content/).

Regardless, I somewhat prefer embedding this at the chrome/ layer.

[jdduke (slow), 2015-03-25 20:08:16]

OK, I've tested this on N7, N7v2, GN, N4, N5, N9, HTC One M8, and Sprout, and
think I have all the inconsistencies ironed out.

[aurimas (slooooooooow), 2015-03-26 20:25:26]

It looks reasonable to me, but I think Ted should have a pass over this too.

https://codereview.chromium.org/894193005/diff/260001/third_party/android_swi...
File
third_party/android_swipe_refresh/java/src/org/chromium/third_party/android/swiperefresh/SwipeRefreshLayout.java
(right):

https://codereview.chromium.org/894193005/diff/260001/third_party/android_swi...
third_party/android_swipe_refresh/java/src/org/chromium/third_party/android/swiperefresh/SwipeRefreshLayout.java:182:
private static final boolean IS_CHROME_BUILD = true;
Is this always going to be set to true? Can we just delete some code then or are
you trying to keep modifications minimal?

[jdduke (slow), 2015-03-26 20:30:10]

https://codereview.chromium.org/894193005/diff/260001/third_party/android_swi...
File
third_party/android_swipe_refresh/java/src/org/chromium/third_party/android/swiperefresh/SwipeRefreshLayout.java
(right):

https://codereview.chromium.org/894193005/diff/260001/third_party/android_swi...
third_party/android_swipe_refresh/java/src/org/chromium/third_party/android/swiperefresh/SwipeRefreshLayout.java:182:
private static final boolean IS_CHROME_BUILD = true;
On 2015/03/26 20:25:25, aurimas wrote:
> Is this always going to be set to true? Can we just delete some code then or
are
> you trying to keep modifications minimal?

Yeah, I was trying to make it obvious what changes were made for Chrome in the
event that we need to update this with upstream changes.

However, I've had to make enough changes that I wonder if it would be cleanest
if I simply rip out all the unused code and make a few top-level notes about the
changes?

[jdduke (slow), 2015-03-27 20:15:15]

https://codereview.chromium.org/894193005/diff/260001/third_party/android_swi...
File
third_party/android_swipe_refresh/java/src/org/chromium/third_party/android/swiperefresh/SwipeRefreshLayout.java
(right):

https://codereview.chromium.org/894193005/diff/260001/third_party/android_swi...
third_party/android_swipe_refresh/java/src/org/chromium/third_party/android/swiperefresh/SwipeRefreshLayout.java:684:
public boolean onTouchEvent(MotionEvent ev) {
What I might do is just rip out the onTouchEvent/onInterceptTouchEvent, as that
functionality has moved more or less 1-to-1 to the start/pull/release functions.
Also I can remove everything that refers to |mTarget|, as it's confusing and
misleading (the normal SwipeRefreshLayout *contains* the view, mTarget, that is
being refreshed, ours simply overlays it). Ted, thoughts?

[Ted C, 2015-04-02 00:58:46]

On 2015/03/27 20:15:15, jdduke wrote:
>
https://codereview.chromium.org/894193005/diff/260001/third_party/android_swi...
> File
>
third_party/android_swipe_refresh/java/src/org/chromium/third_party/android/swiperefresh/SwipeRefreshLayout.java
> (right):
> 
>
https://codereview.chromium.org/894193005/diff/260001/third_party/android_swi...
>
third_party/android_swipe_refresh/java/src/org/chromium/third_party/android/swiperefresh/SwipeRefreshLayout.java:684:
> public boolean onTouchEvent(MotionEvent ev) {
> What I might do is just rip out the onTouchEvent/onInterceptTouchEvent, as
that
> functionality has moved more or less 1-to-1 to the start/pull/release
functions.
> Also I can remove everything that refers to |mTarget|, as it's confusing and
> misleading (the normal SwipeRefreshLayout *contains* the view, mTarget, that
is
> being refreshed, ours simply overlays it). Ted, thoughts?

It is quite sad that you had to make such extensive changes in that file.  At
this point,
any merges from the original code will be huge ordeals, so I think it is fine to
make
any cuts you see fit to make this more maintainable.  If they do end up changing
a
lot, I wonder if we'll get into a state where maintaining the code is more
effort
than rewriting the effect ourselves.

Ideally we could extract some shareable component in the appcompat library, but
I feel
that might be extremely hard.

Also, it is sad to me that CVC needs to be a plumbing conduit for this, but at
first
blush it seems like it might be the only option.

None the less, I'm fine with the proposal and can do a more thorough review of
it
for more targeted comments.

[jdduke (slow), 2015-04-10 22:21:26]

Ted, anybody you'd feel comfortable reviewing this while you're gone? I'm also
happy to wait. I've updated SwipeRefreshLayout, went ahead and just culled the
code we don't, and will 99% most likely never, need. Seems a bit cleaner that
way. It sounds like there aren't any major overhauls planned, just a few
cosmetic fixes that I've already addressed.

[Ted C, 2015-04-10 22:40:19]

On 2015/04/10 22:21:26, jdduke wrote:
> Ted, anybody you'd feel comfortable reviewing this while you're gone? I'm also
> happy to wait. I've updated SwipeRefreshLayout, went ahead and just culled the
> code we don't, and will 99% most likely never, need. Seems a bit cleaner that
> way. It sounds like there aren't any major overhauls planned, just a few
> cosmetic fixes that I've already addressed.

Aurimas already started looking at it, so I'm certainly fine with him taking it
the rest of the way.

[jdduke (slow), 2015-04-13 19:09:13]

jdduke@chromium.org changed reviewers:
+ aelias@chromium.org, dtrainor@chromium.org, sky@chromium.org

[jdduke (slow), 2015-04-13 19:09:14]

OK aurimas@, anything else you'd like to see here?

+aelias for content/browser/renderer_host
+sky for chrome/chrome.gyp
+dtrainor for chrome/android/java/

[David Trainor- moved to gerrit, 2015-04-13 19:37:44]

chrome/android/java lgtm

[sky, 2015-04-13 19:38:55]

LGTM

[jdduke (slow), 2015-04-13 19:52:16]

Patchset #19 (id:360001) has been deleted

[aelias_OOO_until_Jul13, 2015-04-13 20:02:41]

renderer_host lgtm

[aurimas (slooooooooow), 2015-04-14 21:59:36]

LGTM, all the Java code looks reasonable to me.

[jdduke (slow), 2015-04-15 00:57:27]

The CQ bit was checked by jdduke@chromium.org

[jdduke (slow), 2015-04-15 00:57:28]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
dtrainor@chromium.org, aelias@chromium.org, aurimas@chromium.org
Link to the patchset: https://codereview.chromium.org/894193005/#ps430001
(title: "Rebase")

[commit-bot: I haz the power, 2015-04-15 00:57:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/894193005/430001

[commit-bot: I haz the power, 2015-04-15 01:55:19]

Committed patchset #22 (id:430001)

[commit-bot: I haz the power, 2015-04-15 01:56:36]

Patchset 22 (id:??) landed as
https://crrev.com/b003775553a0b51cbbd8bd25a5b9c4a27f46550c
Cr-Commit-Position: refs/heads/master@{#325172}