Query API: Introduces an OfflinePageModelQuery object.

components/offline_pages/offline_page_model_impl_unittest.cc

 // 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.
 
 #include "components/offline_pages/offline_page_model_impl.h"
 
 #include <stdint.h>
 #include <algorithm>
 #include <utility>
 
@@ skipping 20 matching lines @@
 #include "components/offline_pages/offline_page_storage_manager.h"
 #include "components/offline_pages/offline_page_test_archiver.h"
 #include "components/offline_pages/offline_page_test_store.h"
 #include "components/offline_pages/offline_page_types.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 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");
 const GURL kTestUrl3("http://test.xyz");
 const GURL kTestUrl4("http://page.net");
 const GURL kFileUrl("file:///foo");
 const GURL kTestUrlWithFragment("http://example.com#frag");
 const GURL kTestUrl2WithFragment("http://other.page.com#frag");
 const GURL kTestUrl2WithFragment2("http://other.page.com#frag2");
 const ClientId kTestClientId1(kTestClientNamespace, "1234");
@@ skipping 30 matching lines @@
 
   // OfflinePageTestArchiver::Observer implementation.
   void SetLastPathCreatedByArchiver(const base::FilePath& file_path) override;
 
   // OfflinePageModel callbacks.
   void OnSavePageDone(SavePageResult result, int64_t offline_id);
   void OnDeletePageDone(DeletePageResult result);
   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);
   void OnPagesExpired(bool result);
 
   // OfflinePageMetadataStore callbacks.
   void OnStoreUpdateDone(bool /* success */);
 
   std::unique_ptr<OfflinePageTestArchiver> BuildArchiver(
       const GURL& url,
@@ skipping 32 matching lines @@
     offline_ids.push_back(offline_id);
     model()->DeletePagesByOfflineId(offline_ids, callback);
   }
 
   bool HasPages(std::string name_space);
 
   CheckPagesExistOfflineResult CheckPagesExistOffline(std::set<GURL>);
 
   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);
 
   OfflinePageModelImpl* model() { return model_.get(); }
 
   int64_t last_save_offline_id() const { return last_save_offline_id_; }
 
   SavePageResult last_save_result() const { return last_save_result_; }
 
   DeletePageResult last_delete_result() const { return last_delete_result_; }
@@ skipping 214 matching lines @@
   PumpLoop();
   return result;
 }
 
 void OfflinePageModelImplTest::OnGetOfflineIdsForClientIdDone(
     MultipleOfflineIdResult* storage,
     const MultipleOfflineIdResult& result) {
   *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,
                  AsWeakPtr(), base::Unretained(&result)));
   PumpLoop();
   return result;
 }
 
 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(
     MultipleOfflinePageItemResult* storage,
     const MultipleOfflinePageItemResult& result) {
   *storage = result;
 }
 
 void OfflinePageModelImplTest::OnPagesExpired(bool result) {
   last_expire_page_result_ = result;
@@ skipping 362 matching lines @@
   EXPECT_EQ(DeletePageResult::STORE_FAILURE, last_delete_result());
 }
 
 TEST_F(OfflinePageModelImplTest, DetectThatOfflineCopyIsMissing) {
   // Save a page.
   SavePage(kTestUrl, kTestClientId1);
   int64_t offline_id = last_save_offline_id();
 
   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);
 
   // Resetting the model will cause a consistency check.
   ResetModel();
 
   PumpLoop();
 
   // Check if the page has been expired.
   EXPECT_EQ(0UL, GetAllPages().size());
 }
 
 TEST_F(OfflinePageModelImplTest, DetectThatOfflineCopyIsMissingAfterLoad) {
   // Save a page.
   SavePage(kTestUrl, kTestClientId1);
   PumpLoop();
   int64_t offline_id = last_save_offline_id();
 
   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.
   ResetModel();
   PumpLoop();
 
   // Check if the page has been expired.
   EXPECT_EQ(0UL, GetAllPages().size());
 }
 
 TEST_F(OfflinePageModelImplTest, DetectThatHeadlessPageIsDeleted) {
   // Save a page.
   SavePage(kTestUrl, kTestClientId1);
   PumpLoop();
   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();
 
   EXPECT_TRUE(base::PathExists(path));
   // Since we've manually changed the store, we have to reload the model to
   // actually refresh the in-memory copy in model. Otherwise GetAllPages() would
   // still have the page we saved above.
   ResetModel();
   PumpLoop();
@@ skipping 45 matching lines @@
   EXPECT_EQ(DeletePageResult::SUCCESS, last_delete_result());
   EXPECT_EQ(1u, store->GetAllPages().size());
 }
 
 TEST_F(OfflinePageModelImplTest, GetPageByOfflineId) {
   SavePage(kTestUrl, kTestClientId1);
   int64_t offline1 = last_save_offline_id();
   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) {
   SavePage(kTestUrl, kTestClientId1);
   SavePage(kTestUrl2, kTestClientId2);
 
   MultipleOfflinePageItemResult pages = GetPagesByOnlineURL(kTestUrl2);
   EXPECT_EQ(1U, pages.size());
   EXPECT_EQ(kTestUrl2, pages[0].url);
   EXPECT_EQ(kTestClientId2, pages[0].client_id);
@@ skipping 330 matching lines @@
   // Check if feature is correctly enabled by command-line flag.
   scoped_feature_list.reset(new base::test::ScopedFeatureList);
   scoped_feature_list->InitFromCommandLine(
       std::string(kOfflineBookmarksFeature.name) + "," +
           kOfflinePagesSharingFeature.name,
       "");
   EXPECT_TRUE(offline_pages::IsOfflinePagesSharingEnabled());
 }
 
 }  // namespace offline_pages