[Feed] Implement Image Loader Host API in Java

Bug:807364

Change-Id: I9890811bc55169435d14b5b5c35e6014c2bbaa99
Reviewed-on: https://chromium-review.googlesource.com/1047505
Commit-Queue: Gang Wu <gangwu@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558078}

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedImageLoader.java

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.feed;
+
+import android.content.Context;
+import android.graphics.Bitmap;
+import android.graphics.drawable.BitmapDrawable;
+import android.graphics.drawable.Drawable;
+import android.support.v7.content.res.AppCompatResources;
+
+import com.google.android.libraries.feed.common.functional.Consumer;
+import com.google.android.libraries.feed.host.imageloader.ImageLoaderApi;
+
+import org.chromium.base.Callback;
+import org.chromium.chrome.browser.profiles.Profile;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Provides image loading and other host-specific asset fetches for Feed.
+ */
+public class FeedImageLoader implements ImageLoaderApi {
+    private static final String ASSET_PREFIX = "asset://";
+    private static final String DRAWABLE_RESOURCE_TYPE = "drawable";
+
+    private FeedImageLoaderBridge mFeedImageLoaderBridge;
+    private Context mActivityContext;
+
+    /**
+     * Creates a FeedImageLoader for fetching image for the current user.
+     *
+     * @param profile Profile of the user we are rendering the Feed for.
+     * @param activityContext Context of the user we are rendering the Feed for.
+     */
+    public FeedImageLoader(Profile profile, Context activityContext) {
+        this(profile, activityContext, new FeedImageLoaderBridge());
+    }
+
+    /**
+     * Creates a FeedImageLoader for fetching image for the current user.
+     *
+     * @param profile Profile of the user we are rendering the Feed for.
+     * @param activityContext Context of the user we are rendering the Feed for.
+     * @param bridge The FeedImageLoaderBridge implementation can handle fetching image request.
+     */
+    public FeedImageLoader(Profile profile, Context activityContext, FeedImageLoaderBridge bridge) {
+        mFeedImageLoaderBridge = bridge;
+        mFeedImageLoaderBridge.init(profile);
+        mActivityContext = activityContext;
+    }
+
+    @Override
+    public void loadDrawable(List<String> urls, Consumer<Drawable> consumer) {
+        assert mFeedImageLoaderBridge != null;
+        List<String> assetUrls = new ArrayList<>();
+        List<String> networkUrls = new ArrayList<>();
+
+        // Since loading APK resource("asset://"") only can be done in Java side, we filter out
+        // asset urls, and pass the other URLs to C++ side. This will change the order of |urls|,
+        // because we will process asset:// URLs after network URLs, but once
+        // https://crbug.com/840578 resolved, we can process URLs ordering same as |urls|.
+        for (String url : urls) {
+            if (url.startsWith(ASSET_PREFIX)) {
+                assetUrls.add(url);
+            } else {
+                // Assume this is a web image.
+                networkUrls.add(url);
+            }
+        }
+
+        if (networkUrls.size() == 0) {
+            Drawable drawable = getAssetDrawable(assetUrls);
+            consumer.accept(drawable);
+            return;
+        }
+
+        mFeedImageLoaderBridge.fetchImage(networkUrls, new Callback<Bitmap>() {
+            @Override
+            public void onResult(Bitmap bitmap) {
+                if (bitmap != null) {
+                    Drawable drawable = new BitmapDrawable(mActivityContext.getResources(), bitmap);
+                    consumer.accept(drawable);
+                    return;
+                }
+
+                // Since no image was available for downloading over the network, attempt to load a
+                // drawable locally.
+                Drawable drawable = getAssetDrawable(assetUrls);
+                consumer.accept(drawable);
+            }
+        });
+    }
+
+    /** Cleans up FeedImageLoaderBridge. */
+    public void destroy() {
+        assert mFeedImageLoaderBridge != null;
+
+        mFeedImageLoaderBridge.destroy();
+        mFeedImageLoaderBridge = null;
+    }
+
+    private Drawable getAssetDrawable(List<String> assetUrls) {
+        for (String url : assetUrls) {
+            String resourceName = url.substring(ASSET_PREFIX.length());
+            int resourceId = mActivityContext.getResources().getIdentifier(
+                    resourceName, DRAWABLE_RESOURCE_TYPE, mActivityContext.getPackageName());
+            if (resourceId != 0) {
+                Drawable drawable = AppCompatResources.getDrawable(mActivityContext, resourceId);
+                if (drawable != null) {
+                    return drawable;
+                }
+            }
+        }
+        return null;
+    }
+}

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedImageLoaderBridge.java

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.feed;
+
+import android.graphics.Bitmap;
+
+import org.chromium.base.Callback;
+import org.chromium.base.annotations.JNINamespace;
+import org.chromium.chrome.browser.profiles.Profile;
+
+import java.util.List;
+
+/**
+ * Provides access to native implementations of image loader.
+ */
+@JNINamespace("feed")
+public class FeedImageLoaderBridge {
+    private long mNativeFeedImageLoaderBridge;
+
+    /**
+     * Creates a FeedImageLoaderBridge for accessing native image loader implementation for the
+     * current user.
+     */
+    public FeedImageLoaderBridge() {}
+
+    /**
+     * Inits native side bridge.
+     *
+     * @param profile Profile of the user we are rendering the Feed for.
+     */
+    public void init(Profile profile) {
+        mNativeFeedImageLoaderBridge = nativeInit(profile);
+    }
+
+    /** Cleans up native half of this bridge. */
+    public void destroy() {
+        assert mNativeFeedImageLoaderBridge != 0;
+        nativeDestroy(mNativeFeedImageLoaderBridge);
+        mNativeFeedImageLoaderBridge = 0;
+    }
+
+    /**
+     * Fetches images for feed. A {@code null} Bitmap is returned if no image is available. The
+     * callback is never called synchronously.
+     */
+    public void fetchImage(List<String> urls, Callback<Bitmap> callback) {
+        assert mNativeFeedImageLoaderBridge != 0;
+
+        String[] urlsArray = urls.toArray(new String[urls.size()]);
+
+        nativeFetchImage(mNativeFeedImageLoaderBridge, urlsArray, callback);
+    }
+
+    // Native methods
+    private native long nativeInit(Profile profile);
+    private native void nativeDestroy(long nativeFeedImageLoaderBridge);
+    private native void nativeFetchImage(
+            long nativeFeedImageLoaderBridge, String[] urls, Callback<Bitmap> callback);
+}

chrome/android/feed/feed_java_sources.gni

@@ -9,6 +9,8 @@ if (enable_feed_in_chrome) {
   feed_deps = [ "//third_party/feed:feed_lib_java" ]
 
   feed_java_sources = [
+    "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedImageLoader.java",
+    "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedImageLoaderBridge.java",
     "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedNetworkBridge.java",
     "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedNewTabPage.java",
     "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/action/FeedActionHandler.java",

chrome/android/java_sources.gni

@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//components/feed/features.gni")
 import("//components/offline_pages/buildflags/features.gni")
 
 chrome_java_sources = [
@@ -2096,3 +2097,8 @@ if (enable_offline_pages_harness) {
   chrome_java_sources += [ "java/src/org/chromium/chrome/browser/offlinepages/evaluation/OfflinePageEvaluationBridge.java" ]
   chrome_test_java_sources += [ "javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageSavePageLaterEvaluationTest.java" ]
 }
+
+if (enable_feed_in_chrome) {
+  chrome_junit_test_java_sources +=
+      [ "junit/src/org/chromium/chrome/browser/feed/FeedImageLoaderTest.java" ]
+}

chrome/android/junit/src/org/chromium/chrome/browser/feed/FeedImageLoaderTest.java

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.feed;
+
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import android.graphics.Bitmap;
+import android.graphics.drawable.Drawable;
+import android.support.test.filters.SmallTest;
+
+import com.google.android.libraries.feed.common.functional.Consumer;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.AdditionalMatchers;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.robolectric.annotation.Config;
+
+import org.chromium.base.Callback;
+import org.chromium.base.ContextUtils;
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.chrome.browser.profiles.Profile;
+
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * Unit tests for {@link FeedImageLoader}.
+ */
+@RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class FeedImageLoaderTest {
+    public static final String HTTP_STRING1 = "http://www.test1.com";
+    public static final String HTTP_STRING2 = "http://www.test2.com";
+    public static final String HTTP_STRING3 = "http://www.test3.com";
+    public static final String ASSET_STRING = "asset://logo_avatar_anonymous";
+
+    @Mock
+    private FeedImageLoaderBridge mBridge;
+    @Mock
+    private Consumer<Drawable> mConsumer;
+    @Mock
+    private Profile mProfile;
+    @Captor
+    ArgumentCaptor<List<String>> mUrlListArgument;
+    @Captor
+    ArgumentCaptor<Callback<Bitmap>> mCallbackArgument;
+
+    private FeedImageLoader mImageLoader;
+
+    private class FakeConsumer implements Consumer<Drawable> {
+        public Drawable mResponse = null;
+
+        @Override
+        public void accept(Drawable value) {
+            mResponse = value;
+        }
+
+        public void clearResponse() {
+            mResponse = null;
+        }
+    }
+
+    @Before
+    public void setUp() throws Exception {
+        MockitoAnnotations.initMocks(this);
+        doNothing().when(mBridge).init(eq(mProfile));
+        mImageLoader = new FeedImageLoader(mProfile, ContextUtils.getApplicationContext(), mBridge);
+        verify(mBridge, times(1)).init(eq(mProfile));
+        answerFetchImage(null);
+    }
+
+    private void answerFetchImage(Bitmap bitmap) {
+        Answer<Void> answer = new Answer<Void>() {
+            @Override
+            public Void answer(InvocationOnMock invocation) {
+                mCallbackArgument.getValue().onResult(bitmap);
+
+                return null;
+            }
+        };
+        doAnswer(answer).when(mBridge).fetchImage(
+                mUrlListArgument.capture(), mCallbackArgument.capture());
+    }
+
+    @Test
+    @SmallTest
+    public void downloadImageTest() {
+        List<String> urls = Arrays.asList(HTTP_STRING1);
+        mImageLoader.loadDrawable(urls, mConsumer);
+
+        verify(mBridge, times(1))
+                .fetchImage(mUrlListArgument.capture(), mCallbackArgument.capture());
+    }
+
+    @Test
+    @SmallTest
+    public void onlyNetworkURLSendToBridgeTest() {
+        List<String> urls = Arrays.asList(HTTP_STRING1, HTTP_STRING2, ASSET_STRING, HTTP_STRING3);
+        mImageLoader.loadDrawable(urls, mConsumer);
+        List<String> expected_urls = Arrays.asList(HTTP_STRING1, HTTP_STRING2, HTTP_STRING3);
+
+        verify(mBridge, times(1)).fetchImage(eq(expected_urls), mCallbackArgument.capture());
+    }
+
+    @Test
+    @SmallTest
+    public void assetImageTest() {
+        List<String> urls = Arrays.asList(ASSET_STRING);
+        mImageLoader.loadDrawable(urls, mConsumer);
+
+        verify(mConsumer, times(1)).accept(AdditionalMatchers.not(eq(null)));
+    }
+
+    @Test
+    @SmallTest
+    public void sendNullIfDownloadFailTest() {
+        List<String> urls = Arrays.asList(HTTP_STRING1, HTTP_STRING2, HTTP_STRING3);
+        mImageLoader.loadDrawable(urls, mConsumer);
+
+        verify(mConsumer, times(1)).accept(eq(null));
+    }
+
+    @Test
+    @SmallTest
+    public void nullUrlListTest() {
+        List<String> urls = Arrays.asList();
+        mImageLoader.loadDrawable(urls, mConsumer);
+
+        verify(mConsumer, times(1)).accept(eq(null));
+    }
+}

chrome/android/junit/src/org/chromium/chrome/browser/feed/OWNERS

+file://components/feed/OWNERS
+

chrome/browser/BUILD.gn

@@ -4201,6 +4201,8 @@ jumbo_split_static_library("browser") {
     sources += [
       "android/feed/feed_host_service_factory.cc",
       "android/feed/feed_host_service_factory.h",
+      "android/feed/feed_image_loader_bridge.cc",
+      "android/feed/feed_image_loader_bridge.h",
       "android/feed/feed_network_bridge.cc",
       "android/feed/feed_network_bridge.h",
     ]
@@ -4458,7 +4460,10 @@ if (is_android) {
     }
 
     if (enable_feed_in_chrome) {
-      sources += [ "../android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedNetworkBridge.java" ]
+      sources += [
+        "../android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedImageLoaderBridge.java",
+        "../android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedNetworkBridge.java",
+      ]
     }
 
     jni_package = "chrome"

chrome/browser/android/feed/feed_image_loader_bridge.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/android/feed/feed_image_loader_bridge.h"
+
+#include <jni.h>
+
+#include <string>
+#include <vector>
+
+#include "base/android/callback_android.h"
+#include "base/android/jni_array.h"
+#include "base/bind.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "chrome/browser/android/feed/feed_host_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_android.h"
+#include "components/feed/core/feed_host_service.h"
+#include "components/feed/core/feed_image_manager.h"
+#include "jni/FeedImageLoaderBridge_jni.h"
+#include "ui/gfx/android/java_bitmap.h"
+#include "ui/gfx/image/image.h"
+
+using base::android::JavaIntArrayToIntVector;
+using base::android::JavaParamRef;
+using base::android::ScopedJavaGlobalRef;
+using base::android::ScopedJavaLocalRef;
+
+namespace feed {
+
+static jlong JNI_FeedImageLoaderBridge_Init(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& j_this,
+    const JavaParamRef<jobject>& j_profile) {
+  Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
+  FeedHostService* host_service =
+      FeedHostServiceFactory::GetForBrowserContext(profile);
+  FeedImageManager* feed_image_manager = host_service->GetImageManager();
+  DCHECK(feed_image_manager);
+  FeedImageLoaderBridge* native_image_loader_bridge =
+      new FeedImageLoaderBridge(feed_image_manager);
+  return reinterpret_cast<intptr_t>(native_image_loader_bridge);
+}
+
+FeedImageLoaderBridge::FeedImageLoaderBridge(
+    FeedImageManager* feed_image_manager)
+    : feed_image_manager_(feed_image_manager), weak_ptr_factory_(this) {}
+
+FeedImageLoaderBridge::~FeedImageLoaderBridge() = default;
+
+void FeedImageLoaderBridge::Destroy(JNIEnv* env,
+                                    const JavaParamRef<jobject>& j_this) {
+  delete this;
+}
+
+void FeedImageLoaderBridge::FetchImage(
+    JNIEnv* j_env,
+    const JavaParamRef<jobject>& j_this,
+    const JavaParamRef<jobjectArray>& j_urls,
+    const JavaParamRef<jobject>& j_callback) {
+  std::vector<std::string> urls;
+  base::android::AppendJavaStringArrayToStringVector(j_env, j_urls, &urls);
+
+  ScopedJavaGlobalRef<jobject> callback(j_callback);
+  feed_image_manager_->FetchImage(
+      urls, base::BindOnce(&FeedImageLoaderBridge::OnImageFetched,
+                           weak_ptr_factory_.GetWeakPtr(), callback));
+}
+
+void FeedImageLoaderBridge::OnImageFetched(
+    ScopedJavaGlobalRef<jobject> callback,
+    const gfx::Image& image) {
+  ScopedJavaLocalRef<jobject> j_bitmap;
+  if (!image.IsEmpty()) {
+    j_bitmap = gfx::ConvertToJavaBitmap(image.ToSkBitmap());
+  }
+  RunCallbackAndroid(callback, j_bitmap);
+}
+
+}  // namespace feed

chrome/browser/android/feed/feed_image_loader_bridge.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_ANDROID_FEED_FEED_IMAGE_LOADER_BRIDGE_H_
+#define CHROME_BROWSER_ANDROID_FEED_FEED_IMAGE_LOADER_BRIDGE_H_
+
+#include "base/android/scoped_java_ref.h"
+#include "base/memory/weak_ptr.h"
+#include "ui/gfx/image/image.h"
+
+namespace feed {
+
+using base::android::JavaParamRef;
+using base::android::ScopedJavaGlobalRef;
+
+class FeedImageManager;
+
+// Native counterpart of FeedImageLoaderBridge.java. Holds non-owning pointers
+// to native implementation, to which operations are delegated. Results are
+// passed back by a single argument callback so
+// base::android::RunCallbackAndroid() can be used. This bridge is instantiated,
+// owned, and destroyed from Java.
+class FeedImageLoaderBridge {
+ public:
+  explicit FeedImageLoaderBridge(FeedImageManager* feed_image_manager);
+  ~FeedImageLoaderBridge();
+
+  void Destroy(JNIEnv* j_env, const JavaParamRef<jobject>& j_this);
+
+  void FetchImage(JNIEnv* j_env,
+                  const JavaParamRef<jobject>& j_this,
+                  const JavaParamRef<jobjectArray>& j_urls,
+                  const JavaParamRef<jobject>& j_callback);
+
+ private:
+  void OnImageFetched(ScopedJavaGlobalRef<jobject> callback,
+                      const gfx::Image& image);
+
+  FeedImageManager* feed_image_manager_;
+
+  base::WeakPtrFactory<FeedImageLoaderBridge> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(FeedImageLoaderBridge);
+};
+
+}  // namespace feed
+
+#endif  // CHROME_BROWSER_ANDROID_FEED_FEED_IMAGE_LOADER_BRIDGE_H_

components/feed/core/feed_image_manager.cc

@@ -4,6 +4,11 @@
 
 #include "components/feed/core/feed_image_manager.h"
 
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
 #include "base/bind.h"
 #include "components/feed/core/time_serialization.h"
 #include "components/image_fetcher/core/image_decoder.h"
@@ -60,7 +65,6 @@ FeedImageManager::~FeedImageManager() {
 
 void FeedImageManager::FetchImage(std::vector<std::string> urls,
                                   ImageFetchedCallback callback) {
-  DCHECK(urls.size() > 0);
   DCHECK(image_database_.get());
 
   FetchImagesFromDatabase(0, std::move(urls), std::move(callback));

components/feed/core/feed_image_manager_unittest.cc

@@ -4,6 +4,11 @@
 
 #include "components/feed/core/feed_image_manager.h"
 
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
 #include "base/bind.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/test/mock_callback.h"
@@ -23,10 +28,10 @@ using testing::_;
 namespace feed {
 
 namespace {
-const std::string kImageURL = "http://pie.com/";
-const std::string kImageURL2 = "http://cake.com/";
-const std::string kImageData = "pie image";
-const std::string kImageData2 = "cake image";
+const char kImageURL[] = "http://pie.com/";
+const char kImageURL2[] = "http://cake.com/";
+const char kImageData[] = "pie image";
+const char kImageData2[] = "cake image";
 
 class FakeImageDecoder : public image_fetcher::ImageDecoder {
  public:
@@ -124,6 +129,18 @@ class FeedImageManagerTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(FeedImageManagerTest);
 };
 
+TEST_F(FeedImageManagerTest, FetchEmptyUrlVector) {
+  base::MockCallback<ImageFetchedCallback> image_callback;
+
+  // Make sure an empty image passed to callback.
+  EXPECT_CALL(image_callback,
+              Run(testing::Property(&gfx::Image::IsEmpty, testing::Eq(true))));
+  feed_image_manager()->FetchImage(std::vector<std::string>(),
+                                   image_callback.Get());
+
+  RunUntilIdle();
+}
+
 TEST_F(FeedImageManagerTest, FetchImageFromCache) {
   // Save the image in the database.
   image_database()->SaveImage(kImageURL, kImageData);