[Feed] Implement error handling in content storage

Implement error handling in content storage, and change callback in java
to lambda.

Bug: 884230

Change-Id: I0dd9999dc221e7fb4d030d2b8f56d16482b04985
Reviewed-on: https://chromium-review.googlesource.com/1226079Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Gang Wu <gangwu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591503}

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedContentBridge.java

@@ -49,20 +49,24 @@ public class FeedContentBridge {
         mNativeFeedContentBridge = 0;
     }
 
-    public void loadContent(List<String> keys, Callback<Map<String, byte[]>> callback) {
+    public void loadContent(List<String> keys, Callback<Map<String, byte[]>> successCallback,
+            Callback<Map<String, byte[]>> failureCallback) {
         assert mNativeFeedContentBridge != 0;
         String[] keysArray = keys.toArray(new String[keys.size()]);
-        nativeLoadContent(mNativeFeedContentBridge, keysArray, callback);
+        nativeLoadContent(mNativeFeedContentBridge, keysArray, successCallback, failureCallback);
     }
 
-    public void loadContentByPrefix(String prefix, Callback<Map<String, byte[]>> callback) {
+    public void loadContentByPrefix(String prefix, Callback<Map<String, byte[]>> successCallback,
+            Callback<Map<String, byte[]>> failureCallback) {
         assert mNativeFeedContentBridge != 0;
-        nativeLoadContentByPrefix(mNativeFeedContentBridge, prefix, callback);
+        nativeLoadContentByPrefix(
+                mNativeFeedContentBridge, prefix, successCallback, failureCallback);
     }
 
-    public void loadAllContentKeys(Callback<String[]> callback) {
+    public void loadAllContentKeys(
+            Callback<String[]> successCallback, Callback<String[]> failureCallback) {
         assert mNativeFeedContentBridge != 0;
-        nativeLoadAllContentKeys(mNativeFeedContentBridge, callback);
+        nativeLoadAllContentKeys(mNativeFeedContentBridge, successCallback, failureCallback);
     }
 
     public void commitContentMutation(ContentMutation contentMutation, Callback<Boolean> callback) {
@@ -111,12 +115,14 @@ public class FeedContentBridge {
 
     private native long nativeInit(Profile profile);
     private native void nativeDestroy(long nativeFeedContentBridge);
-    private native void nativeLoadContent(
-            long nativeFeedContentBridge, String[] keys, Callback<Map<String, byte[]>> callback);
-    private native void nativeLoadContentByPrefix(
-            long nativeFeedContentBridge, String prefix, Callback<Map<String, byte[]>> callback);
-    private native void nativeLoadAllContentKeys(
-            long nativeFeedContentBridge, Callback<String[]> callback);
+    private native void nativeLoadContent(long nativeFeedContentBridge, String[] keys,
+            Callback<Map<String, byte[]>> successCallback,
+            Callback<Map<String, byte[]>> failureCallback);
+    private native void nativeLoadContentByPrefix(long nativeFeedContentBridge, String prefix,
+            Callback<Map<String, byte[]>> successCallback,
+            Callback<Map<String, byte[]>> failureCallback);
+    private native void nativeLoadAllContentKeys(long nativeFeedContentBridge,
+            Callback<String[]> successCallback, Callback<String[]> failureCallback);
     private native void nativeCommitContentMutation(
             long nativeFeedContentBridge, Callback<Boolean> callback);
     private native void nativeCreateContentMutation(long nativeFeedContentBridge);

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedContentStorage.java

@@ -10,7 +10,8 @@ import com.google.android.libraries.feed.host.storage.CommitResult;
 import com.google.android.libraries.feed.host.storage.ContentMutation;
 import com.google.android.libraries.feed.host.storage.ContentStorage;
 
-import org.chromium.base.Callback;
+import org.chromium.base.VisibleForTesting;
+import org.chromium.chrome.browser.profiles.Profile;
 
 import java.util.Arrays;
 import java.util.List;
@@ -22,56 +23,22 @@ import java.util.Map;
 public class FeedContentStorage implements ContentStorage {
     private FeedContentBridge mFeedContentBridge;
 
-    private static class StorageCallback<T> implements Callback<T> {
-        private final Consumer<Result<T>> mConsumer;
-
-        public StorageCallback(Consumer<Result<T>> consumer) {
-            mConsumer = consumer;
-        }
-
-        @Override
-        public void onResult(T data) {
-            // TODO(gangwu): Need to handle failure case.
-            mConsumer.accept(Result.success(data));
-        }
-    }
-
-    private static class GetAllKeysCallback implements Callback<String[]> {
-        private final Consumer < Result < List<String>>> mConsumer;
-
-        public GetAllKeysCallback(Consumer < Result < List<String>>> consumer) {
-            mConsumer = consumer;
-        }
-
-        @Override
-        public void onResult(String[] keys) {
-            // TODO(gangwu): Need to handle failure case.
-            mConsumer.accept(Result.success(Arrays.asList(keys)));
-        }
-    }
-
-    private static class CommitCallback implements Callback<Boolean> {
-        private final Consumer<CommitResult> mConsumer;
-
-        CommitCallback(Consumer<CommitResult> consumer) {
-            mConsumer = consumer;
-        }
-
-        @Override
-        public void onResult(Boolean result) {
-            if (result) {
-                mConsumer.accept(CommitResult.SUCCESS);
-            } else {
-                mConsumer.accept(CommitResult.FAILURE);
-            }
-        }
+    /**
+     * Creates a {@link FeedContentStorage} for storing content for the current user.
+     *
+     * @param profile {@link Profile} of the user we are rendering the Feed for.
+     */
+    public FeedContentStorage(Profile profile) {
+        mFeedContentBridge = new FeedContentBridge();
+        mFeedContentBridge.init(profile);
     }
 
     /**
-     * Creates a {@link FeedContentStorage} for storing content for the current user.
+     * Creates a {@link FeedContentStorage} for testing.
      *
      * @param bridge {@link FeedContentBridge} implementation can handle content storage request.
      */
+    @VisibleForTesting
     public FeedContentStorage(FeedContentBridge bridge) {
         mFeedContentBridge = bridge;
     }
@@ -86,25 +53,35 @@ public class FeedContentStorage implements ContentStorage {
     @Override
     public void get(List<String> keys, Consumer < Result < Map<String, byte[]>>> consumer) {
         assert mFeedContentBridge != null;
-        mFeedContentBridge.loadContent(keys, new StorageCallback<Map<String, byte[]>>(consumer));
+        mFeedContentBridge.loadContent(keys,
+                (Map<String, byte[]> data)
+                        -> { consumer.accept(Result.success(data)); },
+                (Map<String, byte[]> data) -> { consumer.accept(Result.failure()); });
     }
 
     @Override
     public void getAll(String prefix, Consumer < Result < Map<String, byte[]>>> consumer) {
         assert mFeedContentBridge != null;
-        mFeedContentBridge.loadContentByPrefix(
-                prefix, new StorageCallback<Map<String, byte[]>>(consumer));
+        mFeedContentBridge.loadContentByPrefix(prefix,
+                (Map<String, byte[]> data)
+                        -> { consumer.accept(Result.success(data)); },
+                (Map<String, byte[]> data) -> { consumer.accept(Result.failure()); });
     }
 
     @Override
     public void commit(ContentMutation mutation, Consumer<CommitResult> consumer) {
         assert mFeedContentBridge != null;
-        mFeedContentBridge.commitContentMutation(mutation, new CommitCallback(consumer));
+        mFeedContentBridge.commitContentMutation(mutation, (Boolean result) -> {
+            consumer.accept(result ? CommitResult.SUCCESS : CommitResult.FAILURE);
+        });
     }
 
     @Override
     public void getAllKeys(Consumer < Result < List<String>>> consumer) {
         assert mFeedContentBridge != null;
-        mFeedContentBridge.loadAllContentKeys(new GetAllKeysCallback(consumer));
+        mFeedContentBridge.loadAllContentKeys(
+                (String[] keys)
+                        -> { consumer.accept(Result.success(Arrays.asList(keys))); },
+                (String[] keys) -> { consumer.accept(Result.failure()); });
     }
 }

chrome/android/junit/src/org/chromium/chrome/browser/feed/FeedContentStorageTest.java

@@ -80,27 +80,51 @@ public class FeedContentStorageTest {
     @Captor
     private ArgumentCaptor<Callback<Boolean>> mBooleanCallbackArgument;
     @Captor
-    private ArgumentCaptor<Callback<String[]>> mArrayOfStringCallbackArgument;
+    private ArgumentCaptor<Callback<String[]>> mArrayOfStringSuccessCallbackArgument;
     @Captor
-            private ArgumentCaptor < Callback < Map<String, byte[]>>> mMapCallbackArgument;
+    private ArgumentCaptor<Callback<String[]>> mArrayOfStringFailureCallbackArgument;
+    @Captor
+            private ArgumentCaptor < Callback < Map<String, byte[]>>> mMapSuccessCallbackArgument;
+    @Captor
+            private ArgumentCaptor < Callback < Map<String, byte[]>>> mMapFailureCallbackArgument;
 
     private FeedContentStorage mContentStorage;
 
-    private Answer<Void> createMapAnswer(Map<String, byte[]> map) {
+    private Answer<Void> createMapSuccessAnswer(Map<String, byte[]> map) {
+        return new Answer<Void>() {
+            @Override
+            public Void answer(InvocationOnMock invocation) {
+                mMapSuccessCallbackArgument.getValue().onResult(map);
+                return null;
+            }
+        };
+    }
+
+    private Answer<Void> createMapFailureAnswer(Map<String, byte[]> map) {
         return new Answer<Void>() {
             @Override
             public Void answer(InvocationOnMock invocation) {
-                mMapCallbackArgument.getValue().onResult(map);
+                mMapFailureCallbackArgument.getValue().onResult(map);
                 return null;
             }
         };
     }
 
-    private Answer<Void> createArrayOfStringAnswer(String[] arrayOfString) {
+    private Answer<Void> createArrayOfStringSuccessAnswer(String[] arrayOfString) {
         return new Answer<Void>() {
             @Override
             public Void answer(InvocationOnMock invocation) {
-                mArrayOfStringCallbackArgument.getValue().onResult(arrayOfString);
+                mArrayOfStringSuccessCallbackArgument.getValue().onResult(arrayOfString);
+                return null;
+            }
+        };
+    }
+
+    private Answer<Void> createArrayOfStringFailureAnswer(String[] arrayOfString) {
+        return new Answer<Void>() {
+            @Override
+            public Void answer(InvocationOnMock invocation) {
+                mArrayOfStringFailureCallbackArgument.getValue().onResult(arrayOfString);
                 return null;
             }
         };
@@ -155,17 +179,36 @@ public class FeedContentStorageTest {
     public void getTest() {
         Map<String, byte[]> answerMap = new HashMap<>();
         answerMap.put(CONTENT_KEY1, CONTENT_DATA1);
-        Answer<Void> answer = createMapAnswer(answerMap);
-        doAnswer(answer).when(mBridge).loadContent(
-                mStringListArgument.capture(), mMapCallbackArgument.capture());
+        Answer<Void> answer = createMapSuccessAnswer(answerMap);
+        doAnswer(answer).when(mBridge).loadContent(mStringListArgument.capture(),
+                mMapSuccessCallbackArgument.capture(), mMapFailureCallbackArgument.capture());
         List<String> keys = Arrays.asList(CONTENT_KEY1, CONTENT_KEY2);
 
         mContentStorage.get(keys, mMapConsumer);
-        verify(mBridge, times(1)).loadContent(eq(keys), mMapCallbackArgument.capture());
+        verify(mBridge, times(1))
+                .loadContent(eq(keys), mMapSuccessCallbackArgument.capture(),
+                        mMapFailureCallbackArgument.capture());
         verify(mMapConsumer, times(1)).accept(mMapCaptor.capture());
         verifyMapResult(answerMap, true, mMapCaptor.getValue());
     }
 
+    @Test
+    @SmallTest
+    public void getFailureTest() {
+        Map<String, byte[]> answerMap = new HashMap<>();
+        Answer<Void> answer = createMapFailureAnswer(answerMap);
+        doAnswer(answer).when(mBridge).loadContent(mStringListArgument.capture(),
+                mMapSuccessCallbackArgument.capture(), mMapFailureCallbackArgument.capture());
+        List<String> keys = Arrays.asList(CONTENT_KEY1, CONTENT_KEY2);
+
+        mContentStorage.get(keys, mMapConsumer);
+        verify(mBridge, times(1))
+                .loadContent(eq(keys), mMapSuccessCallbackArgument.capture(),
+                        mMapFailureCallbackArgument.capture());
+        verify(mMapConsumer, times(1)).accept(mMapCaptor.capture());
+        verifyMapResult(answerMap, false, mMapCaptor.getValue());
+    }
+
     @Test
     @SmallTest
     public void getAllTest() {
@@ -173,13 +216,14 @@ public class FeedContentStorageTest {
         answerMap.put(CONTENT_KEY1, CONTENT_DATA1);
         answerMap.put(CONTENT_KEY2, CONTENT_DATA2);
         answerMap.put(CONTENT_KEY3, CONTENT_DATA3);
-        Answer<Void> answer = createMapAnswer(answerMap);
-        doAnswer(answer).when(mBridge).loadContentByPrefix(
-                mStringArgument.capture(), mMapCallbackArgument.capture());
+        Answer<Void> answer = createMapSuccessAnswer(answerMap);
+        doAnswer(answer).when(mBridge).loadContentByPrefix(mStringArgument.capture(),
+                mMapSuccessCallbackArgument.capture(), mMapFailureCallbackArgument.capture());
 
         mContentStorage.getAll(CONTENT_KEY1, mMapConsumer);
         verify(mBridge, times(1))
-                .loadContentByPrefix(eq(CONTENT_KEY1), mMapCallbackArgument.capture());
+                .loadContentByPrefix(eq(CONTENT_KEY1), mMapSuccessCallbackArgument.capture(),
+                        mMapFailureCallbackArgument.capture());
         verify(mMapConsumer, times(1)).accept(mMapCaptor.capture());
         verifyMapResult(answerMap, true, mMapCaptor.getValue());
     }
@@ -188,15 +232,36 @@ public class FeedContentStorageTest {
     @SmallTest
     public void getAllKeysTest() {
         String[] answerStrings = {CONTENT_KEY1, CONTENT_KEY2, CONTENT_KEY3};
-        Answer<Void> answer = createArrayOfStringAnswer(answerStrings);
-        doAnswer(answer).when(mBridge).loadAllContentKeys(mArrayOfStringCallbackArgument.capture());
+        Answer<Void> answer = createArrayOfStringSuccessAnswer(answerStrings);
+        doAnswer(answer).when(mBridge).loadAllContentKeys(
+                mArrayOfStringSuccessCallbackArgument.capture(),
+                mArrayOfStringFailureCallbackArgument.capture());
 
         mContentStorage.getAllKeys(mListConsumer);
-        verify(mBridge, times(1)).loadAllContentKeys(mArrayOfStringCallbackArgument.capture());
+        verify(mBridge, times(1))
+                .loadAllContentKeys(mArrayOfStringSuccessCallbackArgument.capture(),
+                        mArrayOfStringFailureCallbackArgument.capture());
         verify(mListConsumer, times(1)).accept(mStringListCaptor.capture());
         verifyArrayOfStringResult(answerStrings, true, mStringListCaptor.getValue());
     }
 
+    @Test
+    @SmallTest
+    public void getAllKeysFailureTest() {
+        String[] answerStrings = {CONTENT_KEY1, CONTENT_KEY2, CONTENT_KEY3};
+        Answer<Void> answer = createArrayOfStringFailureAnswer(answerStrings);
+        doAnswer(answer).when(mBridge).loadAllContentKeys(
+                mArrayOfStringSuccessCallbackArgument.capture(),
+                mArrayOfStringFailureCallbackArgument.capture());
+
+        mContentStorage.getAllKeys(mListConsumer);
+        verify(mBridge, times(1))
+                .loadAllContentKeys(mArrayOfStringSuccessCallbackArgument.capture(),
+                        mArrayOfStringFailureCallbackArgument.capture());
+        verify(mListConsumer, times(1)).accept(mStringListCaptor.capture());
+        verifyArrayOfStringResult(answerStrings, false, mStringListCaptor.getValue());
+    }
+
     @Test
     @SmallTest
     public void commitTest() {

chrome/browser/android/feed/feed_content_bridge.cc

@@ -65,40 +65,50 @@ void FeedContentBridge::Destroy(JNIEnv* env, const JavaRef<jobject>& j_this) {
   delete this;
 }
 
-void FeedContentBridge::LoadContent(JNIEnv* j_env,
-                                    const JavaRef<jobject>& j_this,
-                                    const JavaRef<jobjectArray>& j_keys,
-                                    const JavaRef<jobject>& j_callback) {
+void FeedContentBridge::LoadContent(
+    JNIEnv* j_env,
+    const JavaRef<jobject>& j_this,
+    const JavaRef<jobjectArray>& j_keys,
+    const JavaRef<jobject>& j_success_callback,
+    const JavaRef<jobject>& j_failure_callback) {
   std::vector<std::string> keys;
   AppendJavaStringArrayToStringVector(j_env, j_keys.obj(), &keys);
-  ScopedJavaGlobalRef<jobject> callback(j_callback);
+  ScopedJavaGlobalRef<jobject> success_callback(j_success_callback);
+  ScopedJavaGlobalRef<jobject> failure_callback(j_failure_callback);
 
   feed_content_database_->LoadContent(
       keys, base::BindOnce(&FeedContentBridge::OnLoadContentDone,
-                           weak_ptr_factory_.GetWeakPtr(), callback));
+                           weak_ptr_factory_.GetWeakPtr(), success_callback,
+                           failure_callback));
 }
 
 void FeedContentBridge::LoadContentByPrefix(
     JNIEnv* j_env,
     const JavaRef<jobject>& j_this,
     const JavaRef<jstring>& j_prefix,
-    const JavaRef<jobject>& j_callback) {
+    const JavaRef<jobject>& j_success_callback,
+    const JavaRef<jobject>& j_failure_callback) {
   std::string prefix = ConvertJavaStringToUTF8(j_env, j_prefix);
-  ScopedJavaGlobalRef<jobject> callback(j_callback);
+  ScopedJavaGlobalRef<jobject> success_callback(j_success_callback);
+  ScopedJavaGlobalRef<jobject> failure_callback(j_failure_callback);
 
   feed_content_database_->LoadContentByPrefix(
       prefix, base::BindOnce(&FeedContentBridge::OnLoadContentDone,
-                             weak_ptr_factory_.GetWeakPtr(), callback));
+                             weak_ptr_factory_.GetWeakPtr(), success_callback,
+                             failure_callback));
 }
 
-void FeedContentBridge::LoadAllContentKeys(JNIEnv* j_env,
-                                           const JavaRef<jobject>& j_this,
-                                           const JavaRef<jobject>& j_callback) {
-  ScopedJavaGlobalRef<jobject> callback(j_callback);
-
-  feed_content_database_->LoadAllContentKeys(
-      base::BindOnce(&FeedContentBridge::OnLoadAllContentKeysDone,
-                     weak_ptr_factory_.GetWeakPtr(), callback));
+void FeedContentBridge::LoadAllContentKeys(
+    JNIEnv* j_env,
+    const JavaRef<jobject>& j_this,
+    const JavaRef<jobject>& j_success_callback,
+    const JavaRef<jobject>& j_failure_callback) {
+  ScopedJavaGlobalRef<jobject> success_callback(j_success_callback);
+  ScopedJavaGlobalRef<jobject> failure_callback(j_failure_callback);
+
+  feed_content_database_->LoadAllContentKeys(base::BindOnce(
+      &FeedContentBridge::OnLoadAllContentKeysDone,
+      weak_ptr_factory_.GetWeakPtr(), success_callback, failure_callback));
 }
 
 void FeedContentBridge::CommitContentMutation(
@@ -168,7 +178,9 @@ void FeedContentBridge::AppendDeleteAllOperation(
 }
 
 void FeedContentBridge::OnLoadContentDone(
-    ScopedJavaGlobalRef<jobject> callback,
+    ScopedJavaGlobalRef<jobject> success_callback,
+    ScopedJavaGlobalRef<jobject> failure_callback,
+    bool success,
     std::vector<FeedContentDatabase::KeyAndData> pairs) {
   std::vector<std::string> keys;
   std::vector<std::string> data;
@@ -184,16 +196,27 @@ void FeedContentBridge::OnLoadContentDone(
   // Create Java Map by JNI call.
   ScopedJavaLocalRef<jobject> j_pairs =
       Java_FeedContentBridge_createKeyAndDataMap(env, j_keys, j_data);
-  RunObjectCallbackAndroid(callback, j_pairs);
+
+  if (!success) {
+    RunObjectCallbackAndroid(failure_callback, j_pairs);
+    return;
+  }
+  RunObjectCallbackAndroid(success_callback, j_pairs);
 }
 
 void FeedContentBridge::OnLoadAllContentKeysDone(
-    ScopedJavaGlobalRef<jobject> callback,
+    ScopedJavaGlobalRef<jobject> success_callback,
+    ScopedJavaGlobalRef<jobject> failure_callback,
+    bool success,
     std::vector<std::string> keys) {
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobjectArray> j_keys = ToJavaArrayOfStrings(env, keys);
 
-  RunObjectCallbackAndroid(callback, j_keys);
+  if (!success) {
+    RunObjectCallbackAndroid(failure_callback, j_keys);
+    return;
+  }
+  RunObjectCallbackAndroid(success_callback, j_keys);
 }
 
 void FeedContentBridge::OnStorageCommitDone(

chrome/browser/android/feed/feed_content_bridge.h

@@ -33,14 +33,19 @@ class FeedContentBridge {
   void LoadContent(JNIEnv* j_env,
                    const base::android::JavaRef<jobject>& j_this,
                    const base::android::JavaRef<jobjectArray>& j_keys,
-                   const base::android::JavaRef<jobject>& j_callback);
-  void LoadContentByPrefix(JNIEnv* j_env,
-                           const base::android::JavaRef<jobject>& j_this,
-                           const base::android::JavaRef<jstring>& j_prefix,
-                           const base::android::JavaRef<jobject>& j_callback);
-  void LoadAllContentKeys(JNIEnv* j_env,
-                          const base::android::JavaRef<jobject>& j_this,
-                          const base::android::JavaRef<jobject>& j_callback);
+                   const base::android::JavaRef<jobject>& j_success_callback,
+                   const base::android::JavaRef<jobject>& j_failure_callback);
+  void LoadContentByPrefix(
+      JNIEnv* j_env,
+      const base::android::JavaRef<jobject>& j_this,
+      const base::android::JavaRef<jstring>& j_prefix,
+      const base::android::JavaRef<jobject>& j_success_callback,
+      const base::android::JavaRef<jobject>& j_failure_callback);
+  void LoadAllContentKeys(
+      JNIEnv* j_env,
+      const base::android::JavaRef<jobject>& j_this,
+      const base::android::JavaRef<jobject>& j_success_callback,
+      const base::android::JavaRef<jobject>& j_failure_callback);
   void CommitContentMutation(JNIEnv* j_env,
                              const base::android::JavaRef<jobject>& j_this,
                              const base::android::JavaRef<jobject>& j_callback);
@@ -64,10 +69,15 @@ class FeedContentBridge {
                                 const base::android::JavaRef<jobject>& j_this);
 
  private:
-  void OnLoadContentDone(base::android::ScopedJavaGlobalRef<jobject> callback,
-                         std::vector<FeedContentDatabase::KeyAndData> pairs);
+  void OnLoadContentDone(
+      base::android::ScopedJavaGlobalRef<jobject> success_callback,
+      base::android::ScopedJavaGlobalRef<jobject> failure_callback,
+      bool success,
+      std::vector<FeedContentDatabase::KeyAndData> pairs);
   void OnLoadAllContentKeysDone(
-      base::android::ScopedJavaGlobalRef<jobject> callback,
+      base::android::ScopedJavaGlobalRef<jobject> success_callback,
+      base::android::ScopedJavaGlobalRef<jobject> failure_callback,
+      bool success,
       std::vector<std::string> keys);
   void OnStorageCommitDone(base::android::ScopedJavaGlobalRef<jobject> callback,
                            bool success);

components/feed/core/feed_content_database.cc

@@ -236,14 +236,9 @@ void FeedContentDatabase::OnLoadEntriesForLoadContent(
     ContentLoadCallback callback,
     bool success,
     std::unique_ptr<std::vector<ContentStorageProto>> content) {
-  std::vector<KeyAndData> results;
-
-  if (!success || !content) {
-    DVLOG_IF(1, !success) << "FeedContentDatabase load content failed.";
-    std::move(callback).Run(std::move(results));
-    return;
-  }
+  DVLOG_IF(1, !success) << "FeedContentDatabase load content failed.";
 
+  std::vector<KeyAndData> results;
   for (const auto& proto : *content) {
     DCHECK(proto.has_key());
     DCHECK(proto.has_content_data());
@@ -251,24 +246,19 @@ void FeedContentDatabase::OnLoadEntriesForLoadContent(
     results.emplace_back(proto.key(), proto.content_data());
   }
 
-  std::move(callback).Run(std::move(results));
+  std::move(callback).Run(success, std::move(results));
 }
 
 void FeedContentDatabase::OnLoadKeysForLoadAllContentKeys(
     ContentKeyCallback callback,
     bool success,
     std::unique_ptr<std::vector<std::string>> keys) {
-  if (!success || !keys) {
-    DVLOG_IF(1, !success) << "FeedContentDatabase load content keys failed.";
-    std::vector<std::string> results;
-    std::move(callback).Run(std::move(results));
-    return;
-  }
+  DVLOG_IF(1, !success) << "FeedContentDatabase load content keys failed.";
 
   // We std::move the |*keys|'s entries to |callback|, after that, |keys| become
   // a pointer holding an empty vector, then we can safely delete unique_ptr
   // |keys| when it out of scope.
-  std::move(callback).Run(std::move(*keys));
+  std::move(callback).Run(success, std::move(*keys));
 }
 
 void FeedContentDatabase::OnOperationCommitted(

components/feed/core/feed_content_database.h

@@ -34,10 +34,12 @@ class FeedContentDatabase {
 
   // Returns the storage data as a vector of key-value pairs when calling
   // loading data.
-  using ContentLoadCallback = base::OnceCallback<void(std::vector<KeyAndData>)>;
+  using ContentLoadCallback =
+      base::OnceCallback<void(bool, std::vector<KeyAndData>)>;
 
   // Returns the content keys as a vector when calling loading all content keys.
-  using ContentKeyCallback = base::OnceCallback<void(std::vector<std::string>)>;
+  using ContentKeyCallback =
+      base::OnceCallback<void(bool, std::vector<std::string>)>;
 
   // Returns whether the commit operation succeeded when calling for database
   // operations, or return whether the entry exists when calling for checking

components/feed/core/feed_content_database_unittest.cc

@@ -66,9 +66,9 @@ class FeedContentDatabaseTest : public testing::Test {
 
   FeedContentDatabase* db() { return feed_db_.get(); }
 
-  MOCK_METHOD1(OnContentEntriesReceived,
-               void(std::vector<std::pair<std::string, std::string>>));
-  MOCK_METHOD1(OnContentKeyReceived, void(std::vector<std::string>));
+  MOCK_METHOD2(OnContentEntriesReceived,
+               void(bool, std::vector<std::pair<std::string, std::string>>));
+  MOCK_METHOD2(OnContentKeyReceived, void(bool, std::vector<std::string>));
   MOCK_METHOD1(OnStorageCommitted, void(bool));
 
  private:
@@ -96,7 +96,7 @@ TEST_F(FeedContentDatabaseTest, Init) {
 TEST_F(FeedContentDatabaseTest, LoadContentAfterInitSuccess) {
   CreateDatabase(/*init_database=*/true);
 
-  EXPECT_CALL(*this, OnContentEntriesReceived(_));
+  EXPECT_CALL(*this, OnContentEntriesReceived(_, _));
   db()->LoadContent(
       {kContentKey1},
       base::BindOnce(&FeedContentDatabaseTest::OnContentEntriesReceived,
@@ -113,8 +113,10 @@ TEST_F(FeedContentDatabaseTest, LoadContentsEntries) {
 
   // Try to Load |kContentKey2| and |kContentKey3|, only |kContentKey2| should
   // return.
-  EXPECT_CALL(*this, OnContentEntriesReceived(_))
-      .WillOnce([](std::vector<std::pair<std::string, std::string>> results) {
+  EXPECT_CALL(*this, OnContentEntriesReceived(_, _))
+      .WillOnce([](bool success,
+                   std::vector<std::pair<std::string, std::string>> results) {
+        EXPECT_TRUE(success);
         ASSERT_EQ(results.size(), 1U);
         EXPECT_EQ(results[0].first, kContentKey2);
         EXPECT_EQ(results[0].second, kContentData2);
@@ -135,8 +137,10 @@ TEST_F(FeedContentDatabaseTest, LoadContentsEntriesByPrefix) {
 
   // Try to Load "ContentKey", both |kContentKey1| and |kContentKey2| should
   // return.
-  EXPECT_CALL(*this, OnContentEntriesReceived(_))
-      .WillOnce([](std::vector<std::pair<std::string, std::string>> results) {
+  EXPECT_CALL(*this, OnContentEntriesReceived(_, _))
+      .WillOnce([](bool success,
+                   std::vector<std::pair<std::string, std::string>> results) {
+        EXPECT_TRUE(success);
         ASSERT_EQ(results.size(), 2U);
         EXPECT_EQ(results[0].first, kContentKey1);
         EXPECT_EQ(results[0].second, kContentData1);
@@ -157,8 +161,9 @@ TEST_F(FeedContentDatabaseTest, LoadAllContentKeys) {
   InjectContentStorageProto(kContentKey1, kContentData1);
   InjectContentStorageProto(kContentKey2, kContentData2);
 
-  EXPECT_CALL(*this, OnContentKeyReceived(_))
-      .WillOnce([](std::vector<std::string> results) {
+  EXPECT_CALL(*this, OnContentKeyReceived(_, _))
+      .WillOnce([](bool success, std::vector<std::string> results) {
+        EXPECT_TRUE(success);
         ASSERT_EQ(results.size(), 2U);
         EXPECT_EQ(results[0], kContentKey1);
         EXPECT_EQ(results[1], kContentKey2);
@@ -185,8 +190,10 @@ TEST_F(FeedContentDatabaseTest, SaveContent) {
   storage_db()->UpdateCallback(true);
 
   // Make sure they're there.
-  EXPECT_CALL(*this, OnContentEntriesReceived(_))
-      .WillOnce([](std::vector<std::pair<std::string, std::string>> results) {
+  EXPECT_CALL(*this, OnContentEntriesReceived(_, _))
+      .WillOnce([](bool success,
+                   std::vector<std::pair<std::string, std::string>> results) {
+        EXPECT_TRUE(success);
         ASSERT_EQ(results.size(), 2U);
         EXPECT_EQ(results[0].first, kContentKey1);
         EXPECT_EQ(results[0].second, kContentData1);
@@ -221,8 +228,10 @@ TEST_F(FeedContentDatabaseTest, DeleteContent) {
   storage_db()->UpdateCallback(true);
 
   // Make sure only |kContentKey2| got deleted.
-  EXPECT_CALL(*this, OnContentEntriesReceived(_))
-      .WillOnce([](std::vector<std::pair<std::string, std::string>> results) {
+  EXPECT_CALL(*this, OnContentEntriesReceived(_, _))
+      .WillOnce([](bool success,
+                   std::vector<std::pair<std::string, std::string>> results) {
+        EXPECT_TRUE(success);
         EXPECT_EQ(results.size(), 1U);
         EXPECT_EQ(results[0].first, kContentKey1);
         EXPECT_EQ(results[0].second, kContentData1);
@@ -253,8 +262,10 @@ TEST_F(FeedContentDatabaseTest, DeleteContentByPrefix) {
   storage_db()->UpdateCallback(true);
 
   // Make sure |kContentKey1| and |kContentKey2| got deleted.
-  EXPECT_CALL(*this, OnContentEntriesReceived(_))
-      .WillOnce([](std::vector<std::pair<std::string, std::string>> results) {
+  EXPECT_CALL(*this, OnContentEntriesReceived(_, _))
+      .WillOnce([](bool success,
+                   std::vector<std::pair<std::string, std::string>> results) {
+        EXPECT_TRUE(success);
         EXPECT_EQ(results.size(), 0U);
       });
   db()->LoadContent(
@@ -284,8 +295,10 @@ TEST_F(FeedContentDatabaseTest, DeleteAllContent) {
   storage_db()->UpdateCallback(true);
 
   // Make sure |kContentKey1| and |kContentKey2| got deleted.
-  EXPECT_CALL(*this, OnContentEntriesReceived(_))
-      .WillOnce([](std::vector<std::pair<std::string, std::string>> results) {
+  EXPECT_CALL(*this, OnContentEntriesReceived(_, _))
+      .WillOnce([](bool success,
+                   std::vector<std::pair<std::string, std::string>> results) {
+        EXPECT_TRUE(success);
         EXPECT_EQ(results.size(), 0U);
       });
   db()->LoadContent(
@@ -316,8 +329,10 @@ TEST_F(FeedContentDatabaseTest, SaveAndDeleteContent) {
   storage_db()->UpdateCallback(true);
 
   // Make sure only |kContentKey2| got deleted.
-  EXPECT_CALL(*this, OnContentEntriesReceived(_))
-      .WillOnce([](std::vector<std::pair<std::string, std::string>> results) {
+  EXPECT_CALL(*this, OnContentEntriesReceived(_, _))
+      .WillOnce([](bool success,
+                   std::vector<std::pair<std::string, std::string>> results) {
+        EXPECT_TRUE(success);
         EXPECT_EQ(results.size(), 1U);
         EXPECT_EQ(results[0].first, kContentKey1);
         EXPECT_EQ(results[0].second, kContentData1);
@@ -329,4 +344,40 @@ TEST_F(FeedContentDatabaseTest, SaveAndDeleteContent) {
   storage_db()->LoadCallback(true);
 }
 
+TEST_F(FeedContentDatabaseTest, LoadContentsFail) {
+  CreateDatabase(/*init_database=*/true);
+
+  // Store |kContentKey1| and |kContentKey2|.
+  InjectContentStorageProto(kContentKey1, kContentData1);
+  InjectContentStorageProto(kContentKey2, kContentData2);
+
+  // Try to Load |kContentKey2| and |kContentKey3|,.
+  EXPECT_CALL(*this, OnContentEntriesReceived(_, _))
+      .WillOnce([](bool success,
+                   std::vector<std::pair<std::string, std::string>> results) {
+        EXPECT_FALSE(success);
+      });
+  db()->LoadContent(
+      {kContentKey2, kContentKey3},
+      base::BindOnce(&FeedContentDatabaseTest::OnContentEntriesReceived,
+                     base::Unretained(this)));
+  storage_db()->LoadCallback(false);
+}
+
+TEST_F(FeedContentDatabaseTest, LoadAllContentKeysFail) {
+  CreateDatabase(/*init_database=*/true);
+
+  // Store |kContentKey1|, |kContentKey2|.
+  InjectContentStorageProto(kContentKey1, kContentData1);
+  InjectContentStorageProto(kContentKey2, kContentData2);
+
+  EXPECT_CALL(*this, OnContentKeyReceived(_, _))
+      .WillOnce([](bool success, std::vector<std::string> results) {
+        EXPECT_FALSE(success);
+      });
+  db()->LoadAllContentKeys(base::BindOnce(
+      &FeedContentDatabaseTest::OnContentKeyReceived, base::Unretained(this)));
+  storage_db()->LoadKeysCallback(false);
+}
+
 }  // namespace feed