Commit 5e191c62 authored by Marc Treib's avatar Marc Treib Committed by Commit Bot

Remove dead code in DomDistillerModel

After https://crrev.com/c/1831873, a lot of code in the model is unused.

Bug: 1007942
Change-Id: Ifc57c83d8cfd828836c704584c4c27c40b098565
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1835618
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703670}
parent baa7b585
...@@ -89,47 +89,6 @@ std::vector<ArticleEntry> DomDistillerModel::GetEntries() const { ...@@ -89,47 +89,6 @@ std::vector<ArticleEntry> DomDistillerModel::GetEntries() const {
return entries_list; return entries_list;
} }
void DomDistillerModel::CalculateChangesForMerge(
const SyncDataList& data,
SyncChangeList* changes_to_apply,
SyncChangeList* changes_missing) {
typedef std::unordered_set<std::string> StringSet;
StringSet entries_to_change;
for (auto it = data.begin(); it != data.end(); ++it) {
std::string entry_id = GetEntryIdFromSyncData(*it);
std::pair<StringSet::iterator, bool> insert_result =
entries_to_change.insert(entry_id);
DCHECK(insert_result.second);
SyncChange::SyncChangeType change_type = SyncChange::ACTION_ADD;
if (GetEntryById(entry_id, nullptr)) {
change_type = SyncChange::ACTION_UPDATE;
}
changes_to_apply->push_back(SyncChange(FROM_HERE, change_type, *it));
}
for (EntryMap::const_iterator it = entries_.begin(); it != entries_.end();
++it) {
if (entries_to_change.find(it->second.entry_id()) ==
entries_to_change.end()) {
changes_missing->push_back(SyncChange(FROM_HERE, SyncChange::ACTION_ADD,
CreateLocalData(it->second)));
}
}
}
void DomDistillerModel::ApplyChangesToModel(const SyncChangeList& changes,
SyncChangeList* changes_applied,
SyncChangeList* changes_missing) {
DCHECK(changes_applied);
DCHECK(changes_missing);
for (auto it = changes.begin(); it != changes.end(); ++it) {
ApplyChangeToModel(*it, changes_applied, changes_missing);
}
}
void DomDistillerModel::AddEntry(const ArticleEntry& entry) { void DomDistillerModel::AddEntry(const ArticleEntry& entry) {
const std::string& entry_id = entry.entry_id(); const std::string& entry_id = entry.entry_id();
KeyType key = next_key_++; KeyType key = next_key_++;
...@@ -141,56 +100,4 @@ void DomDistillerModel::AddEntry(const ArticleEntry& entry) { ...@@ -141,56 +100,4 @@ void DomDistillerModel::AddEntry(const ArticleEntry& entry) {
} }
} }
void DomDistillerModel::RemoveEntry(const ArticleEntry& entry) {
const std::string& entry_id = entry.entry_id();
KeyType key = 0;
bool success = GetKeyById(entry_id, &key);
DCHECK(success);
entries_.erase(key);
entry_id_to_key_map_.erase(entry_id);
for (int i = 0; i < entry.pages_size(); ++i) {
url_to_key_map_.erase(entry.pages(i).url());
}
}
void DomDistillerModel::ApplyChangeToModel(const SyncChange& change,
SyncChangeList* changes_applied,
SyncChangeList* changes_missing) {
DCHECK(change.IsValid());
DCHECK(changes_applied);
DCHECK(changes_missing);
const std::string& entry_id = GetEntryIdFromSyncData(change.sync_data());
if (change.change_type() == SyncChange::ACTION_DELETE) {
ArticleEntry current_entry;
if (GetEntryById(entry_id, &current_entry)) {
RemoveEntry(current_entry);
changes_applied->push_back(SyncChange(
change.location(), SyncChange::ACTION_DELETE, change.sync_data()));
}
// If we couldn't find in sync db, we were deleting anyway so swallow the
// error.
return;
}
ArticleEntry entry = GetEntryFromChange(change);
ArticleEntry current_entry;
if (!GetEntryById(entry_id, &current_entry)) {
AddEntry(entry);
changes_applied->push_back(SyncChange(
change.location(), SyncChange::ACTION_ADD, change.sync_data()));
} else {
if (!AreEntriesEqual(current_entry, entry)) {
// Currently, conflicts are simply resolved by accepting the last one to
// arrive.
RemoveEntry(current_entry);
AddEntry(entry);
changes_applied->push_back(SyncChange(
change.location(), SyncChange::ACTION_UPDATE, change.sync_data()));
}
}
}
} // namespace dom_distiller } // namespace dom_distiller
...@@ -40,30 +40,12 @@ class DomDistillerModel { ...@@ -40,30 +40,12 @@ class DomDistillerModel {
std::vector<ArticleEntry> GetEntries() const; std::vector<ArticleEntry> GetEntries() const;
size_t GetNumEntries() const; size_t GetNumEntries() const;
// Convert a SyncDataList to a SyncChangeList of add or update changes based
// on the state of the model. Also calculate the entries missing from the
// SyncDataList.
void CalculateChangesForMerge(const syncer::SyncDataList& data,
syncer::SyncChangeList* changes_to_apply,
syncer::SyncChangeList* changes_missing);
// Applies the change list to the model, appending the actual changes made to
// the model to |changes_applied|. If conflict resolution does not apply the
// requested change, then adds the "diff" between what was requested and what
// was actually applied to |changes_missing|.
// Note: Currently conflicts are resolved by just applying the requested
// change. This means nothing will be added to |changes_missing|.
void ApplyChangesToModel(const syncer::SyncChangeList& change_list,
syncer::SyncChangeList* changes_applied,
syncer::SyncChangeList* changes_missing);
private: private:
typedef int32_t KeyType; typedef int32_t KeyType;
typedef std::unordered_map<KeyType, ArticleEntry> EntryMap; typedef std::unordered_map<KeyType, ArticleEntry> EntryMap;
typedef std::unordered_map<std::string, KeyType> StringToKeyMap; typedef std::unordered_map<std::string, KeyType> StringToKeyMap;
void AddEntry(const ArticleEntry& entry); void AddEntry(const ArticleEntry& entry);
void RemoveEntry(const ArticleEntry& entry);
// Lookup an entry's key by ID or URL. Returns whether a corresponding key was // Lookup an entry's key by ID or URL. Returns whether a corresponding key was
// found. On success, if |key| is not null, it will contain the entry. // found. On success, if |key| is not null, it will contain the entry.
...@@ -74,10 +56,6 @@ class DomDistillerModel { ...@@ -74,10 +56,6 @@ class DomDistillerModel {
// to an entry in |entries_|. // to an entry in |entries_|.
void GetEntryByKey(KeyType key, ArticleEntry* entry) const; void GetEntryByKey(KeyType key, ArticleEntry* entry) const;
void ApplyChangeToModel(const syncer::SyncChange& change,
syncer::SyncChangeList* changes_applied,
syncer::SyncChangeList* changes_missing);
KeyType next_key_; KeyType next_key_;
EntryMap entries_; EntryMap entries_;
StringToKeyMap url_to_key_map_; StringToKeyMap url_to_key_map_;
......
...@@ -71,61 +71,4 @@ TEST(DomDistillerModelTest, TestGetByUrl) { ...@@ -71,61 +71,4 @@ TEST(DomDistillerModelTest, TestGetByUrl) {
EXPECT_FALSE(model.GetEntryByUrl(GURL("http://example.com/foo"), nullptr)); EXPECT_FALSE(model.GetEntryByUrl(GURL("http://example.com/foo"), nullptr));
} }
// This test ensures that the model handles the case where an URL maps to
// multiple entries. In that case, the model is allowed to have an inconsistent
// url-to-entry mapping, but it should not fail in other ways (i.e. id-to-entry
// should be correct, shouldn't crash).
TEST(DomDistillerModelTest, TestUrlToMultipleEntries) {
ArticleEntry entry1;
entry1.set_entry_id("id1");
entry1.set_title("title1");
ArticleEntryPage* page1 = entry1.add_pages();
page1->set_url("http://example.com/1");
ArticleEntryPage* page2 = entry1.add_pages();
page2->set_url("http://example.com/2");
ArticleEntry entry2;
entry2.set_entry_id("id2");
entry2.set_title("title1");
ArticleEntryPage* page3 = entry2.add_pages();
page3->set_url("http://example.com/1");
std::vector<ArticleEntry> initial_model;
initial_model.push_back(entry1);
initial_model.push_back(entry2);
DomDistillerModel model(initial_model);
EXPECT_TRUE(model.GetEntryByUrl(GURL(page1->url()), nullptr));
EXPECT_TRUE(model.GetEntryByUrl(GURL(page2->url()), nullptr));
EXPECT_TRUE(model.GetEntryByUrl(GURL(page3->url()), nullptr));
ArticleEntry found_entry;
EXPECT_TRUE(model.GetEntryById(entry1.entry_id(), &found_entry));
ASSERT_TRUE(IsEntryValid(found_entry));
EXPECT_TRUE(AreEntriesEqual(entry1, found_entry));
EXPECT_TRUE(model.GetEntryById(entry2.entry_id(), &found_entry));
ASSERT_TRUE(IsEntryValid(found_entry));
EXPECT_TRUE(AreEntriesEqual(entry2, found_entry));
syncer::SyncChangeList changes_to_apply;
syncer::SyncChangeList changes_applied;
syncer::SyncChangeList changes_missing;
entry2.mutable_pages(0)->set_url("http://example.com/foo1");
changes_to_apply.push_back(syncer::SyncChange(
FROM_HERE, syncer::SyncChange::ACTION_UPDATE, CreateLocalData(entry2)));
model.ApplyChangesToModel(changes_to_apply, &changes_applied,
&changes_missing);
EXPECT_TRUE(model.GetEntryById(entry1.entry_id(), &found_entry));
ASSERT_TRUE(IsEntryValid(found_entry));
EXPECT_TRUE(AreEntriesEqual(entry1, found_entry));
EXPECT_TRUE(model.GetEntryById(entry2.entry_id(), &found_entry));
ASSERT_TRUE(IsEntryValid(found_entry));
EXPECT_TRUE(AreEntriesEqual(entry2, found_entry));
}
} // namespace dom_distiller } // namespace dom_distiller
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