Makes the OfflinePageBridge.getAllPages method asynchronous.

chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java

Index: chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
index 5ae070cded84ac43c3a7196a1b69ce5c8d15526a..2b041543532e0874aedb8db554502f0ede0e18df 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
@@ -21,6 +21,7 @@ import org.chromium.content_public.browser.WebContents;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicLong;
 
 /**
  * Access gate to C++ side offline pages functionalities.
@@ -37,7 +38,7 @@ public final class OfflinePageBridge {
     private static Integer sFeatureMode;
 
     /**
-     * Callback used to saving an offline page.
+     * Callback used when saving an offline page.
      */
     public interface SavePageCallback {
         /**
@@ -53,7 +54,7 @@ public final class OfflinePageBridge {
     }
 
     /**
-     * Callback used to deleting an offline page.
+     * Callback used when deleting an offline page.
      */
     public interface DeletePageCallback {
         /**
@@ -68,9 +69,21 @@ public final class OfflinePageBridge {
     }
 
     /**
+     * Callback used when getting a list of all offline pages.
+     */
+    public interface GetAllPagesCallback {
+        void onGetAllPagesDone(List<OfflinePageItem> offlinePages);
+    }
+
+    /**
+     * Callback used when determining whether we have any offline pages.
+     */
+    public interface HasPagesCallback { void onHasPagesDone(boolean hasPages); }
+
+    /**
      * Base observer class listeners to be notified of changes to the offline page model.
      */
-    public abstract static class OfflinePageModelObserver {
+    public static class OfflinePageModelObserver {
         /**
          * Called when the native side of offline pages is loaded and now in usable state.
          */
@@ -90,7 +103,65 @@ public final class OfflinePageBridge {
         public void offlinePageDeleted(BookmarkId bookmarkId) {}
     }
 
-    private static long getTotalSize(List<OfflinePageItem> offlinePages) {
+    /**
+     * Class used to coordinate running async operations that require disk space changes to be
+     * recorded. Implement |performOperation| and call the provided Runnable if disk space
+     * changes should be recorded.
+     *
+     * Call |run| to initiate disk space measurement. |performOperation| will be called
+     * after the disk usage has been calculated.
+     */
+    private abstract class DiskSpaceHistogramWrappedOperation implements Runnable {

[fgorski, 2016/02/26 21:31:10]

Add a TODO for me to manage this piece of code out. ( mention bug 565571 which
should address this) 

I suspect we don't really use the the metrics that we are bending over backwards
to get here.

[dewittj, 2016/03/01 22:28:52]

Done.

+        private final AtomicLong mInitialSizeRef = new AtomicLong();
+
+        /**
+         * Starts a disk measurement operation, calling |performOperation| when complete.
+         */
+        @Override
+        public void run() {
+            // TODO(fgorski): Eliminate call to getAllPages() here.
+            // See http://crbug.com/566939
+            getAllPages(new GetAllPagesCallback() {
+                @Override
+                public void onGetAllPagesDone(List<OfflinePageItem> allPages) {
+                    mInitialSizeRef.set(getTotalSize(allPages));
+
+                    performOperation(new Runnable() {
+                        @Override
+                        public void run() {
+                            onCompleted();
+                        }
+                    });
+                }
+            });
+        }
+
+        /**
+         * The operation for which disk usage changes should be measured.
+         *
+         * @param success A runnable that should be called if disk space changes should be measured.
+         */
+        protected abstract void performOperation(Runnable success);
+
+        /**
+         * Called when performOperation completes in such a way that disk space changes need to be
+         * measured and reported via histograms.
+         */
+        private void onCompleted() {
+            if (isOfflinePageModelLoaded()) {
+                // TODO(fgorski): Eliminate call to getAllPages() here.
+                // See http://crbug.com/566939
+                getAllPages(new GetAllPagesCallback() {
+                    @Override
+                    public void onGetAllPagesDone(List<OfflinePageItem> allPages) {
+                        recordStorageHistograms(mInitialSizeRef.get(), getTotalSize(allPages));
+                    }
+                });
+            }
+        }
+    }
+
+    static long getTotalSize(List<OfflinePageItem> offlinePages) {
         long totalSize = 0;
         for (OfflinePageItem offlinePage : offlinePages) {
             totalSize += offlinePage.getFileSize();
@@ -211,9 +282,33 @@ public final class OfflinePageBridge {
     }
 
     /**
-     * @return Gets all available offline pages. Requires that the model is already loaded.
+     * Gets all available offline pages, returning results via the provided callback. Requires that
+     * the model is already loaded.
+     *
+     * @param callback The callback to run when the operation completes.
      */
-    public List<OfflinePageItem> getAllPages() {
+    public void getAllPages(final GetAllPagesCallback callback) {
+        ThreadUtils.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                callback.onGetAllPagesDone(getAllPagesInternal());
+            }
+        });
+    }
+
+    public void hasPages(final HasPagesCallback callback) {
+        getAllPages(new GetAllPagesCallback() {
+            @Override
+            public void onGetAllPagesDone(List<OfflinePageItem> allPages) {
+                callback.onHasPagesDone(!allPages.isEmpty());
+            }
+        });
+    }
+
+    /**
+     * @return All available offline pages. Requires that the model is already loaded.
+     */
+    private List<OfflinePageItem> getAllPagesInternal() {
         assert mIsNativeOfflinePageModelLoaded;
         List<OfflinePageItem> result = new ArrayList<OfflinePageItem>();
         nativeGetAllPages(mNativeOfflinePageBridge, result);
@@ -261,21 +356,25 @@ public final class OfflinePageBridge {
             return;
         }
 
-        SavePageCallback callbackWrapper = new SavePageCallback() {
+        recordFreeSpaceHistograms(
+                "OfflinePages.SavePage.FreeSpacePercentage", "OfflinePages.SavePage.FreeSpaceMB");
+
+        DiskSpaceHistogramWrappedOperation operation = new DiskSpaceHistogramWrappedOperation() {
             @Override
-            public void onSavePageDone(int savePageResult, String url) {
-                // TODO(fgorski): Eliminate call to getAllPages() here.
-                // See http://crbug.com/566939
-                if (savePageResult == SavePageResult.SUCCESS && isOfflinePageModelLoaded()) {
-                    long totalPageSizeAfter = getTotalSize(getAllPages());
-                    recordStorageHistograms(0, totalPageSizeAfter);
-                }
-                callback.onSavePageDone(savePageResult, url);
+            public void performOperation(final Runnable success) {
+                nativeSavePage(mNativeOfflinePageBridge, new SavePageCallback() {
+                    @Override
+                    public void onSavePageDone(int savePageResult, String url) {
+                        if (savePageResult == SavePageResult.SUCCESS) {
+                            success.run();
+                        }
+                        callback.onSavePageDone(savePageResult, url);
+                    }
+                }, webContents, bookmarkId.getId());
             }
         };
-        recordFreeSpaceHistograms(
-                "OfflinePages.SavePage.FreeSpacePercentage", "OfflinePages.SavePage.FreeSpaceMB");
-        nativeSavePage(mNativeOfflinePageBridge, callbackWrapper, webContents, bookmarkId.getId());
+
+        operation.run();
     }
 
     /**
@@ -297,12 +396,10 @@ public final class OfflinePageBridge {
      */
     public void deletePage(final BookmarkId bookmarkId, DeletePageCallback callback) {
         assert mIsNativeOfflinePageModelLoaded;
+        ArrayList<BookmarkId> ids = new ArrayList<BookmarkId>();
+        ids.add(bookmarkId);
 
-        recordFreeSpaceHistograms("OfflinePages.DeletePage.FreeSpacePercentage",
-                "OfflinePages.DeletePage.FreeSpaceMB");
-
-        DeletePageCallback callbackWrapper = wrapCallbackWithHistogramReporting(callback);
-        nativeDeletePage(mNativeOfflinePageBridge, callbackWrapper, bookmarkId.getId());
+        deletePages(ids, callback);
     }
 
     /**
@@ -312,9 +409,9 @@ public final class OfflinePageBridge {
      * @param bookmarkIds A list of bookmark IDs for which the offline pages will be deleted.
      * @param callback A callback that will be called once operation is completed.
      */
-    public void deletePages(List<BookmarkId> bookmarkIds, DeletePageCallback callback) {
+    public void deletePages(List<BookmarkId> bookmarkIds, final DeletePageCallback callback) {
         assert mIsNativeOfflinePageModelLoaded;
-        long[] ids = new long[bookmarkIds.size()];
+        final long[] ids = new long[bookmarkIds.size()];
         for (int i = 0; i < ids.length; i++) {
             ids[i] = bookmarkIds.get(i).getId();
         }
@@ -322,8 +419,22 @@ public final class OfflinePageBridge {
         recordFreeSpaceHistograms("OfflinePages.DeletePage.FreeSpacePercentage",
                 "OfflinePages.DeletePage.FreeSpaceMB");
 
-        DeletePageCallback callbackWrapper = wrapCallbackWithHistogramReporting(callback);
-        nativeDeletePages(mNativeOfflinePageBridge, callbackWrapper, ids);
+        DiskSpaceHistogramWrappedOperation operation = new DiskSpaceHistogramWrappedOperation() {
+            @Override
+            public void performOperation(final Runnable success) {
+                nativeDeletePages(mNativeOfflinePageBridge, new DeletePageCallback() {
+                    @Override
+                    public void onDeletePageDone(int deletePageResult) {
+                        if (deletePageResult == DeletePageResult.SUCCESS) {
+                            success.run();
+                        }
+                        callback.onDeletePageDone(deletePageResult);
+                    }
+                }, ids);
+            }
+        };
+
+        operation.run();
     }
 
     /**
@@ -398,23 +509,6 @@ public final class OfflinePageBridge {
         return nativeGetOfflineUrlForOnlineUrl(mNativeOfflinePageBridge, onlineUrl);
     }
 
-    private DeletePageCallback wrapCallbackWithHistogramReporting(
-            final DeletePageCallback callback) {
-        final long totalPageSizeBefore = getTotalSize(getAllPages());
-        return new DeletePageCallback() {
-            @Override
-            public void onDeletePageDone(int deletePageResult) {
-                // TODO(fgorski): Eliminate call to getAllPages() here.
-                // See http://crbug.com/566939
-                if (deletePageResult == DeletePageResult.SUCCESS && isOfflinePageModelLoaded()) {
-                    long totalPageSizeAfter = getTotalSize(getAllPages());
-                    recordStorageHistograms(totalPageSizeBefore, totalPageSizeAfter);
-                }
-                callback.onDeletePageDone(deletePageResult);
-            }
-        };
-    }
-
     @CalledByNative
     private void offlinePageModelLoaded() {
         mIsNativeOfflinePageModelLoaded = true;
@@ -469,8 +563,6 @@ public final class OfflinePageBridge {
     private native void nativeSavePage(long nativeOfflinePageBridge, SavePageCallback callback,
             WebContents webContents, long bookmarkId);
     private native void nativeMarkPageAccessed(long nativeOfflinePageBridge, long bookmarkId);
-    private native void nativeDeletePage(long nativeOfflinePageBridge,
-            DeletePageCallback callback, long bookmarkId);
     private native void nativeDeletePages(
             long nativeOfflinePageBridge, DeletePageCallback callback, long[] bookmarkIds);
     private native void nativeGetPagesToCleanUp(