[Offline Pages] Removes two-step expiration related.

components/offline_pages/offline_page_metadata_store_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_metadata_store.h"
 
 #include <stdint.h>
 
 #include <memory>
 
@@ skipping 131 matching lines @@
                                  "version INTEGER NOT NULL, "
                                  "last_access_time INTEGER NOT NULL, "
                                  "access_count INTEGER NOT NULL, "
                                  "status INTEGER NOT NULL DEFAULT 0, "
                                  "user_initiated INTEGER, "
                                  "expiration_time INTEGER NOT NULL DEFAULT 0, "
                                  "client_namespace VARCHAR NOT NULL, "
                                  "client_id VARCHAR NOT NULL, "
                                  "online_url VARCHAR NOT NULL, "
                                  "offline_url VARCHAR NOT NULL DEFAULT '', "
-                                 "file_path VARCHAR NOT NULL "
+                                 "file_path VARCHAR NOT NULL, "
                                  "title VARCHAR NOT NULL DEFAULT ''"
                                  ")"));
   ASSERT_TRUE(connection.CommitTransaction());
   sql::Statement statement(connection.GetUniqueStatement(
       "INSERT INTO " OFFLINE_PAGES_TABLE_V1
       "(offline_id, creation_time, file_size, version, "
       "last_access_time, access_count, client_namespace, "
       "client_id, online_url, file_path, expiration_time, title) "
-      "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
+      "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
   statement.BindInt64(0, kOfflineId);
   statement.BindInt(1, 0);
   statement.BindInt64(2, kFileSize);
   statement.BindInt(3, 0);
   statement.BindInt(4, 0);
   statement.BindInt(5, 1);
   statement.BindCString(6, kTestClientNamespace);
   statement.BindString(7, kTestClientId2.id);
   statement.BindCString(8, kTestURL);
   statement.BindString(9, base::FilePath(kFilePath).MaybeAsASCII());
@@ skipping 42 matching lines @@
   statement.BindInt(4, 1);
   statement.BindCString(5, kTestClientNamespace);
   statement.BindString(6, kTestClientId2.id);
   statement.BindCString(7, kTestURL);
   statement.BindString(8, base::FilePath(kFilePath).MaybeAsASCII());
   statement.BindInt64(9, base::Time::Now().ToInternalValue());
   statement.BindString16(10, base::UTF8ToUTF16("Test title"));
   ASSERT_TRUE(statement.Run());
   ASSERT_TRUE(connection.DoesTableExist(OFFLINE_PAGES_TABLE_V1));
   ASSERT_TRUE(connection.DoesColumnExist(OFFLINE_PAGES_TABLE_V1, "title"));
+  ASSERT_FALSE(connection.DoesColumnExist(OFFLINE_PAGES_TABLE_V1, "version"));

[fgorski, 2016/11/18 00:13:04]

don't touch M55 code, Jian has appropriate fix in one of his patches.

I believe your change will land in M56, please create a method for that

[romax, 2016/11/18 20:50:48]

Done.

+  ASSERT_FALSE(connection.DoesColumnExist(OFFLINE_PAGES_TABLE_V1, "status"));
+  ASSERT_FALSE(
+      connection.DoesColumnExist(OFFLINE_PAGES_TABLE_V1, "user_initiated"));
+  ASSERT_FALSE(
+      connection.DoesColumnExist(OFFLINE_PAGES_TABLE_V1, "offline_url"));
 }
 
 class OfflinePageMetadataStoreFactory {
  public:
   OfflinePageMetadataStore* BuildStore(const base::FilePath& file_path) {
     OfflinePageMetadataStoreSQL* store = new OfflinePageMetadataStoreSQL(
         base::ThreadTaskRunnerHandle::Get(), file_path);
     return store;
   }
 
@@ skipping 135 matching lines @@
 
   return offline_pages_[0];
 }
 
 void OfflinePageMetadataStoreTest::CheckThatOfflinePageCanBeSaved(
     std::unique_ptr<OfflinePageMetadataStore> store) {
   size_t store_size = offline_pages_.size();
   OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestClientId1,
                                base::FilePath(kFilePath), kFileSize);
   offline_page.title = base::UTF8ToUTF16("a title");
-  base::Time expiration_time = base::Time::Now();
-  offline_page.expiration_time = expiration_time;
-  offline_page.original_url = GURL(kOriginalTestURL);

[fgorski, 2016/11/18 00:13:04]

be careful about what you are removing. original_url should stay

[romax, 2016/11/18 20:50:49]

Done.

 
   store->AddOfflinePage(offline_page,
                         base::Bind(&OfflinePageMetadataStoreTest::AddCallback,
                                    base::Unretained(this)));
   PumpLoop();
   EXPECT_EQ(ADD, last_called_callback_);
   EXPECT_EQ(STATUS_TRUE, last_status_);
   ClearResults();
 
   // Close the store first to ensure file lock is removed.
@@ skipping 43 matching lines @@
   store->GetOfflinePages(
       base::Bind(&OfflinePageMetadataStoreTest::GetOfflinePagesCallback,
                  base::Unretained(this)));
   PumpLoop();
   return store;
 }
 
 std::unique_ptr<OfflinePageMetadataStore>
 OfflinePageMetadataStoreTest::BuildStoreWithSchemaFromM54() {
   std::unique_ptr<OfflinePageMetadataStore> store(
-      factory_.BuildStoreM53(temp_directory_.GetPath()));
+      factory_.BuildStoreM54(temp_directory_.GetPath()));
   PumpLoop();
   store->GetOfflinePages(
       base::Bind(&OfflinePageMetadataStoreTest::GetOfflinePagesCallback,
                  base::Unretained(this)));
   PumpLoop();
   return store;
 }
 
 std::unique_ptr<OfflinePageMetadataStore>
 OfflinePageMetadataStoreTest::BuildStoreWithSchemaFromM55() {
@@ skipping 87 matching lines @@
 }
 
 // Loads a store which has an outdated schema.
 // This test case would crash if it's not handling correctly when we're loading
 // old version stores.
 // TODO(romax): Move this to sql_unittest.
 TEST_F(OfflinePageMetadataStoreTest, LoadVersion52Store) {
   std::unique_ptr<OfflinePageMetadataStore> store(
       BuildStoreWithSchemaFromM52());
 
-  CheckThatStoreHasOneItem();
+  OfflinePageItem item = CheckThatStoreHasOneItem();
   CheckThatOfflinePageCanBeSaved(std::move(store));
 }
 
 // Loads a store which has an outdated schema.
 // This test case would crash if it's not handling correctly when we're loading
 // old version stores.
 // TODO(romax): Move this to sql_unittest.
 TEST_F(OfflinePageMetadataStoreTest, LoadVersion53Store) {
   std::unique_ptr<OfflinePageMetadataStore> store(
       BuildStoreWithSchemaFromM53());
 
   OfflinePageItem item = CheckThatStoreHasOneItem();
-  // We should have a valid expiration time after upgrade.
-  EXPECT_NE(base::Time::FromInternalValue(0),
-            offline_pages_[0].expiration_time);
-
   CheckThatOfflinePageCanBeSaved(std::move(store));
 }
 
 // Loads a string with schema from M54.
 // Because for now we only reduce the number of fields it just makes sure there
 // are no crashes in the process.
 // TODO(romax): Move this to sql_unittest.
 TEST_F(OfflinePageMetadataStoreTest, LoadVersion54Store) {
   std::unique_ptr<OfflinePageMetadataStore> store(
       BuildStoreWithSchemaFromM54());
 
   OfflinePageItem item = CheckThatStoreHasOneItem();
-
   CheckThatOfflinePageCanBeSaved(std::move(store));
 }
 
 // Loads a string with schema from M55.
-// Because for now we only reduce the number of fields it just makes sure there
-// are no crashes in the process.
+// Because for now we only removed 'title' and added 'original_url' , it just
+// makes sure there are no crashes in the process.
 // TODO(romax): Move this to sql_unittest.
 TEST_F(OfflinePageMetadataStoreTest, LoadVersion55Store) {
   std::unique_ptr<OfflinePageMetadataStore> store(
       BuildStoreWithSchemaFromM55());
 
   OfflinePageItem item = CheckThatStoreHasOneItem();
-
   CheckThatOfflinePageCanBeSaved(std::move(store));
 }
 
 // Adds metadata of an offline page into a store and then opens the store
 // again to make sure that stored metadata survives store restarts.
 TEST_F(OfflinePageMetadataStoreTest, AddOfflinePage) {
   CheckThatOfflinePageCanBeSaved(BuildStore());
 }
 
 TEST_F(OfflinePageMetadataStoreTest, AddSameOfflinePageTwice) {
   std::unique_ptr<OfflinePageMetadataStore> store(BuildStore());
 
   OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestClientId1,
                                base::FilePath(kFilePath), kFileSize);
   offline_page.title = base::UTF8ToUTF16("a title");
-  base::Time expiration_time = base::Time::Now();
-  offline_page.expiration_time = expiration_time;
 
   store->AddOfflinePage(offline_page,
                         base::Bind(&OfflinePageMetadataStoreTest::AddCallback,
                                    base::Unretained(this)));
   PumpLoop();
   EXPECT_EQ(ADD, last_called_callback_);
   EXPECT_EQ(STATUS_TRUE, last_status_);
   ClearResults();
 
   store->AddOfflinePage(offline_page,
@@ skipping 65 matching lines @@
   EXPECT_EQ(0U, offline_pages_.size());
 }
 
 // Adds metadata of multiple offline pages into a store and removes some.
 TEST_F(OfflinePageMetadataStoreTest, AddRemoveMultipleOfflinePages) {
   std::unique_ptr<OfflinePageMetadataStore> store(BuildStore());
 
   // Add an offline page.
   OfflinePageItem offline_page_1(GURL(kTestURL), 12345LL, kTestClientId1,
                                  base::FilePath(kFilePath), kFileSize);
+  base::FilePath file_path_2 =
+      base::FilePath(FILE_PATH_LITERAL("//other.page.com.mhtml"));
+  OfflinePageItem offline_page_2(GURL("https://other.page.com"), 5678LL,

[fgorski, 2016/11/18 00:13:04]

given line 710, does this compile?

[romax, 2016/11/18 20:50:49]

Done.

+                                 kTestClientId2, file_path_2, 12345,
+                                 base::Time::Now());
   store->AddOfflinePage(offline_page_1,
                         base::Bind(&OfflinePageMetadataStoreTest::AddCallback,
                                    base::Unretained(this)));
   PumpLoop();
   EXPECT_EQ(ADD, last_called_callback_);
   EXPECT_EQ(STATUS_TRUE, last_status_);
 
   ClearResults();
 
   // Add anther offline page.
@@ skipping 52 matching lines @@
   EXPECT_EQ(LOAD, last_called_callback_);
   EXPECT_EQ(STATUS_TRUE, last_status_);
   ASSERT_EQ(1U, offline_pages_.size());
   EXPECT_EQ(offline_page_2.url, offline_pages_[0].url);
   EXPECT_EQ(offline_page_2.offline_id, offline_pages_[0].offline_id);
   EXPECT_EQ(offline_page_2.file_path, offline_pages_[0].file_path);
   EXPECT_EQ(offline_page_2.file_size, offline_pages_[0].file_size);
   EXPECT_EQ(offline_page_2.creation_time, offline_pages_[0].creation_time);
   EXPECT_EQ(offline_page_2.last_access_time,
             offline_pages_[0].last_access_time);
-  EXPECT_EQ(offline_page_2.expiration_time, offline_pages_[0].expiration_time);
   EXPECT_EQ(offline_page_2.access_count, offline_pages_[0].access_count);
   EXPECT_EQ(offline_page_2.client_id, offline_pages_[0].client_id);
 }
 
 // Tests updating offline page metadata from the store.
 TEST_F(OfflinePageMetadataStoreTest, UpdateOfflinePage) {
   std::unique_ptr<OfflinePageMetadataStore> store(BuildStore());
 
   // First, adds a fresh page.
   OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestClientId1,
@@ skipping 12 matching lines @@
   PumpLoop();
 
   EXPECT_EQ(LOAD, last_called_callback_);
   EXPECT_EQ(STATUS_TRUE, last_status_);
   ASSERT_EQ(1U, offline_pages_.size());
   EXPECT_EQ(offline_page, offline_pages_[0]);
 
   // Then update some data.
   offline_page.file_size = kFileSize + 1;
   offline_page.access_count++;
-  offline_page.expiration_time = base::Time::Now();
   offline_page.original_url = GURL("https://example.com/bar");
   std::vector<OfflinePageItem> items_to_update;
   items_to_update.push_back(offline_page);
   store->UpdateOfflinePages(
       items_to_update, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback,
                                   base::Unretained(this), UPDATE));
   PumpLoop();
   EXPECT_EQ(UPDATE, last_called_callback_);
   EXPECT_EQ(STATUS_TRUE, last_status_);
   ASSERT_TRUE(last_update_result() != nullptr);
@@ skipping 64 matching lines @@
                  base::Unretained(this)));
   PumpLoop();
 
   EXPECT_EQ(LOAD, last_called_callback_);
   EXPECT_EQ(STATUS_TRUE, last_status_);
   ASSERT_EQ(0U, offline_pages_.size());
 }
 
 }  // namespace
 }  // namespace offline_pages