[Android] Open up API in jni bridge to fetch images from tab

chrome/android/java/src/org/chromium/chrome/browser/EnhancedBookmarksBridge.java

Index: chrome/android/java/src/org/chromium/chrome/browser/EnhancedBookmarksBridge.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/EnhancedBookmarksBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/EnhancedBookmarksBridge.java
index 3ff5ecc0ca3f2ab4ef9cada36b302c29e572cba3..134481f69b57cf9674fd7f5e758bc6c1e7188431 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/EnhancedBookmarksBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/EnhancedBookmarksBridge.java
@@ -246,6 +246,14 @@ public final class EnhancedBookmarksBridge {
     }
 
     /**
+     * Parses the web content of the given tab, and stores salient images to local database.
+     * @param tab The tab that the user is currently in.
+     */
+    public void fetchImageForTab(Tab tab) {

[Ted C, 2015/02/06 20:13:28]

I would pass WebContents here since I don't think you really need Tab at all.

[Kibeom Kim (inactive), 2015/02/06 20:54:02]

This will also update bookmark's metadata so the item should be bookmarked at
the time it is fetched, or it's just leaking to the image storage. So I think
it'd good to name the function so that it's clear at call-site.


Random note: To me, BookmarkImageService class seemed unsafe. I'm not sure it
considers various leaking scenarios. I'm not 100% sure, but for example, while
fetching the image, if user deleted the bookmark,
|BookmarkImageService::ProcessNewImage| will be called when the fetching is
done. It immediately stores to the database by |PostTaskToStoreImage|, and try
to update bookmark metadata later. If bookmark is not found, it just returns.
Then isn't it that there is no bookmark but image saved to the database.

[Ian Wen, 2015/02/06 22:48:43]

Hmmm.. The scenario you mentioned is not specific to this function, right? If
you call SalientImageForUrl(), and immediately after it you delete a bookmark,
is it still going to be stored by later callbacks? I would file another bug  for
it and modify ProcessForImage() to add an additional check.

[Ian Wen, 2015/02/09 21:53:17]

Done.

+        nativeFetchImageForTab(mNativeEnhancedBookmarksBridge, tab);
+    }
+
+    /**
      * Get all filters associated with the given bookmark.
      *
      * @param bookmark The bookmark to find filters for.
@@ -310,6 +318,7 @@ public final class EnhancedBookmarksBridge {
     private native BookmarkId nativeAddBookmark(long nativeEnhancedBookmarksBridge,
             BookmarkId parent, int index, String title, String url);
     private native void nativeSendSearchRequest(long nativeEnhancedBookmarksBridge, String query);
-    private static native void nativeSalientImageForUrl(long nativeEnhancedBookmarksBridge,
+    private native void nativeSalientImageForUrl(long nativeEnhancedBookmarksBridge,
             String url, SalientImageCallback callback);
+    private native void nativeFetchImageForTab(long nativeEnhancedBookmarksBridge, Tab tab);
 }