Query API: Introduces an OfflinePageModelQuery object.

components/offline_pages/offline_page_model_impl_unittest.cc

Index: components/offline_pages/offline_page_model_impl_unittest.cc
diff --git a/components/offline_pages/offline_page_model_impl_unittest.cc b/components/offline_pages/offline_page_model_impl_unittest.cc
index fa00760796b4d4733d7342277fc070b43a1b0021..e3ae2f94bdf34030286f1f4d215205f781406be7 100644
--- a/components/offline_pages/offline_page_model_impl_unittest.cc
+++ b/components/offline_pages/offline_page_model_impl_unittest.cc
@@ -38,7 +38,7 @@
 namespace offline_pages {
 
 namespace {
-const char kTestClientNamespace[] = "CLIENT_NAMESPACE";
+const char kTestClientNamespace[] = "default";
 const char kUserRequestedNamespace[] = "download";
 const GURL kTestUrl("http://example.com");
 const GURL kTestUrl2("http://other.page.com");
@@ -89,8 +89,9 @@ class OfflinePageModelImplTest
   void OnCheckPagesExistOfflineDone(const CheckPagesExistOfflineResult& result);
   void OnGetOfflineIdsForClientIdDone(MultipleOfflineIdResult* storage,
                                       const MultipleOfflineIdResult& result);
-  void OnGetSingleOfflinePageItemResult(const OfflinePageItem** storage,
-                                        const OfflinePageItem* result);
+  void OnGetSingleOfflinePageItemResult(
+      std::unique_ptr<OfflinePageItem>* storage,
+      const OfflinePageItem* result);
   void OnGetMultipleOfflinePageItemsResult(
       MultipleOfflinePageItemResult* storage,
       const MultipleOfflinePageItemResult& result);
@@ -143,7 +144,7 @@ class OfflinePageModelImplTest
 
   MultipleOfflineIdResult GetOfflineIdsForClientId(const ClientId& client_id);
 
-  const OfflinePageItem* GetPageByOfflineId(int64_t offline_id);
+  std::unique_ptr<OfflinePageItem> GetPageByOfflineId(int64_t offline_id);
 
   MultipleOfflinePageItemResult GetPagesByOnlineURL(const GURL& online_url);
 
@@ -378,9 +379,9 @@ void OfflinePageModelImplTest::OnGetOfflineIdsForClientIdDone(
   *storage = result;
 }
 
-const OfflinePageItem* OfflinePageModelImplTest::GetPageByOfflineId(
+std::unique_ptr<OfflinePageItem> OfflinePageModelImplTest::GetPageByOfflineId(
     int64_t offline_id) {
-  const OfflinePageItem* result = nullptr;
+  std::unique_ptr<OfflinePageItem> result = nullptr;
   model()->GetPageByOfflineId(
       offline_id,
       base::Bind(&OfflinePageModelImplTest::OnGetSingleOfflinePageItemResult,
@@ -390,9 +391,14 @@ const OfflinePageItem* OfflinePageModelImplTest::GetPageByOfflineId(
 }
 
 void OfflinePageModelImplTest::OnGetSingleOfflinePageItemResult(
-    const OfflinePageItem** storage,
+    std::unique_ptr<OfflinePageItem>* storage,
     const OfflinePageItem* result) {
-  *storage = result;
+  if (result == nullptr) {
+    storage->reset(nullptr);
+    return;
+  }
+
+  *storage = base::MakeUnique<OfflinePageItem>(*result);
 }
 
 void OfflinePageModelImplTest::OnGetMultipleOfflinePageItemsResult(
@@ -775,7 +781,7 @@ TEST_F(OfflinePageModelImplTest, DetectThatOfflineCopyIsMissing) {
 
   ResetResults();
 
-  const OfflinePageItem* page = GetPageByOfflineId(offline_id);
+  std::unique_ptr<OfflinePageItem> page = GetPageByOfflineId(offline_id);
 
   // Delete the offline copy of the page.
   base::DeleteFile(page->file_path, false);
@@ -797,7 +803,7 @@ TEST_F(OfflinePageModelImplTest, DetectThatOfflineCopyIsMissingAfterLoad) {
 
   ResetResults();
 
-  const OfflinePageItem* page = GetPageByOfflineId(offline_id);
+  std::unique_ptr<OfflinePageItem> page = GetPageByOfflineId(offline_id);
   // Delete the offline copy of the page and check the metadata.
   base::DeleteFile(page->file_path, false);
   // Reseting the model should trigger the metadata consistency check as well.
@@ -815,7 +821,7 @@ TEST_F(OfflinePageModelImplTest, DetectThatHeadlessPageIsDeleted) {
   int64_t offline_id = last_save_offline_id();
 
   ResetResults();
-  const OfflinePageItem* page = GetPageByOfflineId(offline_id);
+  std::unique_ptr<OfflinePageItem> page = GetPageByOfflineId(offline_id);
   base::FilePath path = page->file_path;
   EXPECT_TRUE(base::PathExists(path));
   GetStore()->ClearAllPages();
@@ -881,20 +887,20 @@ TEST_F(OfflinePageModelImplTest, GetPageByOfflineId) {
   SavePage(kTestUrl2, kTestClientId2);
   int64_t offline2 = last_save_offline_id();
 
-  const OfflinePageItem* page = GetPageByOfflineId(offline1);
-  EXPECT_TRUE(page);
+  std::unique_ptr<OfflinePageItem> page = GetPageByOfflineId(offline1);
+  ASSERT_NE(nullptr, page);

[fgorski, 2016/10/26 18:05:41]

shouldn't comparisons to nullptr be combined with get() ?

ASSERT_TRUE(page); is more appropriate here and below I think.

[dewittj, 2016/10/27 22:49:17]

http://en.cppreference.com/w/cpp/memory/unique_ptr/operator_cmp

std::unique_ptr implements comparisons with nullptr.

But I agree that ASSERT_TRUE is better looking.

[fgorski, 2016/10/28 02:40:39]

OK, I was looking for that an couldn't find. Had I known earlier I wouldn't
complain, but I am glad you like the alternative.

   EXPECT_EQ(kTestUrl, page->url);
   EXPECT_EQ(kTestClientId1, page->client_id);
   EXPECT_EQ(kTestFileSize, page->file_size);
 
   page = GetPageByOfflineId(offline2);
-  EXPECT_TRUE(page);
+  ASSERT_NE(nullptr, page);
   EXPECT_EQ(kTestUrl2, page->url);
   EXPECT_EQ(kTestClientId2, page->client_id);
   EXPECT_EQ(kTestFileSize, page->file_size);
 
   page = GetPageByOfflineId(-42);
-  EXPECT_FALSE(page);
+  ASSERT_EQ(nullptr, page);
 }
 
 TEST_F(OfflinePageModelImplTest, GetPagesByOnlineURL) {