[Offline Pages] Filtering expired pages and fix consistency check.

chrome/browser/android/offline_pages/offline_page_utils_unittest.cc

Index: chrome/browser/android/offline_pages/offline_page_utils_unittest.cc
diff --git a/chrome/browser/android/offline_pages/offline_page_utils_unittest.cc b/chrome/browser/android/offline_pages/offline_page_utils_unittest.cc
index 06173eacea2d88d0a9f0b27416e7e9476826e093..62c37497e20af1692209384c7f764347a25cfba1 100644
--- a/chrome/browser/android/offline_pages/offline_page_utils_unittest.cc
+++ b/chrome/browser/android/offline_pages/offline_page_utils_unittest.cc
@@ -35,9 +35,11 @@ namespace {
 const GURL kTestPage1Url("http://test.org/page1");
 const GURL kTestPage2Url("http://test.org/page2");
 const GURL kTestPage3Url("http://test.org/page3");
+const GURL kTestPage4Url("http://test.org/page4");
 const int64_t kTestFileSize = 876543LL;
 const char* kTestPage1ClientId = "1234";
 const char* kTestPage2ClientId = "5678";
+const char* kTestPage4ClientId = "9876";
 
 }  // namespace
 
@@ -55,6 +57,7 @@ class OfflinePageUtilsTest
   // Necessary callbacks for the offline page model.
   void OnSavePageDone(SavePageResult result, int64_t offlineId);
   void OnClearAllDone();
+  void OnExpirePageDone(bool success);
 
   // OfflinePageTestArchiver::Observer implementation:
   void SetLastPathCreatedByArchiver(const base::FilePath& file_path) override;
@@ -65,6 +68,8 @@ class OfflinePageUtilsTest
   const GURL& offline_url_page_2() const { return offline_url_page_2_; }
   // Offline page URL not related to any page.
   const GURL& offline_url_missing() const { return offline_url_missing_; }
+  // Offline page URL for expired page.
+  const GURL& offline_url_expired() const { return offline_url_expired_; }
 
   TestingProfile* profile() { return &profile_; }
 
@@ -79,6 +84,7 @@ class OfflinePageUtilsTest
   GURL offline_url_page_1_;
   GURL offline_url_page_2_;
   GURL offline_url_missing_;
+  GURL offline_url_expired_;
 
   int64_t offline_id_;
 
@@ -121,6 +127,10 @@ void OfflinePageUtilsTest::OnSavePageDone(SavePageResult result,
   offline_id_ = offline_id;
 }
 
+void OfflinePageUtilsTest::OnExpirePageDone(bool success) {
+  // Result ignored here.
+}
+
 void OfflinePageUtilsTest::OnClearAllDone() {
   // Result ignored here.
 }
@@ -142,29 +152,42 @@ void OfflinePageUtilsTest::CreateOfflinePages() {
       kTestPage1Url, client_id, std::move(archiver),
       base::Bind(&OfflinePageUtilsTest::OnSavePageDone, AsWeakPtr()));
   RunUntilIdle();
-  int64_t offline1 = offline_id();
+  offline_url_page_1_ =
+      model->MaybeGetPageByOfflineId(offline_id())->GetOfflineURL();
 
-  client_id.id = kTestPage2ClientId;
   // Create page 2.
   archiver = BuildArchiver(kTestPage2Url,
                            base::FilePath(FILE_PATH_LITERAL("page2.mhtml")));
+  client_id.id = kTestPage2ClientId;
   model->SavePage(
       kTestPage2Url, client_id, std::move(archiver),
       base::Bind(&OfflinePageUtilsTest::OnSavePageDone, AsWeakPtr()));
   RunUntilIdle();
-  int64_t offline2 = offline_id();
-
-  // Make a copy of local paths of the two pages stored in the model.
-  offline_url_page_1_ =
-      model->MaybeGetPageByOfflineId(offline1)->GetOfflineURL();
   offline_url_page_2_ =
-      model->MaybeGetPageByOfflineId(offline2)->GetOfflineURL();
+      model->MaybeGetPageByOfflineId(offline_id())->GetOfflineURL();
+
+  // Page 3 is not created, as it is missing.
   // Create a file path that is not associated with any offline page.
   offline_url_missing_ = net::FilePathToFileURL(
       profile()
           ->GetPath()
           .Append(chrome::kOfflinePageArchviesDirname)
           .Append(FILE_PATH_LITERAL("missing_file.mhtml")));
+
+  // Create page 4 - expired page.
+  archiver = BuildArchiver(kTestPage4Url,
+                           base::FilePath(FILE_PATH_LITERAL("page4.mhtml")));
+  client_id.id = kTestPage4ClientId;
+  model->SavePage(
+      kTestPage4Url, client_id, std::move(archiver),
+      base::Bind(&OfflinePageUtilsTest::OnSavePageDone, AsWeakPtr()));
+  RunUntilIdle();
+  const OfflinePageItem* page_4 = model->MaybeGetPageByOfflineId(offline_id());
+  offline_url_expired_ = page_4->GetOfflineURL();
+  model->ExpirePages(
+      std::vector<int64_t>({offline_id()}), base::Time::Now(),
+      base::Bind(&OfflinePageUtilsTest::OnExpirePageDone, AsWeakPtr()));
+  RunUntilIdle();
 }
 
 std::unique_ptr<OfflinePageTestArchiver> OfflinePageUtilsTest::BuildArchiver(
@@ -195,8 +218,10 @@ TEST_F(OfflinePageUtilsTest, MaybeGetOfflineURLForOnlineURL) {
   EXPECT_EQ(offline_url_page_2(),
             OfflinePageUtils::MaybeGetOfflineURLForOnlineURL(profile(),
                                                              kTestPage2Url));
-  EXPECT_EQ(GURL(), OfflinePageUtils::MaybeGetOfflineURLForOnlineURL(
-                        profile(), GURL(kTestPage3Url)));
+  EXPECT_EQ(GURL::EmptyGURL(), OfflinePageUtils::MaybeGetOfflineURLForOnlineURL(
+                                   profile(), kTestPage3Url));
+  EXPECT_EQ(GURL::EmptyGURL(), OfflinePageUtils::MaybeGetOfflineURLForOnlineURL(
+                                   profile(), GURL(kTestPage3Url)));

[fgorski, 2016/06/07 22:46:15]

I am not sure I understand this line.
Except for constructing new GURL it does the same.
Did you mean to add test for page 4 here?

[romax, 2016/06/08 01:31:25]

yeah.. copy paste.. (╯‵□′)╯︵┻━┻ 

 }
 
 TEST_F(OfflinePageUtilsTest, MaybeGetOnlineURLForOfflineURL) {
@@ -206,6 +231,8 @@ TEST_F(OfflinePageUtilsTest, MaybeGetOnlineURLForOfflineURL) {
                                profile(), offline_url_page_2()));
   EXPECT_EQ(GURL::EmptyGURL(), OfflinePageUtils::MaybeGetOnlineURLForOfflineURL(
                                    profile(), offline_url_missing()));
+  EXPECT_EQ(kTestPage4Url, OfflinePageUtils::GetOnlineURLForOfflineURL(
+                               profile(), offline_url_expired()));
 }
 
 TEST_F(OfflinePageUtilsTest, IsOfflinePage) {
@@ -213,8 +240,11 @@ TEST_F(OfflinePageUtilsTest, IsOfflinePage) {
   EXPECT_TRUE(OfflinePageUtils::IsOfflinePage(profile(), offline_url_page_2()));
   EXPECT_FALSE(
       OfflinePageUtils::IsOfflinePage(profile(), offline_url_missing()));
+  EXPECT_TRUE(
+      OfflinePageUtils::IsOfflinePage(profile(), offline_url_expired()));
   EXPECT_FALSE(OfflinePageUtils::IsOfflinePage(profile(), kTestPage1Url));
   EXPECT_FALSE(OfflinePageUtils::IsOfflinePage(profile(), kTestPage2Url));
+  EXPECT_FALSE(OfflinePageUtils::IsOfflinePage(profile(), kTestPage4Url));
 }
 
 TEST_F(OfflinePageUtilsTest, HasOfflinePageForOnlineURL) {
@@ -224,6 +254,8 @@ TEST_F(OfflinePageUtilsTest, HasOfflinePageForOnlineURL) {
       OfflinePageUtils::HasOfflinePageForOnlineURL(profile(), kTestPage2Url));
   EXPECT_FALSE(
       OfflinePageUtils::HasOfflinePageForOnlineURL(profile(), kTestPage3Url));
+  EXPECT_FALSE(
+      OfflinePageUtils::HasOfflinePageForOnlineURL(profile(), kTestPage4Url));
 }
 
 }  // namespace offline_pages