App-list history cleanup.

This CL centralizes the dependency to content, so that other files
can run independently.

BUG=380875
R=xiyuan@chromium.org
TEST=build

Review URL: https://codereview.chromium.org/518293002

Cr-Commit-Position: refs/heads/master@{#293065}

chrome/browser/ui/app_list/search/history.cc

@@ -4,12 +4,10 @@
 
 #include "chrome/browser/ui/app_list/search/history.h"
 
-#include "base/files/file_path.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/app_list/search/history_data.h"
 #include "chrome/browser/ui/app_list/search/history_data_store.h"
-#include "content/public/browser/browser_context.h"
 #include "ui/app_list/search/tokenized_string.h"
 
 namespace app_list {
@@ -24,16 +22,12 @@ std::string NormalizeString(const std::string& utf8) {
 
 }  // namespace
 
-History::History(content::BrowserContext* context)
-    : browser_context_(context),
+History::History(scoped_refptr<HistoryDataStore> store)
+    : store_(store),
       data_loaded_(false) {
-  const char kStoreDataFileName[] = "App Launcher Search";
   const size_t kMaxQueryEntries = 1000;
   const size_t kMaxSecondaryQueries = 5;
 
-  const base::FilePath data_file =
-      browser_context_->GetPath().AppendASCII(kStoreDataFileName);
-  store_ = new HistoryDataStore(data_file);
   data_.reset(
       new HistoryData(store_.get(), kMaxQueryEntries, kMaxSecondaryQueries));
   data_->AddObserver(this);

chrome/browser/ui/app_list/search/history.h

@@ -15,10 +15,6 @@
 #include "chrome/browser/ui/app_list/search/history_types.h"
 #include "components/keyed_service/core/keyed_service.h"
 
-namespace content {
-class BrowserContext;
-}
-
 namespace app_list {
 
 class HistoryData;
@@ -36,7 +32,7 @@ class SearchHistoryTest;
 // have been launched before.
 class History : public KeyedService, public HistoryDataObserver {
  public:
-  explicit History(content::BrowserContext* context);
+  explicit History(scoped_refptr<HistoryDataStore> store);
   virtual ~History();
 
   // Returns true if the service is ready.
@@ -55,7 +51,6 @@ class History : public KeyedService, public HistoryDataObserver {
   // HistoryDataObserver overrides:
   virtual void OnHistoryDataLoadedFromStore() OVERRIDE;
 
-  content::BrowserContext* browser_context_;
   scoped_ptr<HistoryData> data_;
   scoped_refptr<HistoryDataStore> store_;
   bool data_loaded_;

chrome/browser/ui/app_list/search/history_data_store.cc

@@ -7,14 +7,8 @@
 #include "base/callback.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/json/json_string_value_serializer.h"
-#include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
-#include "base/task_runner_util.h"
-#include "base/threading/sequenced_worker_pool.h"
 #include "base/values.h"
-#include "content/public/browser/browser_thread.h"
-
-using content::BrowserThread;
 
 namespace app_list {
 
@@ -111,27 +105,44 @@ scoped_ptr<HistoryData::Associations> Parse(
 
 }  // namespace
 
-HistoryDataStore::HistoryDataStore(const base::FilePath& data_file)
-    : data_store_(new DictionaryDataStore(data_file)) {
-  base::DictionaryValue* dict = data_store_->cached_dict();
-  DCHECK(dict);
-  dict->SetString(kKeyVersion, kCurrentVersion);
-  dict->Set(kKeyAssociations, new base::DictionaryValue);
+HistoryDataStore::HistoryDataStore()
+    : cached_dict_(new base::DictionaryValue()) {
+  Init(cached_dict_.get());
+}
+
+HistoryDataStore::HistoryDataStore(
+    scoped_refptr<DictionaryDataStore> data_store)
+    : data_store_(data_store) {
+  Init(data_store_->cached_dict());
 }
 
 HistoryDataStore::~HistoryDataStore() {
 }
 
+void HistoryDataStore::Init(base::DictionaryValue* cached_dict) {
+  DCHECK(cached_dict);
+  cached_dict->SetString(kKeyVersion, kCurrentVersion);
+  cached_dict->Set(kKeyAssociations, new base::DictionaryValue);
+}
+
 void HistoryDataStore::Flush(
     const DictionaryDataStore::OnFlushedCallback& on_flushed) {
-  data_store_->Flush(on_flushed);
+  if (data_store_)
+    data_store_->Flush(on_flushed);
+  else
+    on_flushed.Run();
 }
 
 void HistoryDataStore::Load(
     const HistoryDataStore::OnLoadedCallback& on_loaded) {
-  data_store_->Load(base::Bind(&HistoryDataStore::OnDictionaryLoadedCallback,
-                               this,
-                               on_loaded));
+  if (data_store_) {
+    data_store_->Load(base::Bind(&HistoryDataStore::OnDictionaryLoadedCallback,
+                                 this,
+                                 on_loaded));
+  } else {
+    OnDictionaryLoadedCallback(
+        on_loaded, make_scoped_ptr(cached_dict_->DeepCopy()));
+  }
 }
 
 void HistoryDataStore::SetPrimary(const std::string& query,
@@ -139,7 +150,8 @@ void HistoryDataStore::SetPrimary(const std::string& query,
   base::DictionaryValue* entry_dict = GetEntryDict(query);
   entry_dict->SetWithoutPathExpansion(kKeyPrimary,
                                       new base::StringValue(result));
-  data_store_->ScheduleWrite();
+  if (data_store_)
+    data_store_->ScheduleWrite();
 }
 
 void HistoryDataStore::SetSecondary(
@@ -151,7 +163,8 @@ void HistoryDataStore::SetSecondary(
 
   base::DictionaryValue* entry_dict = GetEntryDict(query);
   entry_dict->SetWithoutPathExpansion(kKeySecondary, results_list.release());
-  data_store_->ScheduleWrite();
+  if (data_store_)
+    data_store_->ScheduleWrite();
 }
 
 void HistoryDataStore::SetUpdateTime(const std::string& query,
@@ -160,17 +173,20 @@ void HistoryDataStore::SetUpdateTime(const std::string& query,
   entry_dict->SetWithoutPathExpansion(kKeyUpdateTime,
                                       new base::StringValue(base::Int64ToString(
                                           update_time.ToInternalValue())));
-  data_store_->ScheduleWrite();
+  if (data_store_)
+    data_store_->ScheduleWrite();
 }
 
 void HistoryDataStore::Delete(const std::string& query) {
   base::DictionaryValue* assoc_dict = GetAssociationDict();
   assoc_dict->RemoveWithoutPathExpansion(query, NULL);
-  data_store_->ScheduleWrite();
+  if (data_store_)
+    data_store_->ScheduleWrite();
 }
 
 base::DictionaryValue* HistoryDataStore::GetAssociationDict() {
-  base::DictionaryValue* cached_dict = data_store_->cached_dict();
+  base::DictionaryValue* cached_dict =
+      cached_dict_ ? cached_dict_.get() : data_store_->cached_dict();
   DCHECK(cached_dict);
 
   base::DictionaryValue* assoc_dict = NULL;

chrome/browser/ui/app_list/search/history_data_store.h

@@ -10,8 +10,6 @@
 
 #include "base/basictypes.h"
 #include "base/callback_forward.h"
-#include "base/files/file_path.h"
-#include "base/files/important_file_writer.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/ui/app_list/search/common/dictionary_data_store.h"
@@ -34,7 +32,11 @@ class HistoryDataStore : public base::RefCountedThreadSafe<HistoryDataStore> {
   typedef base::Callback<void(scoped_ptr<HistoryData::Associations>)>
       OnLoadedCallback;
 
-  explicit HistoryDataStore(const base::FilePath& data_file);
+  // A data store with no storage backend.
+  HistoryDataStore();
+
+  // |data_store| stores the history into the file.
+  explicit HistoryDataStore(scoped_refptr<DictionaryDataStore> data_store);
 
   // Flushes pending writes. |on_flushed| is invoked when disk write is
   // finished.
@@ -58,6 +60,8 @@ class HistoryDataStore : public base::RefCountedThreadSafe<HistoryDataStore> {
 
   virtual ~HistoryDataStore();
 
+  void Init(base::DictionaryValue* cached_dict);
+
   // Gets the dictionary for "associations" key.
   base::DictionaryValue* GetAssociationDict();
 
@@ -67,6 +71,10 @@ class HistoryDataStore : public base::RefCountedThreadSafe<HistoryDataStore> {
   void OnDictionaryLoadedCallback(OnLoadedCallback callback,
                                   scoped_ptr<base::DictionaryValue> dict);
 
+  // |cached_dict_| and |data_store_| is mutually exclusive. |data_store_| is
+  // used if it's backed by a file storage, otherwise |cache_dict_| keeps
+  // on-memory data.
+  scoped_ptr<base::DictionaryValue> cached_dict_;
   scoped_refptr<DictionaryDataStore> data_store_;
 
   DISALLOW_COPY_AND_ASSIGN(HistoryDataStore);

chrome/browser/ui/app_list/search/history_data_store_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "chrome/browser/ui/app_list/search/common/dictionary_data_store.h"
 #include "chrome/browser/ui/app_list/search/history_data.h"
 #include "chrome/browser/ui/app_list/search/history_data_store.h"
 #include "content/public/test/test_browser_thread.h"
@@ -54,7 +55,8 @@ class HistoryDataStoreTest : public testing::Test {
 
   void OpenStore(const std::string& file_name) {
     data_file_ = temp_dir_.path().AppendASCII(file_name);
-    store_ = new HistoryDataStore(data_file_);
+    store_ = new HistoryDataStore(scoped_refptr<DictionaryDataStore>(
+        new DictionaryDataStore(data_file_)));
     Load();
   }
 

chrome/browser/ui/app_list/search/history_factory.cc

@@ -4,9 +4,13 @@
 
 #include "chrome/browser/ui/app_list/search/history_factory.h"
 
+#include "base/memory/ref_counted.h"
 #include "base/memory/singleton.h"
+#include "chrome/browser/ui/app_list/search/common/dictionary_data_store.h"
 #include "chrome/browser/ui/app_list/search/history.h"
+#include "chrome/browser/ui/app_list/search/history_data_store.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "content/public/browser/browser_context.h"
 
 namespace app_list {
 
@@ -31,7 +35,14 @@ HistoryFactory::~HistoryFactory() {}
 
 KeyedService* HistoryFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
-  return new History(context);
+  const char kStoreDataFileName[] = "App Launcher Search";
+  const base::FilePath data_file =
+      context->GetPath().AppendASCII(kStoreDataFileName);
+  scoped_refptr<DictionaryDataStore> dictionary_data_store(
+      new DictionaryDataStore(data_file));
+  scoped_refptr<HistoryDataStore> history_data_store(
+      new HistoryDataStore(dictionary_data_store));
+  return new History(history_data_store);
 }
 
 }  // namespace app_list

chrome/browser/ui/app_list/search/history_unittest.cc

@@ -9,10 +9,12 @@
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/platform_thread.h"
+#include "chrome/browser/ui/app_list/search/common/dictionary_data_store.h"
 #include "chrome/browser/ui/app_list/search/history.h"
 #include "chrome/browser/ui/app_list/search/history_data.h"
 #include "chrome/browser/ui/app_list/search/history_data_observer.h"
 #include "chrome/browser/ui/app_list/search/history_data_store.h"
+#include "chrome/browser/ui/app_list/search/history_factory.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -101,7 +103,13 @@ class SearchHistoryTest : public testing::Test {
   }
 
   void CreateHistory() {
-    history_.reset(new History(profile_.get()));
+    const char kStoreDataFileName[] = "app-launcher-test";
+    const base::FilePath data_file =
+        profile_->GetPath().AppendASCII(kStoreDataFileName);
+    scoped_refptr<DictionaryDataStore> dictionary_data_store(
+        new DictionaryDataStore(data_file));
+    history_.reset(new History(scoped_refptr<HistoryDataStore>(
+        new HistoryDataStore(dictionary_data_store))));
 
     // Replace |data_| with test params.
     history_->data_->RemoveObserver(history_.get());