Introduce Animated Logo to Chrome on Android

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

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
index 2edbf6eaf404192a0a77899c15165133e9f358f4..f66e715b482a5b472430e770c36a5195a662a4fd 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
@@ -10,6 +10,7 @@ import android.content.Context;
 import android.graphics.Bitmap;
 import android.graphics.Canvas;
 import android.graphics.Rect;
+import android.text.TextUtils;
 import android.view.ContextMenu;
 import android.view.LayoutInflater;
 import android.view.Menu;
@@ -55,6 +56,8 @@ import org.chromium.ui.base.PageTransition;
 
 import java.util.concurrent.TimeUnit;
 
+import jp.tomorrowkey.android.gifplayer.BaseGifImage;
+
 /**
  * Provides functionality when the user interacts with the NTP.
  */
@@ -87,6 +90,7 @@ public class NewTabPage
     private boolean mSearchProviderHasLogo;
     private final boolean mOptOutPromoShown;
     private String mOnLogoClickUrl;
+    private String mAnimatedLogoUrl;
     private FakeboxDelegate mFakeboxDelegate;
 
     // The timestamp at which the constructor was called.
@@ -350,10 +354,22 @@ public class NewTabPage
         }
 
         @Override
-        public void openLogoLink() {
+        public void onLogoClicked(boolean isAnimatedLogoShowing) {
             if (mIsDestroyed) return;
-            if (mOnLogoClickUrl == null) return;
-            mTab.loadUrl(new LoadUrlParams(mOnLogoClickUrl, PageTransition.LINK));
+
+            boolean shouldDownloadAnimatedLogo = !isAnimatedLogoShowing
+                    && !TextUtils.isEmpty(mAnimatedLogoUrl);

[newt (away), 2015/09/23 20:38:43]

I'd prefer to continue using simple null checks, rather than TextUtils.isEmpty()
here (and on line 370). TextUtils.isEmpty() is a bit viral: once you start using
it, you have to use it everywhere.

[Ian Wen, 2015/09/23 21:59:51]

(≧σ≦)

+            if (shouldDownloadAnimatedLogo) {
+                mLogoBridge.getAnimatedLogo(new LogoBridge.AnimatedLogoCallback() {
+                    @Override
+                    public void onGifDownloaded(byte[] bytes) {
+                        if (mIsDestroyed) return;
+                        mNewTabPageView.playAnimatedLogo(new BaseGifImage(bytes));

[newt (away), 2015/09/23 20:38:43]

Currently, we construct the BaseGifImage here, but the BaseGifDrawable inside
NewTabPageView. Splitting this apart is a bit confusing. I'd do all the
BaseGifImage/BaseGifDrawable work in the same place: either here or in
NewTabPageView (or maybe LogoBridge).

[Ian Wen, 2015/09/23 21:59:51]

Done. Moved it to bridge. I dislike passing bytes to a view...

+                    }
+                }, mAnimatedLogoUrl);
+            } else if (!TextUtils.isEmpty(mOnLogoClickUrl)) {
+                mTab.loadUrl(new LoadUrlParams(mOnLogoClickUrl, PageTransition.LINK));
+            }
         }
 
         @Override
@@ -364,6 +380,7 @@ public class NewTabPage
                 public void onLogoAvailable(Logo logo, boolean fromCache) {
                     if (mIsDestroyed) return;
                     mOnLogoClickUrl = logo != null ? logo.onClickUrl : null;
+                    mAnimatedLogoUrl = logo != null ? logo.animagedLogoUrl : null;
                     logoObserver.onLogoAvailable(logo, fromCache);
                 }
             };