Delete unused methods of DomDistillerService and DomDistillerStore

Bug: 1007942
Change-Id: Ibf1dd753fbd6a8922b0c909a8657728eab6af2c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831901Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701469}

chrome/browser/dom_distiller/lazy_dom_distiller_service.cc

@@ -50,22 +50,6 @@ std::string LazyDomDistillerService::GetUrlForEntry(
   return instance()->GetUrlForEntry(entry_id);
 }
 
-const std::string LazyDomDistillerService::AddToList(
-    const GURL& url,
-    std::unique_ptr<DistillerPage> distiller_page,
-    const ArticleAvailableCallback& article_cb) {
-  return instance()->AddToList(url, std::move(distiller_page), article_cb);
-}
-
-std::vector<ArticleEntry> LazyDomDistillerService::GetEntries() const {
-  return instance()->GetEntries();
-}
-
-std::unique_ptr<ArticleEntry> LazyDomDistillerService::RemoveEntry(
-    const std::string& entry_id) {
-  return instance()->RemoveEntry(entry_id);
-}
-
 std::unique_ptr<ViewerHandle> LazyDomDistillerService::ViewEntry(
     ViewRequestDelegate* delegate,
     std::unique_ptr<DistillerPage> distiller_page,

chrome/browser/dom_distiller/lazy_dom_distiller_service.h

@@ -36,15 +36,8 @@ class LazyDomDistillerService : public DomDistillerServiceInterface,
 
  public:
   // DomDistillerServiceInterface implementation:
-  const std::string AddToList(
-      const GURL& url,
-      std::unique_ptr<DistillerPage> distiller_page,
-      const ArticleAvailableCallback& article_cb) override;
   bool HasEntry(const std::string& entry_id) override;
   std::string GetUrlForEntry(const std::string& entry_id) override;
-  std::vector<ArticleEntry> GetEntries() const override;
-  std::unique_ptr<ArticleEntry> RemoveEntry(
-      const std::string& entry_id) override;
   std::unique_ptr<ViewerHandle> ViewEntry(
       ViewRequestDelegate* delegate,
       std::unique_ptr<DistillerPage> distiller_page,

components/dom_distiller/core/dom_distiller_service.cc

@@ -32,14 +32,6 @@ ArticleEntry CreateSkeletonEntryForUrl(const GURL& url) {
   return skeleton;
 }
 
-void RunArticleAvailableCallback(
-    const DomDistillerService::ArticleAvailableCallback& article_cb,
-    const ArticleEntry& entry,
-    const DistilledArticleProto* article_proto,
-    bool distillation_succeeded) {
-  article_cb.Run(distillation_succeeded);
-}
-
 }  // namespace
 
 DomDistillerService::DomDistillerService(
@@ -67,47 +59,6 @@ DomDistillerService::CreateDefaultDistillerPageWithHandle(
       std::move(handle));
 }
 
-const std::string DomDistillerService::AddToList(
-    const GURL& url,
-    std::unique_ptr<DistillerPage> distiller_page,
-    const ArticleAvailableCallback& article_cb) {
-  ArticleEntry entry;
-  const bool is_already_added = store_ && store_->GetEntryByUrl(url, &entry);
-
-  TaskTracker* task_tracker = nullptr;
-  if (is_already_added) {
-    task_tracker = GetTaskTrackerForEntry(entry);
-    if (task_tracker == nullptr) {
-      // Entry is in the store but there is no task tracker. This could
-      // happen when distillation has already completed. For now just return
-      // true.
-      // TODO(shashishekhar): Change this to check if article is available,
-      // An article may not be available for a variety of reasons, e.g.
-      // distillation failure or blobs not available locally.
-      base::ThreadTaskRunnerHandle::Get()->PostTask(
-          FROM_HERE, base::BindOnce(article_cb, true));
-      return entry.entry_id();
-    }
-  } else {
-    GetOrCreateTaskTrackerForUrl(url, &task_tracker);
-  }
-
-  if (!article_cb.is_null()) {
-    task_tracker->AddSaveCallback(
-        base::Bind(&RunArticleAvailableCallback, article_cb));
-  }
-
-  if (!is_already_added) {
-    task_tracker->AddSaveCallback(base::Bind(
-        &DomDistillerService::AddDistilledPageToList, base::Unretained(this)));
-    task_tracker->StartDistiller(distiller_factory_.get(),
-                                 std::move(distiller_page));
-    task_tracker->StartBlobFetcher();
-  }
-
-  return task_tracker->GetEntryId();
-}
-
 bool DomDistillerService::HasEntry(const std::string& entry_id) {
   return store_ && store_->GetEntryById(entry_id, nullptr);
 }
@@ -120,32 +71,6 @@ std::string DomDistillerService::GetUrlForEntry(const std::string& entry_id) {
   return "";
 }
 
-std::vector<ArticleEntry> DomDistillerService::GetEntries() const {
-  if (!store_) {
-    return std::vector<ArticleEntry>();
-  }
-  return store_->GetEntries();
-}
-
-std::unique_ptr<ArticleEntry> DomDistillerService::RemoveEntry(
-    const std::string& entry_id) {
-  std::unique_ptr<ArticleEntry> entry(new ArticleEntry);
-  entry->set_entry_id(entry_id);
-  TaskTracker* task_tracker = GetTaskTrackerForEntry(*entry);
-  if (task_tracker != nullptr) {
-    task_tracker->CancelSaveCallbacks();
-  }
-
-  if (!store_ || !store_->GetEntryById(entry_id, entry.get())) {
-    return std::unique_ptr<ArticleEntry>();
-  }
-
-  if (store_->RemoveEntry(*entry)) {
-    return entry;
-  }
-  return std::unique_ptr<ArticleEntry>();
-}
-
 std::unique_ptr<ViewerHandle> DomDistillerService::ViewEntry(
     ViewRequestDelegate* delegate,
     std::unique_ptr<DistillerPage> distiller_page,
@@ -259,19 +184,6 @@ void DomDistillerService::CancelTask(TaskTracker* task) {
   }
 }
 
-void DomDistillerService::AddDistilledPageToList(
-    const ArticleEntry& entry,
-    const DistilledArticleProto* article_proto,
-    bool distillation_succeeded) {
-  DCHECK(IsEntryValid(entry));
-  if (store_ && distillation_succeeded) {
-    DCHECK(article_proto);
-    DCHECK_GT(article_proto->pages_size(), 0);
-    store_->AddEntry(entry);
-    DCHECK_EQ(article_proto->pages_size(), entry.pages_size());
-  }
-}
-
 DistilledPagePrefs* DomDistillerService::GetDistilledPagePrefs() {
   return distilled_page_prefs_.get();
 }

components/dom_distiller/core/dom_distiller_service.h

@@ -11,7 +11,6 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
 #include "components/dom_distiller/core/article_entry.h"
 #include "components/dom_distiller/core/distilled_page_prefs.h"
 #include "components/dom_distiller/core/distiller_page.h"
@@ -20,7 +19,6 @@ class GURL;
 
 namespace dom_distiller {
 
-class DistilledArticleProto;
 class DistilledContentStore;
 class DistillerFactory;
 class DistillerPageFactory;
@@ -37,17 +35,6 @@ class DomDistillerServiceInterface {
   typedef base::Callback<void(bool)> ArticleAvailableCallback;
   virtual ~DomDistillerServiceInterface() {}
 
-  // Distill the article at |url| and add the resulting entry to the DOM
-  // distiller list. |article_cb| is always invoked, and the bool argument to it
-  // represents whether the article is available offline.
-  // Use CreateDefaultDistillerPage() to create a default |distiller_page|.
-  // The provided |distiller_page| is only used if there is not already a
-  // distillation task in progress for the given |url|.
-  virtual const std::string AddToList(
-      const GURL& url,
-      std::unique_ptr<DistillerPage> distiller_page,
-      const ArticleAvailableCallback& article_cb) = 0;
-
   // Returns whether an article stored has the given entry id.
   virtual bool HasEntry(const std::string& entry_id) = 0;
 
@@ -56,13 +43,6 @@ class DomDistillerServiceInterface {
   // empty string if there is no entry associated with the given entry ID.
   virtual std::string GetUrlForEntry(const std::string& entry_id) = 0;
 
-  // Gets the full list of entries.
-  virtual std::vector<ArticleEntry> GetEntries() const = 0;
-
-  // Removes the specified entry from the dom distiller store.
-  virtual std::unique_ptr<ArticleEntry> RemoveEntry(
-      const std::string& entry_id) = 0;
-
   // Request to view an article by entry id. Returns a null pointer if no entry
   // with |entry_id| exists. The ViewerHandle should be destroyed before the
   // ViewRequestDelegate. The request will be cancelled when the handle is
@@ -113,15 +93,8 @@ class DomDistillerService : public DomDistillerServiceInterface {
   ~DomDistillerService() override;
 
   // DomDistillerServiceInterface implementation.
-  const std::string AddToList(
-      const GURL& url,
-      std::unique_ptr<DistillerPage> distiller_page,
-      const ArticleAvailableCallback& article_cb) override;
   bool HasEntry(const std::string& entry_id) override;
   std::string GetUrlForEntry(const std::string& entry_id) override;
-  std::vector<ArticleEntry> GetEntries() const override;
-  std::unique_ptr<ArticleEntry> RemoveEntry(
-      const std::string& entry_id) override;
   std::unique_ptr<ViewerHandle> ViewEntry(
       ViewRequestDelegate* delegate,
       std::unique_ptr<DistillerPage> distiller_page,
@@ -138,9 +111,6 @@ class DomDistillerService : public DomDistillerServiceInterface {
 
  private:
   void CancelTask(TaskTracker* task);
-  void AddDistilledPageToList(const ArticleEntry& entry,
-                              const DistilledArticleProto* article_proto,
-                              bool distillation_succeeded);
 
   TaskTracker* CreateTaskTracker(const ArticleEntry& entry);
 

components/dom_distiller/core/dom_distiller_store.cc

@@ -49,67 +49,10 @@ bool DomDistillerStore::GetEntryByUrl(const GURL& url, ArticleEntry* entry) {
   return model_.GetEntryByUrl(url, entry);
 }
 
-bool DomDistillerStore::AddEntry(const ArticleEntry& entry) {
-  return ChangeEntry(entry, SyncChange::ACTION_ADD);
-}
-
-bool DomDistillerStore::UpdateEntry(const ArticleEntry& entry) {
-  return ChangeEntry(entry, SyncChange::ACTION_UPDATE);
-}
-
-bool DomDistillerStore::RemoveEntry(const ArticleEntry& entry) {
-  return ChangeEntry(entry, SyncChange::ACTION_DELETE);
-}
-
-bool DomDistillerStore::ChangeEntry(const ArticleEntry& entry,
-                                    SyncChange::SyncChangeType changeType) {
-  if (!database_loaded_) {
-    return false;
-  }
-
-  bool hasEntry = model_.GetEntryById(entry.entry_id(), nullptr);
-  if (hasEntry) {
-    if (changeType == SyncChange::ACTION_ADD) {
-      DVLOG(1) << "Already have entry with id " << entry.entry_id() << ".";
-      return false;
-    }
-  } else if (changeType != SyncChange::ACTION_ADD) {
-    DVLOG(1) << "No entry with id " << entry.entry_id() << " found.";
-    return false;
-  }
-
-  SyncChangeList changes_to_apply;
-  changes_to_apply.push_back(
-      SyncChange(FROM_HERE, changeType, CreateLocalData(entry)));
-
-  SyncChangeList changes_applied;
-  SyncChangeList changes_missing;
-
-  ApplyChangesToModel(changes_to_apply, &changes_applied, &changes_missing);
-
-  if (changeType == SyncChange::ACTION_UPDATE && changes_applied.size() != 1) {
-    DVLOG(1) << "Failed to update entry with id " << entry.entry_id() << ".";
-    return false;
-  }
-
-  DCHECK_EQ(size_t(0), changes_missing.size());
-  DCHECK_EQ(size_t(1), changes_applied.size());
-
-  ApplyChangesToDatabase(changes_applied);
-
-  return true;
-}
-
 std::vector<ArticleEntry> DomDistillerStore::GetEntries() const {
   return model_.GetEntries();
 }
 
-void DomDistillerStore::ApplyChangesToModel(const SyncChangeList& changes,
-                                            SyncChangeList* changes_applied,
-                                            SyncChangeList* changes_missing) {
-  model_.ApplyChangesToModel(changes, changes_applied, changes_missing);
-}
-
 void DomDistillerStore::OnDatabaseInit(
     leveldb_proto::Enums::InitStatus status) {
   if (status != leveldb_proto::Enums::InitStatus::kOK) {
@@ -188,7 +131,8 @@ void DomDistillerStore::MergeDataWithModel(const SyncDataList& data,
 
   SyncChangeList changes_to_apply;
   model_.CalculateChangesForMerge(data, &changes_to_apply, changes_missing);
-  ApplyChangesToModel(changes_to_apply, changes_applied, changes_missing);
+  model_.ApplyChangesToModel(changes_to_apply, changes_applied,
+                             changes_missing);
 }
 
 }  // namespace dom_distiller

components/dom_distiller/core/dom_distiller_store.h

@@ -24,11 +24,6 @@ class DomDistillerStoreInterface {
  public:
   virtual ~DomDistillerStoreInterface() {}
 
-  virtual bool AddEntry(const ArticleEntry& entry) = 0;
-  // Returns false if |entry| is not present or |entry| was not updated.
-  virtual bool UpdateEntry(const ArticleEntry& entry) = 0;
-  virtual bool RemoveEntry(const ArticleEntry& entry) = 0;
-
   // Lookup an ArticleEntry by ID or URL. Returns whether a corresponding entry
   // was found. On success, if |entry| is not null, it will contain the entry.
   virtual bool GetEntryById(const std::string& entry_id,
@@ -70,10 +65,6 @@ class DomDistillerStore : public DomDistillerStoreInterface {
 
   ~DomDistillerStore() override;
 
-  bool AddEntry(const ArticleEntry& entry) override;
-  bool UpdateEntry(const ArticleEntry& entry) override;
-  bool RemoveEntry(const ArticleEntry& entry) override;
-
   bool GetEntryById(const std::string& entry_id, ArticleEntry* entry) override;
   bool GetEntryByUrl(const GURL& url, ArticleEntry* entry) override;
   std::vector<ArticleEntry> GetEntries() const override;
@@ -83,22 +74,12 @@ class DomDistillerStore : public DomDistillerStoreInterface {
   void OnDatabaseLoad(bool success, std::unique_ptr<EntryVector> entries);
   void OnDatabaseSave(bool success);
 
-  // Returns true if the change is successfully applied.
-  bool ChangeEntry(const ArticleEntry& entry,
-                   syncer::SyncChange::SyncChangeType changeType);
-
   void MergeDataWithModel(const syncer::SyncDataList& data,
                           syncer::SyncChangeList* changes_applied,
                           syncer::SyncChangeList* changes_missing);
 
   bool ApplyChangesToDatabase(const syncer::SyncChangeList& change_list);
 
-  // Applies the changes to |model_|. If the model returns an error, disables
-  // syncing and database changes and returns false.
-  void ApplyChangesToModel(const syncer::SyncChangeList& change_list,
-                           syncer::SyncChangeList* changes_applied,
-                           syncer::SyncChangeList* changes_missing);
-
   std::unique_ptr<leveldb_proto::ProtoDatabase<ArticleEntry>> database_;
   bool database_loaded_;
 

components/dom_distiller/core/dom_distiller_store_unittest.cc

@@ -173,59 +173,4 @@ TEST_F(DomDistillerStoreTest, TestDatabaseLoadMerge) {
   EXPECT_TRUE(AreEntryMapsEqual(db_model_, expected_model));
 }
 
-TEST_F(DomDistillerStoreTest, TestAddAndRemoveEntry) {
-  CreateStore();
-  fake_db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK);
-  fake_db_->LoadCallback(true);
-
-  EXPECT_TRUE(store_->GetEntries().empty());
-  EXPECT_TRUE(db_model_.empty());
-
-  store_->AddEntry(GetSampleEntry(0));
-
-  EntryMap expected_model;
-  AddEntry(GetSampleEntry(0), &expected_model);
-
-  EXPECT_TRUE(AreEntriesEqual(store_->GetEntries(), expected_model));
-  EXPECT_TRUE(AreEntryMapsEqual(db_model_, expected_model));
-
-  store_->RemoveEntry(GetSampleEntry(0));
-  expected_model.clear();
-
-  EXPECT_TRUE(AreEntriesEqual(store_->GetEntries(), expected_model));
-  EXPECT_TRUE(AreEntryMapsEqual(db_model_, expected_model));
-}
-
-TEST_F(DomDistillerStoreTest, TestAddAndUpdateEntry) {
-  CreateStore();
-  fake_db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK);
-  fake_db_->LoadCallback(true);
-
-  EXPECT_TRUE(store_->GetEntries().empty());
-  EXPECT_TRUE(db_model_.empty());
-
-  store_->AddEntry(GetSampleEntry(0));
-
-  EntryMap expected_model;
-  AddEntry(GetSampleEntry(0), &expected_model);
-
-  EXPECT_TRUE(AreEntriesEqual(store_->GetEntries(), expected_model));
-  EXPECT_TRUE(AreEntryMapsEqual(db_model_, expected_model));
-
-  EXPECT_FALSE(store_->UpdateEntry(GetSampleEntry(0)));
-
-  ArticleEntry updated_entry(GetSampleEntry(0));
-  updated_entry.set_title("updated title.");
-  EXPECT_TRUE(store_->UpdateEntry(updated_entry));
-  expected_model.clear();
-  AddEntry(updated_entry, &expected_model);
-
-  EXPECT_TRUE(AreEntriesEqual(store_->GetEntries(), expected_model));
-  EXPECT_TRUE(AreEntryMapsEqual(db_model_, expected_model));
-
-  store_->RemoveEntry(updated_entry);
-  EXPECT_FALSE(store_->UpdateEntry(updated_entry));
-  EXPECT_FALSE(store_->UpdateEntry(GetSampleEntry(0)));
-}
-
 }  // namespace dom_distiller

components/dom_distiller/core/viewer_unittest.cc

@@ -41,19 +41,8 @@ class TestDomDistillerService : public DomDistillerServiceInterface {
   TestDomDistillerService() {}
   ~TestDomDistillerService() override {}
 
-  MOCK_METHOD3(AddToList,
-               const std::string(const GURL&,
-                                 DistillerPage*,
-                                 const ArticleAvailableCallback&));
-  const std::string AddToList(
-      const GURL& url,
-      std::unique_ptr<DistillerPage> distiller_page,
-      const ArticleAvailableCallback& article_cb) override {
-    return AddToList(url, distiller_page.get(), article_cb);
-  }
   MOCK_METHOD1(HasEntry, bool(const std::string&));
   MOCK_METHOD1(GetUrlForEntry, std::string(const std::string&));
-  MOCK_CONST_METHOD0(GetEntries, std::vector<ArticleEntry>());
   MOCK_METHOD0(ViewUrlImpl, ViewerHandle*());
   std::unique_ptr<ViewerHandle> ViewUrl(
       ViewRequestDelegate*,
@@ -68,10 +57,6 @@ class TestDomDistillerService : public DomDistillerServiceInterface {
       const std::string&) override {
     return std::unique_ptr<ViewerHandle>(ViewEntryImpl());
   }
-  MOCK_METHOD0(RemoveEntryImpl, ArticleEntry*());
-  std::unique_ptr<ArticleEntry> RemoveEntry(const std::string&) override {
-    return std::unique_ptr<ArticleEntry>(RemoveEntryImpl());
-  }
   std::unique_ptr<DistillerPage> CreateDefaultDistillerPage(
       const gfx::Size& render_view_size) override {
     return std::unique_ptr<DistillerPage>();