Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(716)

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java

Issue 2429943002: Remove all synchronous methods from OfflinePageBridge. (Closed)
Patch Set: Fix a test and address nits. Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeUnitTest.java » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 9e17f3c2be99f313a8d15ca548294977de83d4d4..b3912fc66c46162eb4bb6691f54fea3376809637 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
@@ -175,18 +175,6 @@ public class OfflinePageBridge {
nativeGetAllPages(mNativeOfflinePageBridge, result, callback);
}
- /** @return A list of all offline ids that match a particular (namespace, client_id) pair. */
- Set<Long> getOfflineIdsForClientId(ClientId clientId) {
- assert mIsNativeOfflinePageModelLoaded;
- long[] offlineIds = nativeGetOfflineIdsForClientId(
- mNativeOfflinePageBridge, clientId.getNamespace(), clientId.getId());
- Set<Long> result = new HashSet<>(offlineIds.length);
- for (long id : offlineIds) {
- result.add(id);
- }
- return result;
- }
-
/**
* Gets the offline pages associated with the provided client IDs.
*
@@ -197,16 +185,16 @@ public class OfflinePageBridge {
@VisibleForTesting
public void getPagesByClientIds(
final List<ClientId> clientIds, final Callback<List<OfflinePageItem>> callback) {
- runWhenLoaded(new Runnable() {
- @Override
- public void run() {
- List<OfflinePageItem> result = new ArrayList<>();
- for (ClientId clientId : clientIds) {
- result.addAll(getPagesByClientIdInternal(clientId));
- }
- callback.onResult(result);
- }
- });
+ String[] namespaces = new String[clientIds.size()];
+ String[] ids = new String[clientIds.size()];
+
+ for (int i = 0; i < clientIds.size(); i++) {
+ namespaces[i] = clientIds.get(i).getNamespace();
+ ids[i] = clientIds.get(i).getId();
+ }
+
+ List<OfflinePageItem> result = new ArrayList<>();
+ nativeGetPagesByClientId(mNativeOfflinePageBridge, result, namespaces, ids, callback);
}
/**
@@ -282,20 +270,7 @@ public class OfflinePageBridge {
mNativeOfflinePageBridge, requestIds, new RequestsRemovedCallback(callback));
}
- private List<OfflinePageItem> getPagesByClientIdInternal(ClientId clientId) {
- Set<Long> ids = getOfflineIdsForClientId(clientId);
- List<OfflinePageItem> result = new ArrayList<>();
- for (long offlineId : ids) {
- // TODO(dewittj): Restructure the native API to avoid this loop with a native call.
- OfflinePageItem item = nativeGetPageByOfflineId(mNativeOfflinePageBridge, offlineId);
- if (item != null) {
- result.add(item);
- }
- }
- return result;
- }
-
- /**
+ /**
* Get the offline page associated with the provided offline URL.
*
* @param onlineUrl URL of the page.
@@ -316,14 +291,7 @@ public class OfflinePageBridge {
* matching {@link OfflinePageItem} if found. Will pass back <code>null</code> if not.
*/
public void getPageByOfflineId(final long offlineId, final Callback<OfflinePageItem> callback) {
- runWhenLoaded(new Runnable() {
- @Override
- public void run() {
- OfflinePageItem item =
- nativeGetPageByOfflineId(mNativeOfflinePageBridge, offlineId);
- callback.onResult(item);
- }
- });
+ nativeGetPageByOfflineId(mNativeOfflinePageBridge, offlineId, callback);
}
/**
@@ -408,21 +376,15 @@ public class OfflinePageBridge {
* @param callback A callback that will be called once operation is completed.
*/
public void deletePagesByClientId(List<ClientId> clientIds, Callback<Integer> callback) {
- assert mIsNativeOfflinePageModelLoaded;
- List<Long> idList = new ArrayList<>(clientIds.size());
- for (ClientId clientId : clientIds) {
- idList.addAll(getOfflineIdsForClientId(clientId));
- }
- deletePages(idList, callback);
- }
+ String[] namespaces = new String[clientIds.size()];
+ String[] ids = new String[clientIds.size()];
- void deletePages(List<Long> offlineIds, Callback<Integer> callback) {
- long[] ids = new long[offlineIds.size()];
- for (int i = 0; i < offlineIds.size(); i++) {
- ids[i] = offlineIds.get(i);
+ for (int i = 0; i < clientIds.size(); i++) {
+ namespaces[i] = clientIds.get(i).getNamespace();
+ ids[i] = clientIds.get(i).getId();
}
- nativeDeletePages(mNativeOfflinePageBridge, callback, ids);
+ nativeDeletePagesByClientId(mNativeOfflinePageBridge, namespaces, ids, callback);
}
/**
@@ -470,30 +432,6 @@ public class OfflinePageBridge {
}
@VisibleForTesting
- ClientId getClientIdForOfflineId(long offlineId) {
- OfflinePageItem item = nativeGetPageByOfflineId(mNativeOfflinePageBridge, offlineId);
- if (item != null) {
- return item.getClientId();
- }
- return null;
- }
-
- private void runWhenLoaded(final Runnable runnable) {
- if (isOfflinePageModelLoaded()) {
- ThreadUtils.postOnUiThread(runnable);
- return;
- }
-
- addObserver(new OfflinePageModelObserver() {
- @Override
- public void offlinePageModelLoaded() {
- removeObserver(this);
- runnable.run();
- }
- });
- }
-
- @VisibleForTesting
static void setOfflineBookmarksEnabledForTesting(boolean enabled) {
sOfflineBookmarksEnabled = enabled;
}
@@ -567,21 +505,21 @@ public class OfflinePageBridge {
final Callback<List<OfflinePageItem>> callback);
private native void nativeCheckPagesExistOffline(long nativeOfflinePageBridge, Object[] urls,
CheckPagesExistOfflineCallbackInternal callback);
-
- @VisibleForTesting
- native long[] nativeGetOfflineIdsForClientId(
- long nativeOfflinePageBridge, String clientNamespace, String clientId);
-
@VisibleForTesting
native void nativeGetRequestsInQueue(
long nativeOfflinePageBridge, Callback<SavePageRequest[]> callback);
-
@VisibleForTesting
native void nativeRemoveRequestsFromQueue(
long nativeOfflinePageBridge, long[] requestIds, RequestsRemovedCallback callback);
-
@VisibleForTesting
- native OfflinePageItem nativeGetPageByOfflineId(long nativeOfflinePageBridge, long offlineId);
+ native void nativeGetPageByOfflineId(
+ long nativeOfflinePageBridge, long offlineId, Callback<OfflinePageItem> callback);
+ @VisibleForTesting
+ native void nativeGetPagesByClientId(long nativeOfflinePageBridge, List<OfflinePageItem> result,
+ String[] namespaces, String[] ids, Callback<List<OfflinePageItem>> callback);
+ @VisibleForTesting
+ native void nativeDeletePagesByClientId(long nativeOfflinePageBridge, String[] namespaces,
+ String[] ids, Callback<Integer> callback);
private native void nativeSelectPageForOnlineUrl(
long nativeOfflinePageBridge, String onlineUrl, int tabId,
Callback<OfflinePageItem> callback);
@@ -589,8 +527,6 @@ public class OfflinePageBridge {
WebContents webContents, String clientNamespace, String clientId);
private native void nativeSavePageLater(long nativeOfflinePageBridge, String url,
String clientNamespace, String clientId, boolean userRequested);
- private native void nativeDeletePages(
- long nativeOfflinePageBridge, Callback<Integer> callback, long[] offlineIds);
private native String nativeGetOfflinePageHeaderForReload(
long nativeOfflinePageBridge, WebContents webContents);
}
« no previous file with comments | « no previous file | chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeUnitTest.java » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698