From 344bb7e95cb1619d98c15e1456c0044365e6c233 Mon Sep 17 00:00:00 2001 From: bburns Date: Mon, 23 May 2016 11:39:07 -0700 Subject: [PATCH] delete the levelDB storage implementation. BUG=NONE Review-Url: https://codereview.chromium.org/1999443003 Cr-Commit-Position: refs/heads/master@{#395371} --- .../offline_pages/offline_page_bridge.cc | 1 - .../offline_page_model_factory.cc | 1 - components/offline_pages.gypi | 17 - components/offline_pages/BUILD.gn | 7 - components/offline_pages/DEPS | 2 - components/offline_pages/offline_page_item.cc | 1 - .../offline_page_metadata_store_impl.cc | 319 ------------------ .../offline_page_metadata_store_impl.h | 90 ----- ...fline_page_metadata_store_impl_unittest.cc | 179 +--------- .../offline_pages/offline_page_model.cc | 1 - components/offline_pages/proto/BUILD.gn | 12 - .../offline_pages/proto/offline_pages.proto | 65 ---- 12 files changed, 3 insertions(+), 692 deletions(-) delete mode 100644 components/offline_pages/offline_page_metadata_store_impl.cc delete mode 100644 components/offline_pages/offline_page_metadata_store_impl.h delete mode 100644 components/offline_pages/proto/BUILD.gn delete mode 100644 components/offline_pages/proto/offline_pages.proto diff --git a/chrome/browser/android/offline_pages/offline_page_bridge.cc b/chrome/browser/android/offline_pages/offline_page_bridge.cc index 6c229f4eb013f6..76a655d7464e0b 100644 --- a/chrome/browser/android/offline_pages/offline_page_bridge.cc +++ b/chrome/browser/android/offline_pages/offline_page_bridge.cc @@ -20,7 +20,6 @@ #include "components/offline_pages/offline_page_feature.h" #include "components/offline_pages/offline_page_item.h" #include "components/offline_pages/offline_page_model.h" -#include "components/offline_pages/proto/offline_pages.pb.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/web_contents.h" #include "jni/OfflinePageBridge_jni.h" diff --git a/chrome/browser/android/offline_pages/offline_page_model_factory.cc b/chrome/browser/android/offline_pages/offline_page_model_factory.cc index b9992863bb75c8..0ffcf9833ccbad 100644 --- a/chrome/browser/android/offline_pages/offline_page_model_factory.cc +++ b/chrome/browser/android/offline_pages/offline_page_model_factory.cc @@ -15,7 +15,6 @@ #include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/offline_pages/offline_page_metadata_store_sql.h" #include "components/offline_pages/offline_page_model.h" -#include "components/offline_pages/proto/offline_pages.pb.h" #include "content/public/browser/browser_thread.h" namespace offline_pages { diff --git a/components/offline_pages.gypi b/components/offline_pages.gypi index 304b967601d8c3..f0cce514a5af5b 100644 --- a/components/offline_pages.gypi +++ b/components/offline_pages.gypi @@ -16,10 +16,7 @@ '../net/net.gyp:net', '../url/url.gyp:url_lib', '../sql/sql.gyp:sql', - '../third_party/leveldatabase/leveldatabase.gyp:leveldatabase', - 'components.gyp:leveldb_proto', 'keyed_service_core', - 'offline_pages_proto', ], 'sources': [ 'offline_pages/archive_manager.cc', @@ -34,8 +31,6 @@ 'offline_pages/offline_page_item.h', 'offline_pages/offline_page_metadata_store.cc', 'offline_pages/offline_page_metadata_store.h', - 'offline_pages/offline_page_metadata_store_impl.cc', - 'offline_pages/offline_page_metadata_store_impl.h', 'offline_pages/offline_page_model.cc', 'offline_pages/offline_page_model.h', 'offline_pages/offline_page_storage_manager.cc', @@ -94,18 +89,6 @@ 'offline_pages/offline_page_test_store.cc', ], }, - { - # Protobuf compiler / generator for the offline page item protocol buffer. - # GN version: //components/offline_pages/proto - 'target_name': 'offline_pages_proto', - 'type': 'static_library', - 'sources': [ 'offline_pages/proto/offline_pages.proto', ], - 'variables': { - 'proto_in_dir': 'offline_pages/proto', - 'proto_out_dir': 'components/offline_pages/proto', - }, - 'includes': [ '../build/protoc.gypi', ], - }, ], 'conditions': [ ['OS == "android"', { diff --git a/components/offline_pages/BUILD.gn b/components/offline_pages/BUILD.gn index 5cd40e69113200..75464a016e6e22 100644 --- a/components/offline_pages/BUILD.gn +++ b/components/offline_pages/BUILD.gn @@ -19,8 +19,6 @@ static_library("offline_pages") { "offline_page_item.h", "offline_page_metadata_store.cc", "offline_page_metadata_store.h", - "offline_page_metadata_store_impl.cc", - "offline_page_metadata_store_impl.h", "offline_page_metadata_store_sql.cc", "offline_page_metadata_store_sql.h", "offline_page_model.cc", @@ -37,11 +35,8 @@ static_library("offline_pages") { "//base", "//components/bookmarks/browser", "//components/keyed_service/core", - "//components/leveldb_proto", - "//components/offline_pages/proto:offline_pages_proto", "//net", "//sql:sql", - "//third_party/leveldatabase", "//url", ] } @@ -93,8 +88,6 @@ source_set("unit_tests") { ":test_support", "//base", "//base/test:test_support", - "//components/leveldb_proto", - "//components/offline_pages/proto:offline_pages_proto", "//testing/gtest", "//url", ] diff --git a/components/offline_pages/DEPS b/components/offline_pages/DEPS index a30e9fe7d251e4..a2326888f13ab5 100644 --- a/components/offline_pages/DEPS +++ b/components/offline_pages/DEPS @@ -1,8 +1,6 @@ include_rules = [ "+components/keyed_service", - "+components/leveldb_proto", "+components/version_info", "+net", "+sql", - "+third_party/leveldatabase", ] diff --git a/components/offline_pages/offline_page_item.cc b/components/offline_pages/offline_page_item.cc index 98d514473d79f7..fb41a8a6f268ec 100644 --- a/components/offline_pages/offline_page_item.cc +++ b/components/offline_pages/offline_page_item.cc @@ -4,7 +4,6 @@ #include "components/offline_pages/offline_page_item.h" -#include "components/offline_pages/proto/offline_pages.pb.h" #include "net/base/filename_util.h" namespace offline_pages { diff --git a/components/offline_pages/offline_page_metadata_store_impl.cc b/components/offline_pages/offline_page_metadata_store_impl.cc deleted file mode 100644 index ead3fbe3e0369d..00000000000000 --- a/components/offline_pages/offline_page_metadata_store_impl.cc +++ /dev/null @@ -1,319 +0,0 @@ -// 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_impl.h" - -#include -#include -#include - -#include "base/bind.h" -#include "base/files/file_path.h" -#include "base/location.h" -#include "base/metrics/histogram_macros.h" -#include "base/sequenced_task_runner.h" -#include "base/strings/string_number_conversions.h" -#include "base/strings/utf_string_conversions.h" -#include "base/threading/thread_task_runner_handle.h" -#include "build/build_config.h" -#include "components/leveldb_proto/proto_database_impl.h" -#include "components/offline_pages/offline_page_item.h" -#include "components/offline_pages/offline_page_model.h" -#include "components/offline_pages/proto/offline_pages.pb.h" -#include "third_party/leveldatabase/env_chromium.h" -#include "third_party/leveldatabase/src/include/leveldb/db.h" -#include "url/gurl.h" - -using leveldb_proto::ProtoDatabase; - -namespace { -// Statistics are logged to UMA with this string as part of histogram name. They -// can all be found under LevelDB.*.OfflinePageMetadataStore. Changing this -// needs to synchronize with histograms.xml, AND will also become incompatible -// with older browsers still reporting the previous values. -const char kDatabaseUMAClientName[] = "OfflinePageMetadataStore"; -} - -namespace offline_pages { -namespace { - -void OfflinePageItemToEntry(const OfflinePageItem& item, - offline_pages::OfflinePageEntry* item_proto) { - DCHECK(item_proto); - item_proto->set_url(item.url.spec()); - item_proto->set_offline_id(item.offline_id); - item_proto->set_version(item.version); - std::string path_string; -#if defined(OS_POSIX) - path_string = item.file_path.value(); -#elif defined(OS_WIN) - path_string = base::WideToUTF8(item.file_path.value()); -#endif - item_proto->set_file_path(path_string); - item_proto->set_file_size(item.file_size); - item_proto->set_creation_time(item.creation_time.ToInternalValue()); - item_proto->set_last_access_time(item.last_access_time.ToInternalValue()); - item_proto->set_access_count(item.access_count); - item_proto->set_flags( - static_cast<::offline_pages::OfflinePageEntry_Flags>(item.flags)); - item_proto->set_client_id_name_space(item.client_id.name_space); - item_proto->set_client_id(item.client_id.id); -} - -bool OfflinePageItemFromEntry(const offline_pages::OfflinePageEntry& item_proto, - OfflinePageItem* item) { - DCHECK(item); - bool has_offline_id = - item_proto.has_offline_id() || item_proto.has_deprecated_bookmark_id(); - if (!item_proto.has_url() || !has_offline_id || !item_proto.has_version() || - !item_proto.has_file_path()) { - return false; - } - item->url = GURL(item_proto.url()); - item->offline_id = item_proto.offline_id(); - item->version = item_proto.version(); -#if defined(OS_POSIX) - item->file_path = base::FilePath(item_proto.file_path()); -#elif defined(OS_WIN) - item->file_path = base::FilePath(base::UTF8ToWide(item_proto.file_path())); -#endif - if (item_proto.has_file_size()) { - item->file_size = item_proto.file_size(); - } - if (item_proto.has_creation_time()) { - item->creation_time = - base::Time::FromInternalValue(item_proto.creation_time()); - } - if (item_proto.has_last_access_time()) { - item->last_access_time = - base::Time::FromInternalValue(item_proto.last_access_time()); - } - if (item_proto.has_access_count()) { - item->access_count = item_proto.access_count(); - } - if (item_proto.has_flags()) { - item->flags = static_cast(item_proto.flags()); - } - item->client_id.name_space = item_proto.client_id_name_space(); - item->client_id.id = item_proto.client_id(); - - return true; -} - -} // namespace - -OfflinePageMetadataStoreImpl::OfflinePageMetadataStoreImpl( - scoped_refptr background_task_runner, - const base::FilePath& database_dir) - : background_task_runner_(background_task_runner), - database_dir_(database_dir), - weak_ptr_factory_(this) { -} - -OfflinePageMetadataStoreImpl::~OfflinePageMetadataStoreImpl() { -} - -void OfflinePageMetadataStoreImpl::Load(const LoadCallback& callback) { - // First initialize the database. - database_.reset(new leveldb_proto::ProtoDatabaseImpl( - background_task_runner_)); - database_->Init(kDatabaseUMAClientName, database_dir_, - base::Bind(&OfflinePageMetadataStoreImpl::LoadContinuation, - weak_ptr_factory_.GetWeakPtr(), - callback)); -} - -void OfflinePageMetadataStoreImpl::LoadContinuation( - const LoadCallback& callback, - bool success) { - if (!success) { - NotifyLoadResult(callback, - STORE_INIT_FAILED, - std::vector()); - return; - } - - // After initialization, start to load the data. - database_->LoadEntries( - base::Bind(&OfflinePageMetadataStoreImpl::LoadDone, - weak_ptr_factory_.GetWeakPtr(), - callback)); -} - -void OfflinePageMetadataStoreImpl::LoadDone( - const LoadCallback& callback, - bool success, - std::unique_ptr> entries) { - DCHECK(entries); - - std::vector result; - std::unique_ptr::KeyEntryVector> - entries_to_update(new ProtoDatabase::KeyEntryVector()); - std::unique_ptr> keys_to_remove( - new std::vector()); - - LoadStatus status = LOAD_SUCCEEDED; - - if (success) { - for (auto& entry : *entries) { - OfflinePageItem item; - // We don't want to fail the entire database if one item is corrupt, - // so log error and keep going. - if (!OfflinePageItemFromEntry(entry, &item)) { - LOG(ERROR) << "failed to parse entry: " << entry.url() << " skipping."; - continue; - } - // Legacy storage. We upgrade them to the new offline_id keyed storage. - // TODO(bburns): Remove this eventually when we are sure everyone is - // upgraded. - if (!entry.has_offline_id()) { - item.offline_id = OfflinePageModel::GenerateOfflineId(); - - if (!entry.has_deprecated_bookmark_id()) { - LOG(ERROR) << "unexpected entry missing bookmark id"; - continue; - } - item.client_id.name_space = offline_pages::kBookmarkNamespace; - item.client_id.id = base::Int64ToString(entry.deprecated_bookmark_id()); - - OfflinePageEntry upgraded_entry; - OfflinePageItemToEntry(item, &upgraded_entry); - entries_to_update->push_back( - std::make_pair(base::Int64ToString(upgraded_entry.offline_id()), - upgraded_entry)); - // Remove the old entry that is indexed with deprecated id. - keys_to_remove->push_back(item.client_id.id); - } - result.push_back(item); - } - } else { - status = STORE_LOAD_FAILED; - } - - // If we couldn't load _anything_ report a parse failure. - if (entries->size() > 0 && result.size() == 0) { - status = DATA_PARSING_FAILED; - } - - if (status == LOAD_SUCCEEDED && entries_to_update->size() > 0) { - UpdateEntries(std::move(entries_to_update), std::move(keys_to_remove), - base::Bind(&OfflinePageMetadataStoreImpl::DatabaseUpdateDone, - weak_ptr_factory_.GetWeakPtr(), callback, status, - std::move(result))); - } else { - NotifyLoadResult(callback, status, result); - } -} - -void OfflinePageMetadataStoreImpl::NotifyLoadResult( - const LoadCallback& callback, - LoadStatus status, - const std::vector& result) { - UMA_HISTOGRAM_ENUMERATION("OfflinePages.LoadStatus", - status, - OfflinePageMetadataStore::LOAD_STATUS_COUNT); - if (status == LOAD_SUCCEEDED) { - UMA_HISTOGRAM_COUNTS("OfflinePages.SavedPageCount", result.size()); - } else { - DVLOG(1) << "Offline pages database loading failed: " << status; - database_.reset(); - } - callback.Run(status, result); -} - -void OfflinePageMetadataStoreImpl::AddOrUpdateOfflinePage( - const OfflinePageItem& offline_page_item, - const UpdateCallback& callback) { - std::unique_ptr::KeyEntryVector> - entries_to_save(new ProtoDatabase::KeyEntryVector()); - std::unique_ptr> keys_to_remove( - new std::vector()); - - OfflinePageEntry offline_page_proto; - OfflinePageItemToEntry(offline_page_item, &offline_page_proto); - - entries_to_save->push_back(std::make_pair( - base::Int64ToString(offline_page_item.offline_id), offline_page_proto)); - - UpdateEntries(std::move(entries_to_save), std::move(keys_to_remove), - callback); -} - -void OfflinePageMetadataStoreImpl::RemoveOfflinePages( - const std::vector& offline_ids, - const UpdateCallback& callback) { - std::unique_ptr::KeyEntryVector> - entries_to_save(new ProtoDatabase::KeyEntryVector()); - std::unique_ptr> keys_to_remove( - new std::vector()); - - for (int64_t id : offline_ids) - keys_to_remove->push_back(base::Int64ToString(id)); - - UpdateEntries(std::move(entries_to_save), std::move(keys_to_remove), - callback); -} - -void OfflinePageMetadataStoreImpl::UpdateEntries( - std::unique_ptr::KeyEntryVector> - entries_to_save, - std::unique_ptr> keys_to_remove, - const UpdateCallback& callback) { - if (!database_.get()) { - // Failing fast here, because DB is not initialized, and there is nothing - // that can be done about it. - // Callback is invoked through message loop to avoid improper retry and - // simplify testing. - DVLOG(1) << "Offline pages database not available in UpdateEntries."; - base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, - base::Bind(callback, false)); - return; - } - - database_->UpdateEntries( - std::move(entries_to_save), std::move(keys_to_remove), - base::Bind(&OfflinePageMetadataStoreImpl::UpdateDone, - weak_ptr_factory_.GetWeakPtr(), callback)); -} - -void OfflinePageMetadataStoreImpl::UpdateDone( - const OfflinePageMetadataStore::UpdateCallback& callback, - bool success) { - if (!success) { - // TODO(fgorski): Add UMA for this case. Consider rebuilding the store. - DVLOG(1) << "Offline pages database update failed."; - } - - callback.Run(success); -} - -void OfflinePageMetadataStoreImpl::Reset(const ResetCallback& callback) { - database_->Destroy( - base::Bind(&OfflinePageMetadataStoreImpl::ResetDone, - weak_ptr_factory_.GetWeakPtr(), - callback)); -} - -void OfflinePageMetadataStoreImpl::ResetDone( - const ResetCallback& callback, - bool success) { - database_.reset(); - weak_ptr_factory_.InvalidateWeakPtrs(); - callback.Run(success); -} - -void OfflinePageMetadataStoreImpl::DatabaseUpdateDone( - const OfflinePageMetadataStore::LoadCallback& cb, - LoadStatus status, - const std::vector& result, - bool success) { - // If the update failed, log and keep going. We'll try to - // update next time. - if (!success) { - LOG(ERROR) << "Failed to update database"; - } - NotifyLoadResult(cb, status, result); -} - -} // namespace offline_pages diff --git a/components/offline_pages/offline_page_metadata_store_impl.h b/components/offline_pages/offline_page_metadata_store_impl.h deleted file mode 100644 index 6b59794d1c0a5e..00000000000000 --- a/components/offline_pages/offline_page_metadata_store_impl.h +++ /dev/null @@ -1,90 +0,0 @@ -// 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. - -#ifndef COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_METADATA_STORE_IMPL_H_ -#define COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_METADATA_STORE_IMPL_H_ - -#include - -#include - -#include "base/files/file_path.h" -#include "base/gtest_prod_util.h" -#include "base/macros.h" -#include "base/memory/ref_counted.h" -#include "base/memory/weak_ptr.h" -#include "components/leveldb_proto/proto_database.h" -#include "components/offline_pages/offline_page_metadata_store.h" - -namespace base { -class SequencedTaskRunner; -} - -namespace offline_pages { - -class OfflinePageEntry; - -// Implements OfflinePageMetadataStore using leveldb_proto::ProtoDatabase -// component. Stores metadata of offline pages as serialized protobufs in a -// LevelDB key/value pairs. -// Underlying implementation guarantees that all of the method calls will be -// executed sequentially, and started operations will finish even after the -// store is already destroyed (callbacks will be called). -class OfflinePageMetadataStoreImpl : public OfflinePageMetadataStore { - public: - OfflinePageMetadataStoreImpl( - scoped_refptr background_task_runner, - const base::FilePath& database_dir); - ~OfflinePageMetadataStoreImpl() override; - - // OfflinePageMetadataStore implementation: - void Load(const LoadCallback& callback) override; - void AddOrUpdateOfflinePage(const OfflinePageItem& offline_page_record, - const UpdateCallback& callback) override; - void RemoveOfflinePages(const std::vector& offline_ids, - const UpdateCallback& callback) override; - void Reset(const ResetCallback& callback) override; - - private: - friend class OfflinePageMetadataStoreImplTest; - - void LoadContinuation(const LoadCallback& callback, bool success); - void LoadDone(const LoadCallback& callback, - bool success, - std::unique_ptr> entries); - void NotifyLoadResult(const LoadCallback& callback, - LoadStatus status, - const std::vector& result); - - // Implements the update. - void UpdateEntries(std::unique_ptr::KeyEntryVector> entries_to_save, - std::unique_ptr> keys_to_remove, - const UpdateCallback& callback); - - void UpdateDone(const OfflinePageMetadataStore::UpdateCallback& callback, - bool success); - - void ResetDone(const ResetCallback& callback, bool success); - - // Called when a database has to be migrated from old bookmark ids - // to new offline ids. - // TODO(bburns): Perhaps remove this eventually? - void DatabaseUpdateDone(const OfflinePageMetadataStore::LoadCallback& cb, - LoadStatus status, - const std::vector& result, - bool success); - - scoped_refptr background_task_runner_; - base::FilePath database_dir_; - std::unique_ptr> database_; - - base::WeakPtrFactory weak_ptr_factory_; - - DISALLOW_COPY_AND_ASSIGN(OfflinePageMetadataStoreImpl); -}; - -} // namespace offline_pages - -#endif // COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_METADATA_STORE_IMPL_H_ diff --git a/components/offline_pages/offline_page_metadata_store_impl_unittest.cc b/components/offline_pages/offline_page_metadata_store_impl_unittest.cc index 58118b4a45692f..a56851d0c19ec1 100644 --- a/components/offline_pages/offline_page_metadata_store_impl_unittest.cc +++ b/components/offline_pages/offline_page_metadata_store_impl_unittest.cc @@ -2,7 +2,7 @@ // 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_impl.h" +#include "components/offline_pages/offline_page_metadata_store.h" #include @@ -15,15 +15,11 @@ #include "base/strings/utf_string_conversions.h" #include "base/test/test_simple_task_runner.h" #include "base/threading/thread_task_runner_handle.h" -#include "components/leveldb_proto/proto_database_impl.h" #include "components/offline_pages/offline_page_item.h" #include "components/offline_pages/offline_page_metadata_store_sql.h" #include "components/offline_pages/offline_page_model.h" -#include "components/offline_pages/proto/offline_pages.pb.h" #include "testing/gtest/include/gtest/gtest.h" -using leveldb_proto::ProtoDatabaseImpl; - namespace offline_pages { namespace { @@ -41,15 +37,6 @@ class OfflinePageMetadataStoreFactory { virtual OfflinePageMetadataStore* BuildStore(const base::FilePath& file) = 0; }; -class OfflinePageMetadataStoreImplFactory - : public OfflinePageMetadataStoreFactory { - public: - OfflinePageMetadataStore* BuildStore(const base::FilePath& file) override { - return new OfflinePageMetadataStoreImpl(base::ThreadTaskRunnerHandle::Get(), - file); - } -}; - class OfflinePageMetadataStoreSQLFactory : public OfflinePageMetadataStoreFactory { public: @@ -73,7 +60,7 @@ class OfflinePageMetadataStoreTestBase : public testing::Test { PumpLoop(); } - std::unique_ptr BuildStore(); + std::unique_ptr BuildStore(); void PumpLoop(); void LoadCallback(OfflinePageMetadataStore::LoadStatus load_status, @@ -148,9 +135,7 @@ OfflinePageMetadataStoreTest::BuildStore() { return store; } -typedef testing::Types - MyTypes; +typedef testing::Types MyTypes; TYPED_TEST_CASE(OfflinePageMetadataStoreTest, MyTypes); // Loads empty store and makes sure that there are no offline pages stored in @@ -397,162 +382,4 @@ TYPED_TEST(OfflinePageMetadataStoreTest, UpdateOfflinePage) { } } // namespace - -class OfflinePageMetadataStoreImplTest - : public OfflinePageMetadataStoreTest { - public: - void UpdateStoreEntries( - OfflinePageMetadataStoreImpl* store, - std::unique_ptr::KeyEntryVector> entries_to_save); -}; - -void OfflinePageMetadataStoreImplTest::UpdateStoreEntries( - OfflinePageMetadataStoreImpl* store, - std::unique_ptr::KeyEntryVector> entries_to_save) { - std::unique_ptr> keys_to_remove( - new std::vector()); - store->UpdateEntries( - std::move(entries_to_save), std::move(keys_to_remove), - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); -} - -// Test that loading a store with a bad value still loads. -// Needs to be outside of the anonymous namespace in order for FRIEND_TEST -// to work. -TEST_F(OfflinePageMetadataStoreImplTest, LoadCorruptedStore) { - std::unique_ptr store(this->BuildStore()); - - // Write one ok page. - OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestClientId1, - base::FilePath(kFilePath), kFileSize); - store->AddOrUpdateOfflinePage( - offline_page, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); - this->PumpLoop(); - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - - // Manually write one broken page (no id) - std::unique_ptr< - leveldb_proto::ProtoDatabase::KeyEntryVector> - entries_to_save( - new leveldb_proto::ProtoDatabase::KeyEntryVector()); - - OfflinePageEntry offline_page_proto; - entries_to_save->push_back(std::make_pair("0", offline_page_proto)); - - UpdateStoreEntries((OfflinePageMetadataStoreImpl*)store.get(), - std::move(entries_to_save)); - this->PumpLoop(); - - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - - this->ClearResults(); - - // Close the store first to ensure file lock is removed. - store.reset(); - store = this->BuildStore(); - this->PumpLoop(); - - // One of the pages was busted, so only expect one page. - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - EXPECT_EQ(1U, this->offline_pages_.size()); - EXPECT_EQ(offline_page.url, this->offline_pages_[0].url); - EXPECT_EQ(offline_page.offline_id, this->offline_pages_[0].offline_id); - EXPECT_EQ(offline_page.version, this->offline_pages_[0].version); - EXPECT_EQ(offline_page.file_path, this->offline_pages_[0].file_path); - EXPECT_EQ(offline_page.file_size, this->offline_pages_[0].file_size); - EXPECT_EQ(offline_page.creation_time, this->offline_pages_[0].creation_time); - EXPECT_EQ(offline_page.last_access_time, - this->offline_pages_[0].last_access_time); - EXPECT_EQ(offline_page.access_count, this->offline_pages_[0].access_count); -} - -// Test that loading a store with nothing but bad values errors. -// Needs to be outside of the anonymous namespace in order for FRIEND_TEST -// to work. -TEST_F(OfflinePageMetadataStoreImplTest, LoadTotallyCorruptedStore) { - std::unique_ptr store(this->BuildStore()); - - // Manually write two broken pages (no id) - std::unique_ptr< - leveldb_proto::ProtoDatabase::KeyEntryVector> - entries_to_save( - new leveldb_proto::ProtoDatabase::KeyEntryVector()); - - OfflinePageEntry offline_page_proto; - entries_to_save->push_back(std::make_pair("0", offline_page_proto)); - entries_to_save->push_back(std::make_pair("1", offline_page_proto)); - - UpdateStoreEntries((OfflinePageMetadataStoreImpl*)store.get(), - std::move(entries_to_save)); - ; - this->PumpLoop(); - - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - - this->ClearResults(); - - // Close the store first to ensure file lock is removed. - store.reset(); - store = this->BuildStore(); - this->PumpLoop(); - - // One of the pages was busted, so only expect one page. - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_FALSE, this->last_status_); -} - -TEST_F(OfflinePageMetadataStoreImplTest, UpgradeStoreFromBookmarkIdToClientId) { - std::unique_ptr store(this->BuildStore()); - - // Manually write a page referring to legacy bookmark id. - std::unique_ptr< - leveldb_proto::ProtoDatabase::KeyEntryVector> - entries_to_save( - new leveldb_proto::ProtoDatabase::KeyEntryVector()); - - OfflinePageEntry offline_page_proto; - offline_page_proto.set_deprecated_bookmark_id(1LL); - offline_page_proto.set_version(1); - offline_page_proto.set_url(kTestURL); - offline_page_proto.set_file_path("/foo/bar"); - entries_to_save->push_back(std::make_pair("1", offline_page_proto)); - - UpdateStoreEntries((OfflinePageMetadataStoreImpl*)store.get(), - std::move(entries_to_save)); - PumpLoop(); - - EXPECT_EQ(ADD, last_called_callback_); - EXPECT_EQ(STATUS_TRUE, last_status_); - - ClearResults(); - - // Close the store first to ensure file lock is removed. - store.reset(); - store = BuildStore(); - PumpLoop(); - - // The page should be upgraded with new Client ID format. - EXPECT_EQ(LOAD, last_called_callback_); - EXPECT_EQ(STATUS_TRUE, last_status_); - EXPECT_EQ(1U, offline_pages_.size()); - EXPECT_TRUE(offline_pages_[0].offline_id != 0); - EXPECT_EQ(offline_pages::kBookmarkNamespace, - offline_pages_[0].client_id.name_space); - EXPECT_EQ(base::Int64ToString(offline_page_proto.deprecated_bookmark_id()), - offline_pages_[0].client_id.id); - EXPECT_EQ(GURL(kTestURL), offline_pages_[0].url); - EXPECT_EQ(offline_page_proto.version(), offline_pages_[0].version); - EXPECT_EQ(offline_page_proto.file_path(), - offline_pages_[0].file_path.MaybeAsASCII()); -} - } // namespace offline_pages diff --git a/components/offline_pages/offline_page_model.cc b/components/offline_pages/offline_page_model.cc index 1d7a60c664dc88..d6ad2c0731c2c2 100644 --- a/components/offline_pages/offline_page_model.cc +++ b/components/offline_pages/offline_page_model.cc @@ -22,7 +22,6 @@ #include "components/offline_pages/client_policy_controller.h" #include "components/offline_pages/offline_page_item.h" #include "components/offline_pages/offline_page_storage_manager.h" -#include "components/offline_pages/proto/offline_pages.pb.h" #include "url/gurl.h" using ArchiverResult = offline_pages::OfflinePageArchiver::ArchiverResult; diff --git a/components/offline_pages/proto/BUILD.gn b/components/offline_pages/proto/BUILD.gn deleted file mode 100644 index 7cfb1a6f4740b8..00000000000000 --- a/components/offline_pages/proto/BUILD.gn +++ /dev/null @@ -1,12 +0,0 @@ -# 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. - -import("//third_party/protobuf/proto_library.gni") - -# GYP version: components/offline_pages.gypi:offline_pages_proto -proto_library("offline_pages_proto") { - sources = [ - "offline_pages.proto", - ] -} diff --git a/components/offline_pages/proto/offline_pages.proto b/components/offline_pages/proto/offline_pages.proto deleted file mode 100644 index b764edc9c90756..00000000000000 --- a/components/offline_pages/proto/offline_pages.proto +++ /dev/null @@ -1,65 +0,0 @@ -// 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. -// -// Offline page item protocol for storage and exchanging of offline page -// metadata. - -syntax = "proto2"; - -option optimize_for = LITE_RUNTIME; -option retain_unknown_fields = true; - -package offline_pages; - -message OfflinePageEntry { - // URL of the offline page. - optional string url = 1; - - // Bookmark ID of the offline page, this is deprecated in favor of the - // more generic ClientID below. No new pages should have this field. - optional int64 deprecated_bookmark_id = 2 [default = -1]; - - // Offline ID of the page. Every page entry has a unique offline_id. - optional int64 offline_id = 10; - - // Version of the offline page metadata. - optional int32 version = 3; - - // Path to the offline archive. - optional string file_path = 4; - - // Size of the offline archive. - optional int64 file_size = 5; - - // Creation time of the offline archive. - optional int64 creation_time = 6; - - // Last access time of the offline archive. - optional int64 last_access_time = 7; - - // Number of times that the offline archive has been accessed. - optional int32 access_count = 8; - - // Flags about the state and behavior of the offline page. - enum Flags { - // No flag is set. - NONE = 0; - // Indicates that the page is marked for deletion. The real deletion will - // occur soon, after which the undo will not be possible. - MARKED_FOR_DELETION = 1; - }; - - // Flags for the offline page. - optional Flags flags = 9; - - // Information about this offline page in the namespace/id of the client that - // requested that it be saved. Useful for reverse lookups. - - // Namespace (e.g. client name) to separate the (possibly identical) ids - // from two different clients. - optional string client_id_name_space = 11; - // Identifier that has meaning for clients of the offline page API. - // This is provided by consumers of the offline pages API and is opaque to us. - optional string client_id = 12; -}