Commit fc287111 authored by tfarina@chromium.org's avatar tfarina@chromium.org

Make HistoryDatabase own StarredURLDatabase instead of inheriting from it.

This switches HistoryDatabase to composition, so the clients doesn't have to
depend/include starred_url_database.h anymore.

NOTE: This was a TODO for sky@.

R=sky@chromium.org

Review URL: http://codereview.chromium.org/8550006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@111179 0039d316-1c4b-4281-b951-d872f2087c98
parent b808ec2c
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/rand_util.h" #include "base/rand_util.h"
#include "base/string_util.h" #include "base/string_util.h"
#include "chrome/browser/diagnostics/sqlite_diagnostics.h" #include "chrome/browser/diagnostics/sqlite_diagnostics.h"
#include "chrome/browser/history/starred_url_database.h"
#include "sql/transaction.h" #include "sql/transaction.h"
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
...@@ -261,7 +262,8 @@ sql::InitStatus HistoryDatabase::EnsureCurrentVersion( ...@@ -261,7 +262,8 @@ sql::InitStatus HistoryDatabase::EnsureCurrentVersion(
// Put migration code here // Put migration code here
if (cur_version == 15) { if (cur_version == 15) {
if (!MigrateBookmarksToFile(tmp_bookmarks_path) || StarredURLDatabase starred_url_database(&db_);
if (!starred_url_database.MigrateBookmarksToFile(tmp_bookmarks_path) ||
!DropStarredIDFromURLs()) { !DropStarredIDFromURLs()) {
LOG(WARNING) << "Unable to update history database to version 16."; LOG(WARNING) << "Unable to update history database to version 16.";
return sql::INIT_FAILURE; return sql::INIT_FAILURE;
......
...@@ -6,11 +6,12 @@ ...@@ -6,11 +6,12 @@
#define CHROME_BROWSER_HISTORY_HISTORY_DATABASE_H_ #define CHROME_BROWSER_HISTORY_HISTORY_DATABASE_H_
#pragma once #pragma once
#include "base/basictypes.h"
#include "base/compiler_specific.h"
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/history/download_database.h" #include "chrome/browser/history/download_database.h"
#include "chrome/browser/history/history_types.h" #include "chrome/browser/history/history_types.h"
#include "chrome/browser/history/starred_url_database.h"
#include "chrome/browser/history/url_database.h" #include "chrome/browser/history/url_database.h"
#include "chrome/browser/history/visit_database.h" #include "chrome/browser/history/visit_database.h"
#include "chrome/browser/history/visitsegment_database.h" #include "chrome/browser/history/visitsegment_database.h"
...@@ -30,10 +31,7 @@ namespace history { ...@@ -30,10 +31,7 @@ namespace history {
// as the storage interface. Logic for manipulating this storage layer should // as the storage interface. Logic for manipulating this storage layer should
// be in HistoryBackend.cc. // be in HistoryBackend.cc.
class HistoryDatabase : public DownloadDatabase, class HistoryDatabase : public DownloadDatabase,
// TODO(sky): See if we can nuke StarredURLDatabase and just create on the public URLDatabase,
// stack for migration. Then HistoryDatabase would directly descend from
// URLDatabase.
public StarredURLDatabase,
public VisitDatabase, public VisitDatabase,
public VisitSegmentDatabase { public VisitSegmentDatabase {
public: public:
...@@ -145,7 +143,7 @@ class HistoryDatabase : public DownloadDatabase, ...@@ -145,7 +143,7 @@ class HistoryDatabase : public DownloadDatabase,
private: private:
FRIEND_TEST_ALL_PREFIXES(IconMappingMigrationTest, TestIconMappingMigration); FRIEND_TEST_ALL_PREFIXES(IconMappingMigrationTest, TestIconMappingMigration);
// Implemented for URLDatabase. // Overridden from URLDatabase:
virtual sql::Connection& GetDB() OVERRIDE; virtual sql::Connection& GetDB() OVERRIDE;
// Migration ----------------------------------------------------------------- // Migration -----------------------------------------------------------------
......
...@@ -118,7 +118,8 @@ void ResetBookmarkNode(const history::StarredEntry& entry, ...@@ -118,7 +118,8 @@ void ResetBookmarkNode(const history::StarredEntry& entry,
// static // static
const int64 StarredURLDatabase::kBookmarkBarID = 1; const int64 StarredURLDatabase::kBookmarkBarID = 1;
StarredURLDatabase::StarredURLDatabase() { StarredURLDatabase::StarredURLDatabase(sql::Connection* db)
: db_(db) {
} }
StarredURLDatabase::~StarredURLDatabase() { StarredURLDatabase::~StarredURLDatabase() {
...@@ -166,6 +167,10 @@ bool StarredURLDatabase::GetAllStarredEntries( ...@@ -166,6 +167,10 @@ bool StarredURLDatabase::GetAllStarredEntries(
return true; return true;
} }
sql::Connection& StarredURLDatabase::GetDB() {
return *db_;
}
bool StarredURLDatabase::EnsureStarredIntegrity() { bool StarredURLDatabase::EnsureStarredIntegrity() {
std::set<StarredNode*> roots; std::set<StarredNode*> roots;
std::set<StarID> folders_with_duplicate_ids; std::set<StarID> folders_with_duplicate_ids;
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <set> #include <set>
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/compiler_specific.h"
#include "base/string16.h" #include "base/string16.h"
#include "chrome/browser/history/history_types.h" #include "chrome/browser/history/history_types.h"
#include "chrome/browser/history/url_database.h" #include "chrome/browser/history/url_database.h"
...@@ -16,10 +17,6 @@ ...@@ -16,10 +17,6 @@
class FilePath; class FilePath;
namespace sql {
class Connection;
}
namespace history { namespace history {
// Bookmarks were originally part of the url database, they have since been // Bookmarks were originally part of the url database, they have since been
...@@ -27,11 +24,12 @@ namespace history { ...@@ -27,11 +24,12 @@ namespace history {
// contains just enough to allow migration. // contains just enough to allow migration.
class StarredURLDatabase : public URLDatabase { class StarredURLDatabase : public URLDatabase {
public: public:
// Must call InitStarTable() AND any additional init functions provided by explicit StarredURLDatabase(sql::Connection* db);
// URLDatabase before using this class' functions.
StarredURLDatabase();
virtual ~StarredURLDatabase(); virtual ~StarredURLDatabase();
// Writes bookmarks to the specified file.
bool MigrateBookmarksToFile(const FilePath& path);
protected: protected:
friend class StarredURLDatabaseTest; friend class StarredURLDatabaseTest;
...@@ -39,11 +37,8 @@ class StarredURLDatabase : public URLDatabase { ...@@ -39,11 +37,8 @@ class StarredURLDatabase : public URLDatabase {
// This entry always exists. // This entry always exists.
static const int64 kBookmarkBarID; static const int64 kBookmarkBarID;
// Writes bookmarks to the specified file.
bool MigrateBookmarksToFile(const FilePath& path);
// Returns the database for the functions in this interface. // Returns the database for the functions in this interface.
virtual sql::Connection& GetDB() = 0; virtual sql::Connection& GetDB() OVERRIDE;
private: private:
// Used when checking integrity of starred table. // Used when checking integrity of starred table.
...@@ -178,6 +173,8 @@ class StarredURLDatabase : public URLDatabase { ...@@ -178,6 +173,8 @@ class StarredURLDatabase : public URLDatabase {
// BookmarkStorage will read from. // BookmarkStorage will read from.
bool MigrateBookmarksToFileImpl(const FilePath& path); bool MigrateBookmarksToFileImpl(const FilePath& path);
sql::Connection* db_;
DISALLOW_COPY_AND_ASSIGN(StarredURLDatabase); DISALLOW_COPY_AND_ASSIGN(StarredURLDatabase);
}; };
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include <vector> #include <vector>
#include "base/compiler_specific.h"
#include "base/file_util.h" #include "base/file_util.h"
#include "base/path_service.h" #include "base/path_service.h"
#include "base/scoped_temp_dir.h" #include "base/scoped_temp_dir.h"
...@@ -19,7 +20,8 @@ namespace history { ...@@ -19,7 +20,8 @@ namespace history {
class StarredURLDatabaseTest : public testing::Test, class StarredURLDatabaseTest : public testing::Test,
public StarredURLDatabase { public StarredURLDatabase {
public: public:
StarredURLDatabaseTest() { StarredURLDatabaseTest()
: StarredURLDatabase(&db_) {
} }
void AddPage(const GURL& url) { void AddPage(const GURL& url) {
...@@ -65,7 +67,7 @@ class StarredURLDatabaseTest : public testing::Test, ...@@ -65,7 +67,7 @@ class StarredURLDatabaseTest : public testing::Test,
private: private:
// Test setup. // Test setup.
void SetUp() { virtual void SetUp() OVERRIDE {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
db_file_ = temp_dir_.path().AppendASCII("VisitTest.db"); db_file_ = temp_dir_.path().AppendASCII("VisitTest.db");
file_util::Delete(db_file_, false); file_util::Delete(db_file_, false);
...@@ -85,15 +87,10 @@ class StarredURLDatabaseTest : public testing::Test, ...@@ -85,15 +87,10 @@ class StarredURLDatabaseTest : public testing::Test,
CreateMainURLIndex(); CreateMainURLIndex();
EnsureStarredIntegrity(); EnsureStarredIntegrity();
} }
void TearDown() { virtual void TearDown() OVERRIDE {
db_.Close(); db_.Close();
} }
// Provided for URL/StarredURLDatabase.
virtual sql::Connection& GetDB() {
return db_;
}
ScopedTempDir temp_dir_; ScopedTempDir temp_dir_;
FilePath db_file_; FilePath db_file_;
sql::Connection db_; sql::Connection db_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment