Introduce Animated Logo to Chrome on Android

chrome/android/java/src/org/chromium/chrome/browser/ntp/LogoBridge.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/LogoBridge.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/LogoBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/LogoBridge.java
index cf2937d62c70aefdcfc75c4d139763be192ad4a1..0d4da005de3e5476e1269b98b7303dbda7165ae6 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/LogoBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/LogoBridge.java
@@ -5,6 +5,7 @@
 package org.chromium.chrome.browser.ntp;
 
 import android.graphics.Bitmap;
+import android.text.TextUtils;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.chrome.browser.profiles.Profile;
@@ -33,10 +34,43 @@ class LogoBridge {
          */
         public final String altText;
 
-        Logo(Bitmap image, String onClickUrl, String altText) {
+        /**
+         * The URL to download the animated GIF. If null, there is no GIF to download.
+         */
+        public final String gifUrl;

[newt (away), 2015/09/18 20:46:01]

animatedLogoUrl

[Ian Wen, 2015/09/22 21:39:05]

Done.

+
+        Logo(Bitmap image, String onClickUrl, String altText, String gifUrl) {
             this.image = image;
             this.onClickUrl = onClickUrl;
             this.altText = altText;
+            this.gifUrl = gifUrl;
+        }
+
+        @Override
+        public int hashCode() {
+            final int prime = 31;
+            int result = 1;
+            result = prime * result + ((altText == null) ? 0 : altText.hashCode());
+            result = prime * result + ((gifUrl == null) ? 0 : gifUrl.hashCode());
+            result = prime * result + ((image == null) ? 0 : image.hashCode());
+            result = prime * result + ((onClickUrl == null) ? 0 : onClickUrl.hashCode());
+            return result;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) return true;
+            if (!(obj instanceof Logo)) return false;
+            Logo other = (Logo) obj;
+            if (!TextUtils.equals(altText, other.altText)) return false;
+            if (!TextUtils.equals(gifUrl, other.gifUrl)) return false;
+            if (!TextUtils.equals(onClickUrl, other.onClickUrl)) return false;
+            if (image == null) {
+                if (other.image != null) return false;
+            } else if (!image.sameAs(other.image)) {

[newt (away), 2015/09/18 20:46:01]

Why sameAs()? This compares pixel config, but not the actual data. Would "==" be
acceptable in this case?

Even better would be to fix the underlying issue (multiple taps triggers
multiple animated logo downloads) so that we don't need to do equality
comparisons on logos.

[Ian Wen, 2015/09/22 21:39:05]

Removed.

+                return false;
+            }
+            return true;
         }
     }
 
@@ -55,6 +89,20 @@ class LogoBridge {
         void onLogoAvailable(Logo logo, boolean fromCache);
     }
 
+    /**
+     * A listener that is notified when the GIF file is successfully downloaded.
+     */
+    interface GifListener {

[newt (away), 2015/09/18 20:46:01]

Call this AnimatedLogoCallback or AnimatedLogoObserver. "Callback" is most
appropriate since whoever implements this is not merely "observing" that the
animated logo was downloaded, but is the sole actor that's handling the
post-download actions.

"Observer" or "Listener" would imply that some other code is handling the
completed download, and that we're  just interested in some side action that
should also happen when the download is complete.

(Admittedly, LogoObserver doesn't exactly follow this pattern. It's a bit weird
because multiple requests are coalesced, so there could be multiple
LogoObservers that handle the downloading of a single logo.)

[Ian Wen, 2015/09/22 21:39:05]

Done.

+
+        /**
+         * Called when the GIF is successfully downloaded.
+         *
+         * @param bytes The byte array representing the raw data for the GIF.

[newt (away), 2015/09/18 20:46:01]

"animated GIF"

[Ian Wen, 2015/09/22 21:39:05]

Done.

+         */
+        @CalledByNative("GifListener")
+        void onGifDownloaded(byte[] bytes);
+    }
+
     private long mNativeLogoBridge;
 
     /**
@@ -87,12 +135,23 @@ class LogoBridge {
         nativeGetCurrentLogo(mNativeLogoBridge, logoObserver);
     }
 
+    /**
+     * Asynchronously gets the GIF animation.

[newt (away), 2015/09/18 20:46:01]

"Downloads an animated GIF logo."

(It's pretty obvious that downloading is asynchronous :)

[Ian Wen, 2015/09/22 21:39:05]

Done.

+     * @param listener The listener that will be notified when the GIF is ready.
+     * @param gifUrl   The url from which to download the GIF.
+     */
+    void getGif(GifListener listener, String gifUrl) {

[newt (away), 2015/09/18 20:46:01]

"getAnimatedLogo"

[Ian Wen, 2015/09/22 21:39:05]

Done.

+        nativeGetAnimatedLogo(mNativeLogoBridge, listener, gifUrl);
+    }
+
     @CalledByNative
-    private static Logo createLogo(Bitmap image, String onClickUrl, String altText) {
-        return new Logo(image, onClickUrl, altText);
+    private static Logo createLogo(Bitmap image, String onClickUrl, String altText, String gifUrl) {
+        return new Logo(image, onClickUrl, altText, gifUrl);
     }
 
     private native long nativeInit(Profile profile);
     private native void nativeGetCurrentLogo(long nativeLogoBridge, LogoObserver logoObserver);
+    private native void nativeGetAnimatedLogo(long nativeLogoBridge, GifListener listenr,

[newt (away), 2015/09/18 20:46:01]

"listenr" typo, but it should be changed to "callback" anyway.

[Ian Wen, 2015/09/22 21:39:05]

Done.

+            String gifUrl);
     private native void nativeDestroy(long nativeLogoBridge);
 }