Refactor the offline page storage to include client namespace and id.

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

 // Copyright 2015 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.offlinepages;
 
 import android.os.AsyncTask;
 
 import org.chromium.base.ObserverList;
 import org.chromium.base.ThreadUtils;
@@ skipping 70 matching lines @@
          * Called when the native side of offline pages is changed due to adding, removing or
          * update an offline page.
          */
         public void offlinePageModelChanged() {}
 
         /**
          * Called when an offline page is deleted. This can be called as a result of
          * #checkOfflinePageMetadata().
          * @param bookmarkId A bookmark ID of the deleted offline page.
          */
         public void offlinePageDeleted(BookmarkId bookmarkId) {}

[fgorski, 2016/02/23 17:15:30]

I think this is already an offlineId on the C++ side.

[bburns, 2016/02/23 19:25:38]

This changes the consumer API, I'd rather change that later.  (see below for the
conversion code.)

TODO'd

     }
 
     private static long getTotalSize(List<OfflinePageItem> offlinePages) {
         long totalSize = 0;
         for (OfflinePageItem offlinePage : offlinePages) {
             totalSize += offlinePage.getFileSize();
         }
         return totalSize;
     }
 
@@ skipping 113 matching lines @@
      * @return Gets all available offline pages. Requires that the model is already loaded.
      */
     public List<OfflinePageItem> getAllPages() {
         assert mIsNativeOfflinePageModelLoaded;
         List<OfflinePageItem> result = new ArrayList<OfflinePageItem>();
         nativeGetAllPages(mNativeOfflinePageBridge, result);
         return result;
     }
 
     /**
+     * @return A list of all offline ids that match a particular
+     * (namespace, client_id)
+     */
+    public long[] getOfflineIdsForClientId(String clientIdNamespace, String clientId) {

[fgorski, 2016/02/23 17:15:30]

Looking at all the other methods in this class, and across chromium codebase,
this should be a Set<Long>.

It should still be fine to get an array from native side, but you should wrap it
in a collection here.

[bburns, 2016/02/23 19:25:38]

Done.

+        assert mIsNativeOfflinePageModelLoaded;
+        return nativeGetOfflineIdsForClientId(mNativeOfflinePageBridge, clientIdNamespace,
+            clientId);
+    }
+
+    /**
      * Gets an offline page associated with a provided bookmark ID.
      *
      * @param bookmarkId Id of the bookmark associated with an offline page.
      * @return An {@link OfflinePageItem} matching the bookmark Id or <code>null</code> if none
      * exist.
      */
     public OfflinePageItem getPageByBookmarkId(BookmarkId bookmarkId) {

[fgorski, 2016/02/23 17:15:30]

It bothers me that we are not converting the bookmarkId to the offline page ID
here properly. At least add a todo. And we have quite a few places in this file
with the same problem.

[bburns, 2016/02/23 19:25:38]

Ok, I fixed that here.

-        return nativeGetPageByBookmarkId(mNativeOfflinePageBridge, bookmarkId.getId());
+        return nativeGetPageByOfflineId(mNativeOfflinePageBridge, bookmarkId.getId());
     }
 
     /**
      * Gets an offline page associated with a provided online URL.
      *
      * @param onlineURL URL of the page.
      * @return An {@link OfflinePageItem} matching the URL or <code>null</code> if none exist.
      */
     public OfflinePageItem getPageByOnlineURL(String onlineURL) {
         return nativeGetPageByOnlineURL(mNativeOfflinePageBridge, onlineURL);
@@ skipping 26 matching lines @@
                 // See http://crbug.com/566939
                 if (savePageResult == SavePageResult.SUCCESS && isOfflinePageModelLoaded()) {
                     long totalPageSizeAfter = getTotalSize(getAllPages());
                     recordStorageHistograms(0, totalPageSizeAfter);
                 }
                 callback.onSavePageDone(savePageResult, url);
             }
         };
         recordFreeSpaceHistograms(
                 "OfflinePages.SavePage.FreeSpacePercentage", "OfflinePages.SavePage.FreeSpaceMB");
-        nativeSavePage(mNativeOfflinePageBridge, callbackWrapper, webContents, bookmarkId.getId());
+        nativeSavePage(mNativeOfflinePageBridge, callbackWrapper, webContents, bookmarkId.getId(),
+                "bookmark", Long.toString(bookmarkId.getId()));
     }
 
     /**
      * Marks that an offline page related to a specified bookmark has been accessed.
      *
      * @param bookmarkId Bookmark ID for which the offline copy will be deleted.
      */
     public void markPageAccessed(BookmarkId bookmarkId) {
         assert mIsNativeOfflinePageModelLoaded;
         nativeMarkPageAccessed(mNativeOfflinePageBridge, bookmarkId.getId());
@@ skipping 110 matching lines @@
     }
 
     @CalledByNative
     private void offlinePageModelChanged() {
         for (OfflinePageModelObserver observer : mObservers) {
             observer.offlinePageModelChanged();
         }
     }
 
     @CalledByNative
-    private void offlinePageDeleted(long bookmarkId) {
-        BookmarkId id = new BookmarkId(bookmarkId, BookmarkType.NORMAL);
+    private void offlinePageDeleted(long offlineId) {
+        BookmarkId id = new BookmarkId(offlineId, BookmarkType.NORMAL);
         for (OfflinePageModelObserver observer : mObservers) {
             observer.offlinePageDeleted(id);
         }
     }
 
     @CalledByNative
     private static void createOfflinePageAndAddToList(List<OfflinePageItem> offlinePagesList,
-            String url, long bookmarkId, String offlineUrl, long fileSize, long creationTime,
+            String url, long offlineId, String offlineUrl, long fileSize, long creationTime,
             int accessCount, long lastAccessTimeMs) {
         offlinePagesList.add(createOfflinePageItem(
-                url, bookmarkId, offlineUrl, fileSize, creationTime, accessCount,
+                url, offlineId, offlineUrl, fileSize, creationTime, accessCount,
                 lastAccessTimeMs));
     }
 
     @CalledByNative
-    private static OfflinePageItem createOfflinePageItem(String url, long bookmarkId,
+    private static OfflinePageItem createOfflinePageItem(String url, long offlineId,
             String offlineUrl, long fileSize, long creationTime, int accessCount,
             long lastAccessTimeMs) {
         return new OfflinePageItem(
-                url, bookmarkId, offlineUrl, fileSize, creationTime, accessCount, lastAccessTimeMs);
+                url, offlineId, offlineUrl, fileSize, creationTime, accessCount, lastAccessTimeMs);
     }
 
     private static native int nativeGetFeatureMode();
     private static native boolean nativeCanSavePage(String url);
 
     private native long nativeInit(Profile profile);
     private native void nativeDestroy(long nativeOfflinePageBridge);
     private native void nativeGetAllPages(
             long nativeOfflinePageBridge, List<OfflinePageItem> offlinePages);
-    private native OfflinePageItem nativeGetPageByBookmarkId(
-            long nativeOfflinePageBridge, long bookmarkId);
+    private native long[] nativeGetOfflineIdsForClientId(
+            long nativeOfflinePageBridge, String clientNamespace, String clientId);
+    private native OfflinePageItem nativeGetPageByOfflineId(
+            long nativeOfflinePageBridge, long offlineId);
     private native OfflinePageItem nativeGetPageByOnlineURL(
             long nativeOfflinePageBridge, String onlineURL);
     private native void nativeSavePage(long nativeOfflinePageBridge, SavePageCallback callback,
-            WebContents webContents, long bookmarkId);
-    private native void nativeMarkPageAccessed(long nativeOfflinePageBridge, long bookmarkId);
+            WebContents webContents, long offlineId, String clientNamespace, String clientId);
+    private native void nativeMarkPageAccessed(long nativeOfflinePageBridge, long offlineId);
     private native void nativeDeletePage(long nativeOfflinePageBridge,
-            DeletePageCallback callback, long bookmarkId);
+            DeletePageCallback callback, long offlineId);
     private native void nativeDeletePages(
-            long nativeOfflinePageBridge, DeletePageCallback callback, long[] bookmarkIds);
+            long nativeOfflinePageBridge, DeletePageCallback callback, long[] offlineIds);
     private native void nativeGetPagesToCleanUp(
             long nativeOfflinePageBridge, List<OfflinePageItem> offlinePages);
     private native void nativeCheckMetadataConsistency(long nativeOfflinePageBridge);
     private native String nativeGetOfflineUrlForOnlineUrl(
             long nativeOfflinePageBridge, String onlineUrl);
     private native boolean nativeIsOfflinePageUrl(long nativeOfflinePageBridge, String offlineUrl);
 }