Remove singleton pattern from ImageFectherBridge.

This CL changes ImageFetcherBridge behaviour for different profiles.
Today, ImageFectherBridge is singleton, so there is only one instance
created for one regular and multiple incognito profiles on Android. To
avoid data leak between profiles, we need to create different instances
for each profile.

By this CL;
- Each call to |ImageFectherBridge#getForProfile| function creates an
instance of ImageFectherBridge that stores profile as member variable.
- Each call to any other function of ImageFectherBridge will directly
hit the native services through native bridge.
- Native image_fetcher_bridge is a wrapper to reach the service that
belongs to the given profile. There is no member functions anymore, but
all are static.

Bug: 1075562, 1041781, 1083923

Change-Id: Ic96d1c8fa1a7bd093abcf8939375f8934fd3cd92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2231124
Commit-Queue: Side YILMAZ <sideyilmaz@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779817}

chrome/android/junit/src/org/chromium/chrome/browser/image_fetcher/ImageFetcherBridgeTest.java

@@ -8,7 +8,6 @@ import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
-import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.verify;
 
 import android.graphics.Bitmap;
@@ -37,7 +36,6 @@ import jp.tomorrowkey.android.gifplayer.BaseGifImage;
 @RunWith(BaseRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
 public class ImageFetcherBridgeTest {
-    private static long sNativePointer = 100L;
     private static final int WIDTH_PX = 10;
     private static final int HEIGHT_PX = 20;
     private static final int EXPIRATION_INTERVAL_MINS = 60;
@@ -59,37 +57,11 @@ public class ImageFetcherBridgeTest {
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
-        doReturn(sNativePointer).when(mNatives).init(mProfile);
 
         ImageFetcherBridgeJni.TEST_HOOKS.setInstanceForTesting(mNatives);
         mBridge = new ImageFetcherBridge(mProfile);
     }
 
-    @Test
-    public void testDestroy() {
-        mBridge.destroy();
-        verify(mNatives).destroy(sNativePointer, mBridge);
-
-        // Check that calling methods after destroy throw AssertionErrors.
-        mExpectedException.expect(AssertionError.class);
-        mExpectedException.expectMessage("destroy called twice");
-        mBridge.destroy();
-        mExpectedException.expectMessage("getFilePath called after destroy");
-        mBridge.getFilePath("");
-        mExpectedException.expectMessage("fetchGif called after destroy");
-        mBridge.fetchGif(-1, ImageFetcher.Params.create("", ""), null);
-        mExpectedException.expectMessage("fetchImage called after destroy");
-        mBridge.fetchImage(-1, ImageFetcher.Params.create("", "", 100, 100), null);
-        mExpectedException.expectMessage("fetchImage called after destroy");
-        mBridge.fetchImage(-1, ImageFetcher.Params.create("", "", 100, 100), null);
-        mExpectedException.expectMessage("reportEvent called after destroy");
-        mBridge.reportEvent("", -1);
-        mExpectedException.expectMessage("reportCacheHitTime called after destroy");
-        mBridge.reportCacheHitTime("", -1L);
-        mExpectedException.expectMessage("reportTotalFetchTimeFromNative called after destroy");
-        mBridge.reportTotalFetchTimeFromNative("", -1L);
-    }
-
     @Test
     public void testFetchImage() {
         ArgumentCaptor<Callback<Bitmap>> callbackCaptor = ArgumentCaptor.forClass(Callback.class);
@@ -99,8 +71,8 @@ public class ImageFetcherBridgeTest {
             return null;
         })
                 .when(mNatives)
-                .fetchImage(eq(sNativePointer), eq(mBridge), anyInt(), anyString(), anyString(),
-                        eq(0), callbackCaptor.capture());
+                .fetchImage(eq(mProfile), anyInt(), anyString(), anyString(), eq(0),
+                        callbackCaptor.capture());
 
         mBridge.fetchImage(
                 -1, ImageFetcher.Params.create("", "", WIDTH_PX, HEIGHT_PX), mBitmapCallback);
@@ -116,7 +88,7 @@ public class ImageFetcherBridgeTest {
             return null;
         })
                 .when(mNatives)
-                .fetchImage(eq(sNativePointer), eq(mBridge), anyInt(), anyString(), anyString(),
+                .fetchImage(eq(mProfile), anyInt(), anyString(), anyString(),
                         eq(EXPIRATION_INTERVAL_MINS), callbackCaptor.capture());
 
         mBridge.fetchImage(-1,
@@ -135,8 +107,8 @@ public class ImageFetcherBridgeTest {
             return null;
         })
                 .when(mNatives)
-                .fetchImage(eq(sNativePointer), eq(mBridge), anyInt(), anyString(), anyString(),
-                        eq(0), callbackCaptor.capture());
+                .fetchImage(eq(mProfile), anyInt(), anyString(), anyString(), eq(0),
+                        callbackCaptor.capture());
 
         mBridge.fetchImage(-1, ImageFetcher.Params.create("", "", 100, 100), mBitmapCallback);
         ArgumentCaptor<Bitmap> bitmapCaptor = ArgumentCaptor.forClass(Bitmap.class);
@@ -157,8 +129,8 @@ public class ImageFetcherBridgeTest {
             return null;
         })
                 .when(mNatives)
-                .fetchImageData(eq(sNativePointer), eq(mBridge), anyInt(), anyString(), anyString(),
-                        eq(0), callbackCaptor.capture());
+                .fetchImageData(eq(mProfile), anyInt(), anyString(), anyString(), eq(0),
+                        callbackCaptor.capture());
 
         mBridge.fetchGif(-1, ImageFetcher.Params.create("", ""), mGifCallback);
         ArgumentCaptor<BaseGifImage> gifCaptor = ArgumentCaptor.forClass(BaseGifImage.class);
@@ -175,8 +147,8 @@ public class ImageFetcherBridgeTest {
             return null;
         })
                 .when(mNatives)
-                .fetchImageData(eq(sNativePointer), eq(mBridge), anyInt(), anyString(), anyString(),
-                        eq(0), callbackCaptor.capture());
+                .fetchImageData(eq(mProfile), anyInt(), anyString(), anyString(), eq(0),
+                        callbackCaptor.capture());
 
         mBridge.fetchGif(-1, ImageFetcher.Params.create("", ""), mGifCallback);
         verify(mGifCallback).onResult(null);
@@ -185,30 +157,31 @@ public class ImageFetcherBridgeTest {
     @Test
     public void testGetFilePath() {
         mBridge.getFilePath("testing is cool");
-        verify(mNatives).getFilePath(sNativePointer, mBridge, "testing is cool");
+        verify(mNatives).getFilePath(mProfile, "testing is cool");
     }
 
     @Test
     public void testReportEvent() {
         mBridge.reportEvent("client", 10);
-        verify(mNatives).reportEvent(sNativePointer, mBridge, "client", 10);
+        verify(mNatives).reportEvent("client", 10);
     }
 
     @Test
     public void testReportCacheHitTime() {
         mBridge.reportCacheHitTime("client", 10L);
-        verify(mNatives).reportCacheHitTime(sNativePointer, mBridge, "client", 10L);
+        verify(mNatives).reportCacheHitTime("client", 10L);
     }
 
     @Test
     public void testReportTotalFetchTimeFromNative() {
         mBridge.reportTotalFetchTimeFromNative("client", 10L);
-        verify(mNatives).reportTotalFetchTimeFromNative(sNativePointer, mBridge, "client", 10L);
+        verify(mNatives).reportTotalFetchTimeFromNative("client", 10L);
     }
 
     @Test
     public void testSetupForTesting() {
-        ImageFetcherBridge.setupForTesting(mBridge);
-        Assert.assertEquals(mBridge, ImageFetcherBridge.getInstance());
+        // Since ImageFetcherBridge creates different instance on each call of getForProfile
+        // function, two instances below should not be equal.
+        Assert.assertNotEquals(mBridge, ImageFetcherBridge.getForProfile(mProfile));
     }
 }
\ No newline at end of file

chrome/android/junit/src/org/chromium/chrome/browser/image_fetcher/ImageFetcherFactoryTest.java

@@ -5,6 +5,8 @@
 package org.chromium.chrome.browser.image_fetcher;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
 
 import android.support.test.filters.SmallTest;
 
@@ -17,6 +19,7 @@ import org.robolectric.annotation.Config;
 
 import org.chromium.base.DiscardableReferencePool;
 import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.chrome.browser.profiles.Profile;
 
 /**
  * Test for ImageFetcherFactory.
@@ -28,6 +31,8 @@ public class ImageFetcherFactoryTest {
     ImageFetcherBridge mImageFetcherBridge;
     @Mock
     DiscardableReferencePool mReferencePool;
+    @Mock
+    Profile mProfile;
 
     @Before
     public void setUp() {
@@ -59,4 +64,25 @@ public class ImageFetcherFactoryTest {
                                 InMemoryCachedImageFetcher.DEFAULT_CACHE_SIZE)
                         .getConfig());
     }
+
+    @Test
+    @SmallTest
+    public void testCreateImageFetcher() {
+        int config = ImageFetcherConfig.NETWORK_ONLY;
+
+        ImageFetcher imageFetcher = ImageFetcherFactory.createImageFetcher(config, mProfile);
+        assertNotNull(imageFetcher);
+        assertNotEquals(mImageFetcherBridge, imageFetcher.getImageFetcherBridge());
+
+        ImageFetcher imageFetcherWithRefPool =
+                ImageFetcherFactory.createImageFetcher(config, mProfile, mReferencePool);
+        assertNotNull(imageFetcherWithRefPool);
+        assertNotEquals(mImageFetcherBridge, imageFetcherWithRefPool.getImageFetcherBridge());
+
+        ImageFetcher imageFetcherWithRefPoolAndCacheSize = ImageFetcherFactory.createImageFetcher(
+                config, mProfile, mReferencePool, InMemoryCachedImageFetcher.DEFAULT_CACHE_SIZE);
+        assertNotNull(imageFetcherWithRefPoolAndCacheSize);
+        assertNotEquals(
+                mImageFetcherBridge, imageFetcherWithRefPoolAndCacheSize.getImageFetcherBridge());
+    }
 }

chrome/browser/android/image_fetcher/image_fetcher_bridge.cc

@@ -61,53 +61,30 @@ constexpr net::NetworkTrafficAnnotationTag kTrafficAnnotation =
 
 }  // namespace
 
-// static
-jlong JNI_ImageFetcherBridge_Init(JNIEnv* j_env,
-                                  const JavaParamRef<jobject>& j_profile) {
-  Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
-  SimpleFactoryKey* simple_factory_key = profile->GetProfileKey();
-  base::FilePath file_path =
-      ImageFetcherServiceFactory::GetCachePath(simple_factory_key)
-          .Append(kPathPostfix);
-
-  ImageFetcherService* if_service =
-      ImageFetcherServiceFactory::GetForKey(simple_factory_key);
-
-  ImageFetcherBridge* native_if_bridge =
-      new ImageFetcherBridge(if_service, file_path);
-  return reinterpret_cast<intptr_t>(native_if_bridge);
-}
-
-ImageFetcherBridge::ImageFetcherBridge(
-    ImageFetcherService* image_fetcher_service,
-    base::FilePath base_file_path)
-    : image_fetcher_service_(image_fetcher_service),
-      base_file_path_(base_file_path) {}
-
 ImageFetcherBridge::~ImageFetcherBridge() = default;
 
-void ImageFetcherBridge::Destroy(JNIEnv* j_env,
-                                 const JavaRef<jobject>& j_this) {
-  delete this;
-}
-
+// static
 ScopedJavaLocalRef<jstring> ImageFetcherBridge::GetFilePath(
     JNIEnv* j_env,
-    const JavaRef<jobject>& j_this,
-    const JavaRef<jstring>& j_url) {
+    const JavaParamRef<jobject>& j_profile,
+    const JavaParamRef<jstring>& j_url) {
   std::string url = base::android::ConvertJavaStringToUTF8(j_url);
+  base::FilePath base_file_path =
+      ImageFetcherBridge::GetFilePathForProfile(j_profile);
   std::string file_path =
-      base_file_path_.Append(ImageCache::HashUrlToKey(url)).MaybeAsASCII();
+      base_file_path.Append(ImageCache::HashUrlToKey(url)).MaybeAsASCII();
   return base::android::ConvertUTF8ToJavaString(j_env, file_path);
 }
 
-void ImageFetcherBridge::FetchImageData(JNIEnv* j_env,
-                                        const JavaRef<jobject>& j_this,
+// static
+void ImageFetcherBridge::FetchImageData(
+    JNIEnv* j_env,
+    const JavaParamRef<jobject>& j_profile,
     const jint j_image_fetcher_config,
-                                        const JavaRef<jstring>& j_url,
-                                        const JavaRef<jstring>& j_client_name,
+    const JavaParamRef<jstring>& j_url,
+    const JavaParamRef<jstring>& j_client_name,
     const jint j_expiration_interval_mins,
-                                        const JavaRef<jobject>& j_callback) {
+    const JavaParamRef<jobject>& j_callback) {
   ScopedJavaGlobalRef<jobject> callback(j_callback);
   ImageFetcherConfig config =
       static_cast<ImageFetcherConfig>(j_image_fetcher_config);
@@ -124,20 +101,22 @@ void ImageFetcherBridge::FetchImageData(JNIEnv* j_env,
   // We can skip transcoding here because this method is used in java as
   // ImageFetcher.fetchGif, which decodes the data in a Java-only library.
   params.set_skip_transcoding(true);
-  image_fetcher_service_->GetImageFetcher(config)->FetchImageData(
+  ImageFetcherService* image_fetcher_service =
+      ImageFetcherBridge::GetImageFetcherServiceForProfile(j_profile);
+  image_fetcher_service->GetImageFetcher(config)->FetchImageData(
       GURL(url),
-      base::BindOnce(&ImageFetcherBridge::OnImageDataFetched,
-                     weak_ptr_factory_.GetWeakPtr(), callback),
+      base::BindOnce(&ImageFetcherBridge::OnImageDataFetched, callback),
       std::move(params));
 }
 
+// static
 void ImageFetcherBridge::FetchImage(JNIEnv* j_env,
-                                    const JavaRef<jobject>& j_this,
+                                    const JavaParamRef<jobject>& j_profile,
                                     const jint j_image_fetcher_config,
-                                    const JavaRef<jstring>& j_url,
-                                    const JavaRef<jstring>& j_client_name,
+                                    const JavaParamRef<jstring>& j_url,
+                                    const JavaParamRef<jstring>& j_client_name,
                                     const jint j_expiration_interval_mins,
-                                    const JavaRef<jobject>& j_callback) {
+                                    const JavaParamRef<jobject>& j_callback) {
   ScopedJavaGlobalRef<jobject> callback(j_callback);
   ImageFetcherConfig config =
       static_cast<ImageFetcherConfig>(j_image_fetcher_config);
@@ -150,17 +129,17 @@ void ImageFetcherBridge::FetchImage(JNIEnv* j_env,
     params.set_hold_for_expiration_interval(
         base::TimeDelta::FromMinutes(j_expiration_interval_mins));
   }
-  image_fetcher_service_->GetImageFetcher(config)->FetchImage(
-      GURL(url),
-      base::BindOnce(&ImageFetcherBridge::OnImageFetched,
-                     weak_ptr_factory_.GetWeakPtr(), callback),
+  ImageFetcherService* image_fetcher_service =
+      ImageFetcherBridge::GetImageFetcherServiceForProfile(j_profile);
+  image_fetcher_service->GetImageFetcher(config)->FetchImage(
+      GURL(url), base::BindOnce(&ImageFetcherBridge::OnImageFetched, callback),
       std::move(params));
 }
 
+// static
 void ImageFetcherBridge::ReportEvent(
     JNIEnv* j_env,
-    const base::android::JavaRef<jobject>& j_this,
-    const base::android::JavaRef<jstring>& j_client_name,
+    const base::android::JavaParamRef<jstring>& j_client_name,
     const jint j_event_id) {
   std::string client_name =
       base::android::ConvertJavaStringToUTF8(j_client_name);
@@ -168,10 +147,10 @@ void ImageFetcherBridge::ReportEvent(
   ImageFetcherMetricsReporter::ReportEvent(client_name, event);
 }
 
+// static
 void ImageFetcherBridge::ReportCacheHitTime(
     JNIEnv* j_env,
-    const base::android::JavaRef<jobject>& j_this,
-    const base::android::JavaRef<jstring>& j_client_name,
+    const base::android::JavaParamRef<jstring>& j_client_name,
     const jlong start_time_millis) {
   std::string client_name =
       base::android::ConvertJavaStringToUTF8(j_client_name);
@@ -180,10 +159,10 @@ void ImageFetcherBridge::ReportCacheHitTime(
                                                                 start_time);
 }
 
+// static
 void ImageFetcherBridge::ReportTotalFetchTimeFromNative(
     JNIEnv* j_env,
-    const base::android::JavaRef<jobject>& j_this,
-    const base::android::JavaRef<jstring>& j_client_name,
+    const base::android::JavaParamRef<jstring>& j_client_name,
     const jlong start_time_millis) {
   std::string client_name =
       base::android::ConvertJavaStringToUTF8(j_client_name);
@@ -192,6 +171,88 @@ void ImageFetcherBridge::ReportTotalFetchTimeFromNative(
                                                                   start_time);
 }
 
+// ------------------ JNI functions ------------------
+// static
+ScopedJavaLocalRef<jstring> JNI_ImageFetcherBridge_GetFilePath(
+    JNIEnv* j_env,
+    const JavaParamRef<jobject>& j_profile,
+    const JavaParamRef<jstring>& j_url) {
+  return ImageFetcherBridge::GetFilePath(j_env, j_profile, j_url);
+}
+
+// static
+void JNI_ImageFetcherBridge_FetchImageData(
+    JNIEnv* j_env,
+    const JavaParamRef<jobject>& j_profile,
+    const jint j_image_fetcher_config,
+    const JavaParamRef<jstring>& j_url,
+    const JavaParamRef<jstring>& j_client_name,
+    const jint j_expiration_interval_mins,
+    const JavaParamRef<jobject>& j_callback) {
+  ImageFetcherBridge::FetchImageData(j_env, j_profile, j_image_fetcher_config,
+                                     j_url, j_client_name,
+                                     j_expiration_interval_mins, j_callback);
+}
+
+// static
+void JNI_ImageFetcherBridge_FetchImage(
+    JNIEnv* j_env,
+    const JavaParamRef<jobject>& j_profile,
+    const jint j_image_fetcher_config,
+    const JavaParamRef<jstring>& j_url,
+    const JavaParamRef<jstring>& j_client_name,
+    const jint j_expiration_interval_mins,
+    const JavaParamRef<jobject>& j_callback) {
+  ImageFetcherBridge::FetchImage(j_env, j_profile, j_image_fetcher_config,
+                                 j_url, j_client_name,
+                                 j_expiration_interval_mins, j_callback);
+}
+
+// static
+void JNI_ImageFetcherBridge_ReportEvent(
+    JNIEnv* j_env,
+    const base::android::JavaParamRef<jstring>& j_client_name,
+    const jint j_event_id) {
+  ImageFetcherBridge::ReportEvent(j_env, j_client_name, j_event_id);
+}
+
+// static
+void JNI_ImageFetcherBridge_ReportCacheHitTime(
+    JNIEnv* j_env,
+    const base::android::JavaParamRef<jstring>& j_client_name,
+    const jlong start_time_millis) {
+  ImageFetcherBridge::ReportCacheHitTime(j_env, j_client_name,
+                                         start_time_millis);
+}
+
+// static
+void JNI_ImageFetcherBridge_ReportTotalFetchTimeFromNative(
+    JNIEnv* j_env,
+    const base::android::JavaParamRef<jstring>& j_client_name,
+    const jlong start_time_millis) {
+  ImageFetcherBridge::ReportTotalFetchTimeFromNative(j_env, j_client_name,
+                                                     start_time_millis);
+}
+
+// ------------------ Private functions ------------------
+// static
+base::FilePath ImageFetcherBridge::GetFilePathForProfile(
+    const JavaParamRef<jobject>& j_profile) {
+  Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
+  SimpleFactoryKey* simple_factory_key = profile->GetProfileKey();
+  return ImageFetcherServiceFactory::GetCachePath(simple_factory_key)
+      .Append(kPathPostfix);
+}
+
+// static
+ImageFetcherService* ImageFetcherBridge::GetImageFetcherServiceForProfile(
+    const JavaParamRef<jobject>& j_profile) {
+  Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
+  SimpleFactoryKey* simple_factory_key = profile->GetProfileKey();
+  return ImageFetcherServiceFactory::GetForKey(simple_factory_key);
+}
+
+// static
 void ImageFetcherBridge::OnImageDataFetched(
     base::android::ScopedJavaGlobalRef<jobject> callback,
     const std::string& image_data,
@@ -203,6 +264,7 @@ void ImageFetcherBridge::OnImageDataFetched(
   RunObjectCallbackAndroid(callback, j_bytes);
 }
 
+// static
 void ImageFetcherBridge::OnImageFetched(
     base::android::ScopedJavaGlobalRef<jobject> callback,
     const gfx::Image& image,

chrome/browser/android/image_fetcher/image_fetcher_bridge.h

@@ -21,63 +21,63 @@ class ImageFetcherService;
 // Native counterpart of ImageFetcherBridge.java.
 class ImageFetcherBridge {
  public:
-  ImageFetcherBridge(ImageFetcherService* image_fetcher_service,
-                     base::FilePath base_file_path);
+  ImageFetcherBridge();
   ~ImageFetcherBridge();
 
-  void Destroy(JNIEnv* j_env, const base::android::JavaRef<jobject>& j_this);
-
-  base::android::ScopedJavaLocalRef<jstring> GetFilePath(
+  static base::android::ScopedJavaLocalRef<jstring> GetFilePath(
       JNIEnv* j_env,
-      const base::android::JavaRef<jobject>& j_this,
-      const base::android::JavaRef<jstring>& j_url);
+      const base::android::JavaParamRef<jobject>& j_profile,
+      const base::android::JavaParamRef<jstring>& j_url);
 
-  void FetchImageData(JNIEnv* j_env,
-                      const base::android::JavaRef<jobject>& j_this,
+  static void FetchImageData(
+      JNIEnv* j_env,
+      const base::android::JavaParamRef<jobject>& j_profile,
       const jint j_image_fetcher_config,
-                      const base::android::JavaRef<jstring>& j_url,
-                      const base::android::JavaRef<jstring>& j_client_name,
+      const base::android::JavaParamRef<jstring>& j_url,
+      const base::android::JavaParamRef<jstring>& j_client_name,
       const jint j_expiration_interval_mins,
-                      const base::android::JavaRef<jobject>& j_callback);
+      const base::android::JavaParamRef<jobject>& j_callback);
 
-  void FetchImage(JNIEnv* j_env,
-                  const base::android::JavaRef<jobject>& j_this,
+  static void FetchImage(
+      JNIEnv* j_env,
+      const base::android::JavaParamRef<jobject>& j_profile,
       const jint j_image_fetcher_config,
-                  const base::android::JavaRef<jstring>& j_url,
-                  const base::android::JavaRef<jstring>& j_client_name,
+      const base::android::JavaParamRef<jstring>& j_url,
+      const base::android::JavaParamRef<jstring>& j_client_name,
       const jint j_expiration_interval_mins,
-                  const base::android::JavaRef<jobject>& j_callback);
+      const base::android::JavaParamRef<jobject>& j_callback);
 
-  void ReportEvent(JNIEnv* j_env,
-                   const base::android::JavaRef<jobject>& j_this,
-                   const base::android::JavaRef<jstring>& j_client_name,
+  static void ReportEvent(
+      JNIEnv* j_env,
+      const base::android::JavaParamRef<jstring>& j_client_name,
       const jint j_event_id);
 
-  void ReportCacheHitTime(JNIEnv* j_env,
-                          const base::android::JavaRef<jobject>& j_this,
-                          const base::android::JavaRef<jstring>& j_client_name,
+  static void ReportCacheHitTime(
+      JNIEnv* j_env,
+      const base::android::JavaParamRef<jstring>& j_client_name,
       const jlong start_time_millis);
 
-  void ReportTotalFetchTimeFromNative(
+  static void ReportTotalFetchTimeFromNative(
       JNIEnv* j_env,
-      const base::android::JavaRef<jobject>& j_this,
-      const base::android::JavaRef<jstring>& j_client_name,
+      const base::android::JavaParamRef<jstring>& j_client_name,
       const jlong start_time_millis);
 
  private:
-  void OnImageDataFetched(base::android::ScopedJavaGlobalRef<jobject> callback,
+  static void OnImageDataFetched(
+      base::android::ScopedJavaGlobalRef<jobject> callback,
       const std::string& image_data,
       const RequestMetadata& request_metadata);
 
-  void OnImageFetched(base::android::ScopedJavaGlobalRef<jobject> callback,
+  static void OnImageFetched(
+      base::android::ScopedJavaGlobalRef<jobject> callback,
       const gfx::Image& image,
       const RequestMetadata& request_metadata);
 
-  // This service outlives the bridge.
-  ImageFetcherService* image_fetcher_service_;
-  base::FilePath base_file_path_;
+  static ImageFetcherService* GetImageFetcherServiceForProfile(
+      const base::android::JavaParamRef<jobject>& j_profile);
 
-  base::WeakPtrFactory<ImageFetcherBridge> weak_ptr_factory_{this};
+  static base::FilePath GetFilePathForProfile(
+      const base::android::JavaParamRef<jobject>& j_profile);
 
   DISALLOW_COPY_AND_ASSIGN(ImageFetcherBridge);
 };

chrome/browser/image_fetcher/android/java/src/org/chromium/chrome/browser/image_fetcher/ImageFetcherBridge.java

@@ -9,6 +9,7 @@ import android.graphics.Bitmap;
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.Callback;
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.chrome.browser.profiles.Profile;
@@ -20,36 +21,28 @@ import jp.tomorrowkey.android.gifplayer.BaseGifImage;
  */
 @JNINamespace("image_fetcher")
 public class ImageFetcherBridge {
-    private static ImageFetcherBridge sImageFetcherBridge;
-
-    private long mNativeImageFetcherBridge;
-
-    // Get the instance of the ImageFetcherBridge. If used before browser initialization, this will
-    // throw an IllegalStateException.
-    public static ImageFetcherBridge getInstance() {
-        if (sImageFetcherBridge == null) {
-            // TODO(https://crbug.com/1041781): Use the current profile (i.e., regular profile or
-            // incognito profile) instead of always using regular profile.
-            Profile profile = Profile.getLastUsedRegularProfile();
-            sImageFetcherBridge = new ImageFetcherBridge(profile);
-        }
+    private Profile mProfile;
 
-        return sImageFetcherBridge;
+    /**
+     * Get the ImageFetcherBridge for the given profile.
+     *
+     * @param profile   The profile for which the ImageFetcherBridge is returned.
+     * @return          The ImageFetcherBridge for the given profile.
+     */
+    public static ImageFetcherBridge getForProfile(Profile profile) {
+        ThreadUtils.assertOnUiThread();
+
+        return new ImageFetcherBridge(profile);
     }
 
     /**
-     * Creates a ImageFetcherBridge for accessing the native ImageFetcher implementation.
+     * Creates a ImageFetcherBridge for the given profile.
+     *
+     * @param profile The profile to reach regarding image_fetcher_service on native side.
      */
     @VisibleForTesting
     ImageFetcherBridge(Profile profile) {
-        mNativeImageFetcherBridge = ImageFetcherBridgeJni.get().init(profile);
-    }
-
-    /** Cleans up native half of bridge. */
-    public void destroy() {
-        assert mNativeImageFetcherBridge != 0 : "destroy called twice";
-        ImageFetcherBridgeJni.get().destroy(mNativeImageFetcherBridge, ImageFetcherBridge.this);
-        mNativeImageFetcherBridge = 0;
+        mProfile = profile;
     }
 
     /**
@@ -59,23 +52,20 @@ public class ImageFetcherBridge {
      * @return The full path to the resource on disk.
      */
     public String getFilePath(String url) {
-        assert mNativeImageFetcherBridge != 0 : "getFilePath called after destroy";
-        return ImageFetcherBridgeJni.get().getFilePath(
-                mNativeImageFetcherBridge, ImageFetcherBridge.this, url);
+        return ImageFetcherBridgeJni.get().getFilePath(mProfile, url);
     }
 
     /**
      * Fetch a gif from native or null if the gif can't be fetched or decoded.
      *
+     * @param config The configuration of the image fetcher.
      * @param params The parameters to specify image fetching details.
      * @param callback The callback to call when the gif is ready. The callback will be invoked on
      *      the same thread it was called on.
      */
     public void fetchGif(@ImageFetcherConfig int config, final ImageFetcher.Params params,
             Callback<BaseGifImage> callback) {
-        assert mNativeImageFetcherBridge != 0 : "fetchGif called after destroy";
-        ImageFetcherBridgeJni.get().fetchImageData(mNativeImageFetcherBridge,
-                ImageFetcherBridge.this, config, params.url, params.clientName,
+        ImageFetcherBridgeJni.get().fetchImageData(mProfile, config, params.url, params.clientName,
                 params.expirationIntervalMinutes, (byte[] data) -> {
                     if (data == null || data.length == 0) {
                         callback.onResult(null);
@@ -96,10 +86,8 @@ public class ImageFetcherBridge {
      */
     public void fetchImage(@ImageFetcherConfig int config, final ImageFetcher.Params params,
             Callback<Bitmap> callback) {
-        assert mNativeImageFetcherBridge != 0 : "fetchImage called after destroy";
-        ImageFetcherBridgeJni.get().fetchImage(mNativeImageFetcherBridge, ImageFetcherBridge.this,
-                config, params.url, params.clientName, params.expirationIntervalMinutes,
-                (bitmap) -> {
+        ImageFetcherBridgeJni.get().fetchImage(mProfile, config, params.url, params.clientName,
+                params.expirationIntervalMinutes, (bitmap) -> {
                     callback.onResult(
                             ImageFetcher.resizeImage(bitmap, params.width, params.height));
                 });
@@ -112,9 +100,7 @@ public class ImageFetcherBridge {
      * @param eventId The event to report.
      */
     public void reportEvent(String clientName, @ImageFetcherEvent int eventId) {
-        assert mNativeImageFetcherBridge != 0 : "reportEvent called after destroy";
-        ImageFetcherBridgeJni.get().reportEvent(
-                mNativeImageFetcherBridge, ImageFetcherBridge.this, clientName, eventId);
+        ImageFetcherBridgeJni.get().reportEvent(clientName, eventId);
     }
 
     /**
@@ -125,9 +111,7 @@ public class ImageFetcherBridge {
      *      total duration.
      */
     public void reportCacheHitTime(String clientName, long startTimeMillis) {
-        assert mNativeImageFetcherBridge != 0 : "reportCacheHitTime called after destroy";
-        ImageFetcherBridgeJni.get().reportCacheHitTime(
-                mNativeImageFetcherBridge, ImageFetcherBridge.this, clientName, startTimeMillis);
+        ImageFetcherBridgeJni.get().reportCacheHitTime(clientName, startTimeMillis);
     }
 
     /**
@@ -138,38 +122,19 @@ public class ImageFetcherBridge {
      *      total duration.
      */
     public void reportTotalFetchTimeFromNative(String clientName, long startTimeMillis) {
-        assert mNativeImageFetcherBridge
-                != 0 : "reportTotalFetchTimeFromNative called after destroy";
-        ImageFetcherBridgeJni.get().reportTotalFetchTimeFromNative(
-                mNativeImageFetcherBridge, ImageFetcherBridge.this, clientName, startTimeMillis);
-    }
-
-    /**
-     * Setup the bridge for testing.
-     * @param imageFetcherBridge The bridge used for testing.
-     */
-    public static void setupForTesting(ImageFetcherBridge imageFetcherBridge) {
-        sImageFetcherBridge = imageFetcherBridge;
+        ImageFetcherBridgeJni.get().reportTotalFetchTimeFromNative(clientName, startTimeMillis);
     }
 
     @NativeMethods
     interface Natives {
         // Native methods
-        long init(Profile profile);
-
-        void destroy(long nativeImageFetcherBridge, ImageFetcherBridge caller);
-        String getFilePath(long nativeImageFetcherBridge, ImageFetcherBridge caller, String url);
-        void fetchImageData(long nativeImageFetcherBridge, ImageFetcherBridge caller,
-                @ImageFetcherConfig int config, String url, String clientName,
-                int expirationIntervalMinutes, Callback<byte[]> callback);
-        void fetchImage(long nativeImageFetcherBridge, ImageFetcherBridge caller,
-                @ImageFetcherConfig int config, String url, String clientName,
-                int expirationIntervalMinutes, Callback<Bitmap> callback);
-        void reportEvent(long nativeImageFetcherBridge, ImageFetcherBridge caller,
-                String clientName, int eventId);
-        void reportCacheHitTime(long nativeImageFetcherBridge, ImageFetcherBridge caller,
-                String clientName, long startTimeMillis);
-        void reportTotalFetchTimeFromNative(long nativeImageFetcherBridge,
-                ImageFetcherBridge caller, String clientName, long startTimeMillis);
+        String getFilePath(Profile profile, String url);
+        void fetchImageData(Profile profile, @ImageFetcherConfig int config, String url,
+                String clientName, int expirationIntervalMinutes, Callback<byte[]> callback);
+        void fetchImage(Profile profile, @ImageFetcherConfig int config, String url,
+                String clientName, int expirationIntervalMinutes, Callback<Bitmap> callback);
+        void reportEvent(String clientName, int eventId);
+        void reportCacheHitTime(String clientName, long startTimeMillis);
+        void reportTotalFetchTimeFromNative(String clientName, long startTimeMillis);
     }
 }

chrome/browser/image_fetcher/android/java/src/org/chromium/chrome/browser/image_fetcher/ImageFetcherFactory.java

@@ -5,44 +5,95 @@
 package org.chromium.chrome.browser.image_fetcher;
 
 import org.chromium.base.DiscardableReferencePool;
+import org.chromium.chrome.browser.profiles.Profile;
 
 /**
  * Factory to provide the image fetcher best suited for the given config.
  */
 public class ImageFetcherFactory {
-    // Store static references to singleton image fetchers.
-    private static CachedImageFetcher sCachedImageFetcher;
-    private static NetworkImageFetcher sNetworkImageFetcher;
-
     /**
-     * Alias for createImageFetcher below.
+     * Alias for createImageFetcher below. If used before browser initialization, this will throw an
+     * IllegalStateException. This method always uses regular profile to get ImageFetcherBridge.
+     *
+     * https://crbug.com/1083923: Remove after replacing all use cases.
+     *
+     * @deprecated use {@link ImageFetcherFactory#createImageFetcher(int, Profile)} instead.
      */
+    @Deprecated
     public static ImageFetcher createImageFetcher(@ImageFetcherConfig int config) {
-        return createImageFetcher(config, ImageFetcherBridge.getInstance(), null,
-                InMemoryCachedImageFetcher.DEFAULT_CACHE_SIZE);
+        ImageFetcherBridge bridge =
+                ImageFetcherBridge.getForProfile(Profile.getLastUsedRegularProfile());
+        return createImageFetcher(
+                config, bridge, null, InMemoryCachedImageFetcher.DEFAULT_CACHE_SIZE);
     }
 
     /**
-     * Alias for createImageFetcher below.
+     * Alias for createImageFetcher below. If used before browser initialization, this will throw an
+     * IllegalStateException. This method always uses regular profile to get ImageFetcherBridge.
+     *
+     * https://crbug.com/1083923: Remove after replacing all use cases.
+     *
+     * @deprecated use {@link ImageFetcherFactory#createImageFetcher(int, DiscardableReferencePool,
+     *         Profile)} instead.
      */
+    @Deprecated
     public static ImageFetcher createImageFetcher(
             @ImageFetcherConfig int config, DiscardableReferencePool discardableReferencePool) {
-        return createImageFetcher(config, ImageFetcherBridge.getInstance(),
-                discardableReferencePool, InMemoryCachedImageFetcher.DEFAULT_CACHE_SIZE);
+        ImageFetcherBridge bridge =
+                ImageFetcherBridge.getForProfile(Profile.getLastUsedRegularProfile());
+        return createImageFetcher(config, bridge, discardableReferencePool,
+                InMemoryCachedImageFetcher.DEFAULT_CACHE_SIZE);
     }
 
     /**
-     * Alias for createImageFetcher below.
+     * Alias for createImageFetcher below. If used before browser initialization, this will throw an
+     * IllegalStateException. This method always uses regular profile to get ImageFetcherBridge.
+     *
+     * https://crbug.com/1083923: Remove after replacing all use cases.
+     *
+     * @deprecated use {@link ImageFetcherFactory#createImageFetcher(int, DiscardableReferencePool,
+     *         int, Profile)} instead.
      */
+    @Deprecated
     public static ImageFetcher createImageFetcher(@ImageFetcherConfig int config,
             DiscardableReferencePool discardableReferencePool, int inMemoryCacheSize) {
-        return createImageFetcher(config, ImageFetcherBridge.getInstance(),
-                discardableReferencePool, inMemoryCacheSize);
+        ImageFetcherBridge bridge =
+                ImageFetcherBridge.getForProfile(Profile.getLastUsedRegularProfile());
+        return createImageFetcher(config, bridge, discardableReferencePool, inMemoryCacheSize);
+    }
+
+    /**
+     * Alias for createImageFetcher below.
+     */
+    public static ImageFetcher createImageFetcher(@ImageFetcherConfig int config, Profile profile) {
+        ImageFetcherBridge bridge = ImageFetcherBridge.getForProfile(profile);
+        return createImageFetcher(
+                config, bridge, null, InMemoryCachedImageFetcher.DEFAULT_CACHE_SIZE);
+    }
+
+    /**
+     * Alias for createImageFetcher below.
+     */
+    public static ImageFetcher createImageFetcher(@ImageFetcherConfig int config, Profile profile,
+            DiscardableReferencePool discardableReferencePool) {
+        ImageFetcherBridge bridge = ImageFetcherBridge.getForProfile(profile);
+        return createImageFetcher(config, bridge, discardableReferencePool,
+                InMemoryCachedImageFetcher.DEFAULT_CACHE_SIZE);
+    }
+
+    /**
+     * Alias for createImageFetcher below.
+     */
+    public static ImageFetcher createImageFetcher(@ImageFetcherConfig int config, Profile profile,
+            DiscardableReferencePool discardableReferencePool, int inMemoryCacheSize) {
+        ImageFetcherBridge bridge = ImageFetcherBridge.getForProfile(profile);
+        return createImageFetcher(config, bridge, discardableReferencePool, inMemoryCacheSize);
     }
 
     /**
      * Return the image fetcher that matches the given config. This will return an image fetcher
-     * config that you must destroy.
+     * config that you must destroy. This function is only used for functions above and tests in
+     * this package.
      *
      * @param config The type of ImageFetcher you need.
      * @param imageFetcherBridge Bridge to use.
@@ -50,22 +101,16 @@ public class ImageFetcherFactory {
      * @param inMemoryCacheSize The size of the in memory cache (in bytes).
      * @return The correct ImageFetcher according to the provided config.
      */
-    public static ImageFetcher createImageFetcher(@ImageFetcherConfig int config,
+    static ImageFetcher createImageFetcher(@ImageFetcherConfig int config,
             ImageFetcherBridge imageFetcherBridge,
             DiscardableReferencePool discardableReferencePool, int inMemoryCacheSize) {
         // TODO(crbug.com/947191):Allow server-side configuration image fetcher clients.
         switch (config) {
             case ImageFetcherConfig.NETWORK_ONLY:
-                if (sNetworkImageFetcher == null) {
-                    sNetworkImageFetcher = new NetworkImageFetcher(imageFetcherBridge);
-                }
-                return sNetworkImageFetcher;
+                return new NetworkImageFetcher(imageFetcherBridge);
             case ImageFetcherConfig.DISK_CACHE_ONLY:
-                if (sCachedImageFetcher == null) {
-                    sCachedImageFetcher = new CachedImageFetcher(
+                return new CachedImageFetcher(
                         imageFetcherBridge, new CachedImageFetcher.ImageLoader());
-                }
-                return sCachedImageFetcher;
             case ImageFetcherConfig.IN_MEMORY_ONLY:
                 assert discardableReferencePool != null;
                 return new InMemoryCachedImageFetcher(