summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormrossetti@chromium.org <mrossetti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-04-13 21:17:18 +0000
committermrossetti@chromium.org <mrossetti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-04-13 21:17:18 +0000
commitb04cc6f45cf50d34dad447f94b18930e7884ec77 (patch)
tree50f46bad7970ca531a1c4870eeecffcd74d5b307
parent3e3c452f5119bc837ebc98af5de8d5d75cd04ce6 (diff)
downloadchromium_src-b04cc6f45cf50d34dad447f94b18930e7884ec77.zip
chromium_src-b04cc6f45cf50d34dad447f94b18930e7884ec77.tar.gz
chromium_src-b04cc6f45cf50d34dad447f94b18930e7884ec77.tar.bz2
Single URL Expires Were Not Being Deleted.
Some notification clients expect URLs while others expect URLRows but in some cases the URLRows were not being provided. Eliminated the duplication of data structures by removing the set of URLs and relying only on the vector of URLRows. tim@ for OWNERS review of sync/... BUG=122740 TEST=Added unit test. TBR=tim@chromium.org Review URL: https://chromiumcodereview.appspot.com/10031028 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@132268 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/autocomplete/network_action_predictor.cc7
-rw-r--r--chrome/browser/autocomplete/network_action_predictor.h5
-rw-r--r--chrome/browser/autocomplete/network_action_predictor_unittest.cc10
-rw-r--r--chrome/browser/history/android/android_provider_backend.cc2
-rw-r--r--chrome/browser/history/android/android_provider_backend_unittest.cc15
-rw-r--r--chrome/browser/history/expire_history_backend.cc3
-rw-r--r--chrome/browser/history/expire_history_backend_unittest.cc9
-rw-r--r--chrome/browser/history/history.cc2
-rw-r--r--chrome/browser/history/history_extension_api.cc7
-rw-r--r--chrome/browser/history/history_notifications.h10
-rw-r--r--chrome/browser/history/history_types.h13
-rw-r--r--chrome/browser/history/in_memory_url_index_unittest.cc33
-rw-r--r--chrome/browser/history/shortcuts_backend.cc8
-rw-r--r--chrome/browser/history/top_sites.cc8
-rw-r--r--chrome/browser/sync/glue/typed_url_change_processor.cc7
-rw-r--r--chrome/browser/sync/profile_sync_service_typed_url_unittest.cc2
-rw-r--r--chrome/browser/ui/webui/history_ui.cc20
-rw-r--r--chrome/browser/visitedlink/visitedlink_master.cc20
-rw-r--r--chrome/browser/visitedlink/visitedlink_master.h5
-rw-r--r--chrome/browser/visitedlink/visitedlink_unittest.cc12
20 files changed, 120 insertions, 78 deletions
diff --git a/chrome/browser/autocomplete/network_action_predictor.cc b/chrome/browser/autocomplete/network_action_predictor.cc
index 7afc9f8..310200e 100644
--- a/chrome/browser/autocomplete/network_action_predictor.cc
+++ b/chrome/browser/autocomplete/network_action_predictor.cc
@@ -202,7 +202,7 @@ void NetworkActionPredictor::Observe(
if (urls_deleted_details->all_history)
DeleteAllRows();
else
- DeleteRowsWithURLs(urls_deleted_details->urls);
+ DeleteRowsWithURLs(urls_deleted_details->rows);
break;
}
@@ -465,14 +465,15 @@ void NetworkActionPredictor::DeleteAllRows() {
DATABASE_ACTION_DELETE_ALL, DATABASE_ACTION_COUNT);
}
-void NetworkActionPredictor::DeleteRowsWithURLs(const std::set<GURL>& urls) {
+void NetworkActionPredictor::DeleteRowsWithURLs(const history::URLRows& rows) {
if (!initialized_)
return;
std::vector<NetworkActionPredictorDatabase::Row::Id> id_list;
for (DBCacheMap::iterator it = db_cache_.begin(); it != db_cache_.end();) {
- if (urls.find(it->first.url) != urls.end()) {
+ if (std::find_if(rows.begin(), rows.end(),
+ history::URLRow::URLRowHasURL(it->first.url)) != rows.end()) {
const DBIdCacheMap::iterator id_it = db_id_cache_.find(it->first);
DCHECK(id_it != db_id_cache_.end());
id_list.push_back(id_it->second);
diff --git a/chrome/browser/autocomplete/network_action_predictor.h b/chrome/browser/autocomplete/network_action_predictor.h
index a8351e4..0cf02a4 100644
--- a/chrome/browser/autocomplete/network_action_predictor.h
+++ b/chrome/browser/autocomplete/network_action_predictor.h
@@ -13,6 +13,7 @@
#include "base/memory/weak_ptr.h"
#include "base/string16.h"
#include "chrome/browser/autocomplete/network_action_predictor_database.h"
+#include "chrome/browser/history/history_types.h"
#include "chrome/browser/profiles/profile_keyed_service.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
@@ -176,8 +177,8 @@ class NetworkActionPredictor
// Removes all rows from the database and caches.
void DeleteAllRows();
- // Removes rows from the database and caches that contain a URL in |urls|.
- void DeleteRowsWithURLs(const std::set<GURL>& urls);
+ // Removes rows from the database and caches that contain a URL in |rows|.
+ void DeleteRowsWithURLs(const history::URLRows& rows);
// Used to batch operations on the database.
void BeginTransaction();
diff --git a/chrome/browser/autocomplete/network_action_predictor_unittest.cc b/chrome/browser/autocomplete/network_action_predictor_unittest.cc
index 25bff10..e5ce30b 100644
--- a/chrome/browser/autocomplete/network_action_predictor_unittest.cc
+++ b/chrome/browser/autocomplete/network_action_predictor_unittest.cc
@@ -170,8 +170,8 @@ class NetworkActionPredictorTest : public testing::Test {
predictor_->DeleteAllRows();
}
- void DeleteRowsWithURLs(const std::set<GURL>& urls) {
- predictor_->DeleteRowsWithURLs(urls);
+ void DeleteRowsWithURLs(const history::URLRows& rows) {
+ predictor_->DeleteRowsWithURLs(rows);
}
void DeleteOldIdsFromCaches(
@@ -278,11 +278,11 @@ TEST_F(NetworkActionPredictorTest, DeleteRowsWithURLs) {
EXPECT_EQ(arraysize(test_url_db), db_cache()->size());
EXPECT_EQ(arraysize(test_url_db), db_id_cache()->size());
- std::set<GURL> urls;
+ history::URLRows rows;
for (size_t i = 0; i < 2; ++i)
- urls.insert(test_url_db[i].url);
+ rows.push_back(history::URLRow(test_url_db[i].url));
- DeleteRowsWithURLs(urls);
+ DeleteRowsWithURLs(rows);
EXPECT_EQ(arraysize(test_url_db) - 2, db_cache()->size());
EXPECT_EQ(arraysize(test_url_db) - 2, db_id_cache()->size());
diff --git a/chrome/browser/history/android/android_provider_backend.cc b/chrome/browser/history/android/android_provider_backend.cc
index 3a06a7a..077e45f 100644
--- a/chrome/browser/history/android/android_provider_backend.cc
+++ b/chrome/browser/history/android/android_provider_backend.cc
@@ -900,7 +900,6 @@ bool AndroidProviderBackend::SimulateUpdateURL(
if (!history_db_->GetURLRow(ids[0].url_id, &old_url_row))
return false;
deleted_details->rows.push_back(old_url_row);
- deleted_details->urls.insert(old_url_row.url());
FaviconID favicon_id = statement->statement()->ColumnInt64(4);
if (favicon_id) {
@@ -1026,7 +1025,6 @@ bool AndroidProviderBackend::DeleteHistoryInternal(
if (!history_db_->GetURLRow(i->url_id, &url_row))
return false;
deleted_details->rows.push_back(url_row);
- deleted_details->urls.insert(url_row.url());
if (thumbnail_db_->GetIconMappingsForPageURL(url_row.url(), NULL))
favicon->urls.insert(url_row.url());
}
diff --git a/chrome/browser/history/android/android_provider_backend_unittest.cc b/chrome/browser/history/android/android_provider_backend_unittest.cc
index 989ce60..d2c9107 100644
--- a/chrome/browser/history/android/android_provider_backend_unittest.cc
+++ b/chrome/browser/history/android/android_provider_backend_unittest.cc
@@ -547,9 +547,6 @@ TEST_F(AndroidProviderBackendTest, DeleteHistoryAndBookmarks) {
ASSERT_TRUE(delegate_.deleted_details());
EXPECT_FALSE(delegate_.modified_details());
EXPECT_EQ(1u, delegate_.deleted_details()->rows.size());
- EXPECT_EQ(1u, delegate_.deleted_details()->urls.size());
- EXPECT_TRUE(delegate_.deleted_details()->urls.end() !=
- delegate_.deleted_details()->urls.find(row1.url()));
EXPECT_EQ(row1.url(), delegate_.deleted_details()->rows[0].url());
EXPECT_EQ(row1.last_visit_time(),
delegate_.deleted_details()->rows[0].last_visit());
@@ -597,9 +594,6 @@ TEST_F(AndroidProviderBackendTest, DeleteHistoryAndBookmarks) {
ASSERT_TRUE(delegate_.deleted_details());
EXPECT_FALSE(delegate_.modified_details());
EXPECT_EQ(1u, delegate_.deleted_details()->rows.size());
- EXPECT_EQ(1u, delegate_.deleted_details()->urls.size());
- EXPECT_TRUE(delegate_.deleted_details()->urls.end() !=
- delegate_.deleted_details()->urls.find(row2.url()));
EXPECT_EQ(row2.url(), delegate_.deleted_details()->rows[0].url());
EXPECT_EQ(row2.last_visit_time(),
delegate_.deleted_details()->rows[0].last_visit());
@@ -755,9 +749,6 @@ TEST_F(AndroidProviderBackendTest, UpdateURL) {
// Verify notifications, Update involves insert and delete URLS.
ASSERT_TRUE(delegate_.deleted_details());
EXPECT_EQ(1u, delegate_.deleted_details()->rows.size());
- EXPECT_EQ(1u, delegate_.deleted_details()->urls.size());
- EXPECT_TRUE(delegate_.deleted_details()->urls.end() !=
- delegate_.deleted_details()->urls.find(row1.url()));
EXPECT_EQ(row1.url(), delegate_.deleted_details()->rows[0].url());
EXPECT_EQ(row1.last_visit_time(),
delegate_.deleted_details()->rows[0].last_visit());
@@ -810,9 +801,6 @@ TEST_F(AndroidProviderBackendTest, UpdateURL) {
// Verify notifications, Update involves insert and delete URLS.
ASSERT_TRUE(delegate_.deleted_details());
EXPECT_EQ(1u, delegate_.deleted_details()->rows.size());
- EXPECT_EQ(1u, delegate_.deleted_details()->urls.size());
- EXPECT_TRUE(delegate_.deleted_details()->urls.end() !=
- delegate_.deleted_details()->urls.find(row2.url()));
EXPECT_EQ(row2.url(), delegate_.deleted_details()->rows[0].url());
EXPECT_EQ(row2.last_visit_time(),
delegate_.deleted_details()->rows[0].last_visit());
@@ -1496,9 +1484,6 @@ TEST_F(AndroidProviderBackendTest, DeleteHistory) {
// Verify notification
ASSERT_TRUE(delegate_.deleted_details());
ASSERT_EQ(2u, delegate_.deleted_details()->rows.size());
- ASSERT_EQ(2u, delegate_.deleted_details()->urls.size());
- ASSERT_TRUE(delegate_.modified_details());
- ASSERT_EQ(1u, delegate_.modified_details()->changed_urls.size());
EXPECT_EQ(row1.url(),
delegate_.modified_details()->changed_urls[0].url());
EXPECT_EQ(Time::UnixEpoch(),
diff --git a/chrome/browser/history/expire_history_backend.cc b/chrome/browser/history/expire_history_backend.cc
index 2946588..9ccdd59 100644
--- a/chrome/browser/history/expire_history_backend.cc
+++ b/chrome/browser/history/expire_history_backend.cc
@@ -359,8 +359,7 @@ void ExpireHistoryBackend::BroadcastDeleteNotifications(
// determine if they care whether anything was deleted).
URLsDeletedDetails* deleted_details = new URLsDeletedDetails;
deleted_details->all_history = false;
- for (size_t i = 0; i < dependencies->deleted_urls.size(); i++)
- deleted_details->urls.insert(dependencies->deleted_urls[i].url());
+ deleted_details->rows = dependencies->deleted_urls;
delegate_->BroadcastNotifications(
chrome::NOTIFICATION_HISTORY_URLS_DELETED, deleted_details);
}
diff --git a/chrome/browser/history/expire_history_backend_unittest.cc b/chrome/browser/history/expire_history_backend_unittest.cc
index 691681f..ab2d570 100644
--- a/chrome/browser/history/expire_history_backend_unittest.cc
+++ b/chrome/browser/history/expire_history_backend_unittest.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <algorithm>
#include <string>
#include <utility>
@@ -381,10 +382,10 @@ void ExpireHistoryTest::EnsureURLInfoGone(const URLRow& row) {
bool found_delete_notification = false;
for (size_t i = 0; i < notifications_.size(); i++) {
if (notifications_[i].first == chrome::NOTIFICATION_HISTORY_URLS_DELETED) {
- const URLsDeletedDetails* deleted_details =
- reinterpret_cast<URLsDeletedDetails*>(notifications_[i].second);
- if (deleted_details->urls.find(row.url()) !=
- deleted_details->urls.end()) {
+ const history::URLRows& rows(reinterpret_cast<URLsDeletedDetails*>(
+ notifications_[i].second)->rows);
+ if (std::find_if(rows.begin(), rows.end(),
+ history::URLRow::URLRowHasURL(row.url())) != rows.end()) {
found_delete_notification = true;
}
} else {
diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index 781d8aa..9e234eb 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -687,7 +687,7 @@ void HistoryService::Observe(int type,
if (deleted_details->all_history)
visited_links->DeleteAllURLs();
else // Delete individual ones.
- visited_links->DeleteURLs(deleted_details->urls);
+ visited_links->DeleteURLs(deleted_details->rows);
break;
}
diff --git a/chrome/browser/history/history_extension_api.cc b/chrome/browser/history/history_extension_api.cc
index 02536b4..af0f8ca 100644
--- a/chrome/browser/history/history_extension_api.cc
+++ b/chrome/browser/history/history_extension_api.cc
@@ -145,10 +145,9 @@ void HistoryExtensionEventRouter::HistoryUrlsRemoved(
DictionaryValue* dict = new DictionaryValue();
dict->SetBoolean(kAllHistoryKey, details->all_history);
ListValue* urls = new ListValue();
- for (std::set<GURL>::const_iterator iterator = details->urls.begin();
- iterator != details->urls.end();
- ++iterator) {
- urls->Append(new StringValue(iterator->spec()));
+ for (history::URLRows::const_iterator iterator = details->rows.begin();
+ iterator != details->rows.end(); ++iterator) {
+ urls->Append(new StringValue(iterator->url().spec()));
}
dict->Set(kUrlsKey, urls);
args.Append(dict);
diff --git a/chrome/browser/history/history_notifications.h b/chrome/browser/history/history_notifications.h
index ac750e1..25e0b4f 100644
--- a/chrome/browser/history/history_notifications.h
+++ b/chrome/browser/history/history_notifications.h
@@ -57,15 +57,9 @@ struct URLsDeletedDetails : public HistoryDetails {
// Set when all history was deleted. False means just a subset was deleted.
bool all_history;
- // The URLRows which have been deleted.
+ // The URLRows of URLs deleted. This is valid only when all_history is false
+ // indicating that a subset of history has been deleted.
URLRows rows;
-
- // The list of unique URLs affected. This is valid only when a subset of
- // history is deleted. When all of it is deleted, this will be empty, since
- // we do not bother to list all URLs. (This information can be gleaned from
- // |rows| but, since there are several clients who need the set, we pre-build
- // it so that the clients don't have to.)
- std::set<GURL> urls;
};
// Details for NOTIFY_URLS_STARRED.
diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h
index 30ded66..b2e7ca8 100644
--- a/chrome/browser/history/history_types.h
+++ b/chrome/browser/history/history_types.h
@@ -132,6 +132,19 @@ class URLRow {
hidden_ = hidden;
}
+ // Helper functor that determines if an URLRow refers to a given URL.
+ class URLRowHasURL {
+ public:
+ explicit URLRowHasURL(const GURL& url) : url_(url) {}
+
+ bool operator()(const URLRow& row) {
+ return row.url() == url_;
+ }
+
+ private:
+ const GURL& url_;
+ };
+
protected:
// Swaps the contents of this URLRow with another, which allows it to be
// destructively copied without memory allocations.
diff --git a/chrome/browser/history/in_memory_url_index_unittest.cc b/chrome/browser/history/in_memory_url_index_unittest.cc
index 607a032..ee34125 100644
--- a/chrome/browser/history/in_memory_url_index_unittest.cc
+++ b/chrome/browser/history/in_memory_url_index_unittest.cc
@@ -17,10 +17,14 @@
#include "chrome/browser/history/history.h"
#include "chrome/browser/history/history_backend.h"
#include "chrome/browser/history/history_database.h"
+#include "chrome/browser/history/history_notifications.h"
#include "chrome/browser/history/in_memory_url_index_types.h"
#include "chrome/browser/history/in_memory_url_index.h"
#include "chrome/browser/history/url_index_private_data.h"
+#include "chrome/common/chrome_notification_types.h"
#include "chrome/common/chrome_paths.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_source.h"
#include "chrome/test/base/testing_profile.h"
#include "content/test/test_browser_thread.h"
#include "sql/transaction.h"
@@ -129,6 +133,9 @@ class InMemoryURLIndexTest : public testing::Test {
bool GetCacheFilePath(FilePath* file_path) const;
void PostRestoreFromCacheFileTask();
void PostSaveToCacheFileTask();
+ void Observe(int notification_type,
+ const content::NotificationSource& source,
+ const content::NotificationDetails& details);
const std::set<std::string>& scheme_whitelist();
@@ -186,6 +193,13 @@ void InMemoryURLIndexTest::PostSaveToCacheFileTask() {
url_index_->PostSaveToCacheFileTask();
}
+void InMemoryURLIndexTest::Observe(
+ int notification_type,
+ const content::NotificationSource& source,
+ const content::NotificationDetails& details) {
+ url_index_->Observe(notification_type, source, details);
+}
+
const std::set<std::string>& InMemoryURLIndexTest::scheme_whitelist() {
return url_index_->scheme_whitelist();
}
@@ -831,7 +845,7 @@ TEST_F(InMemoryURLIndexTest, DeleteRows) {
url_index_->HistoryItemsForTerms(ASCIIToUTF16("DrudgeReport"));
ASSERT_EQ(1U, matches.size());
- // Determine the row id for that result, delete that id, then search again.
+ // Delete the URL for then search again.
EXPECT_TRUE(DeleteURL(matches[0].url_info.url()));
EXPECT_TRUE(url_index_->HistoryItemsForTerms(
ASCIIToUTF16("DrudgeReport")).empty());
@@ -841,6 +855,23 @@ TEST_F(InMemoryURLIndexTest, DeleteRows) {
EXPECT_FALSE(DeleteURL(url));
}
+TEST_F(InMemoryURLIndexTest, ExpireRow) {
+ ScoredHistoryMatches matches =
+ url_index_->HistoryItemsForTerms(ASCIIToUTF16("DrudgeReport"));
+ ASSERT_EQ(1U, matches.size());
+
+ // Determine the row id for the result, remember that id, broadcast a
+ // delete notification, then ensure that the row has been deleted.
+ URLsDeletedDetails* deleted_details = new URLsDeletedDetails;
+ deleted_details->all_history = false;
+ deleted_details->rows.push_back(matches[0].url_info);
+ Observe(chrome::NOTIFICATION_HISTORY_URLS_DELETED,
+ content::Source<InMemoryURLIndexTest>(this),
+ content::Details<history::HistoryDetails>(deleted_details));
+ EXPECT_TRUE(url_index_->HistoryItemsForTerms(
+ ASCIIToUTF16("DrudgeReport")).empty());
+}
+
TEST_F(InMemoryURLIndexTest, WhitelistedURLs) {
struct TestData {
const std::string url_spec;
diff --git a/chrome/browser/history/shortcuts_backend.cc b/chrome/browser/history/shortcuts_backend.cc
index db350e5..dfa6982 100644
--- a/chrome/browser/history/shortcuts_backend.cc
+++ b/chrome/browser/history/shortcuts_backend.cc
@@ -227,13 +227,15 @@ void ShortcutsBackend::Observe(int type,
all_history) {
DeleteAllShortcuts();
}
- const std::set<GURL>& urls =
- content::Details<const history::URLsDeletedDetails>(details)->urls;
+ const URLRows& rows(
+ content::Details<const history::URLsDeletedDetails>(details)->rows);
std::vector<std::string> shortcut_ids;
for (GuidToShortcutsIteratorMap::iterator it = guid_map_.begin();
it != guid_map_.end(); ++it) {
- if (urls.find(it->second->second.url) != urls.end())
+ if (std::find_if(rows.begin(), rows.end(),
+ URLRow::URLRowHasURL(it->second->second.url)) !=
+ rows.end())
shortcut_ids.push_back(it->first);
}
DeleteShortcutsWithIds(shortcut_ids);
diff --git a/chrome/browser/history/top_sites.cc b/chrome/browser/history/top_sites.cc
index e6416dc..ac13506 100644
--- a/chrome/browser/history/top_sites.cc
+++ b/chrome/browser/history/top_sites.cc
@@ -682,10 +682,10 @@ void TopSites::Observe(int type,
backend_->ResetDatabase();
} else {
std::set<size_t> indices_to_delete; // Indices into top_sites_.
- for (std::set<GURL>::iterator i = deleted_details->urls.begin();
- i != deleted_details->urls.end(); ++i) {
- if (cache_->IsKnownURL(*i))
- indices_to_delete.insert(cache_->GetURLIndex(*i));
+ for (URLRows::const_iterator i = deleted_details->rows.begin();
+ i != deleted_details->rows.end(); ++i) {
+ if (cache_->IsKnownURL(i->url()))
+ indices_to_delete.insert(cache_->GetURLIndex(i->url()));
}
if (indices_to_delete.empty())
diff --git a/chrome/browser/sync/glue/typed_url_change_processor.cc b/chrome/browser/sync/glue/typed_url_change_processor.cc
index 194f822..9c853f7 100644
--- a/chrome/browser/sync/glue/typed_url_change_processor.cc
+++ b/chrome/browser/sync/glue/typed_url_change_processor.cc
@@ -151,12 +151,13 @@ void TypedUrlChangeProcessor::HandleURLsDeleted(
return;
}
} else {
- for (std::set<GURL>::iterator url = details->urls.begin();
- url != details->urls.end(); ++url) {
+ for (history::URLRows::const_iterator row = details->rows.begin();
+ row != details->rows.end(); ++row) {
sync_api::WriteNode sync_node(&trans);
// The deleted URL could have been non-typed, so it might not be found
// in the sync DB.
- if (sync_node.InitByClientTagLookup(syncable::TYPED_URLS, url->spec()))
+ if (sync_node.InitByClientTagLookup(syncable::TYPED_URLS,
+ row->url().spec()))
sync_node.Remove();
}
}
diff --git a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc b/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc
index 7ebe6b5..98ccc6a 100644
--- a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc
@@ -714,7 +714,7 @@ TEST_F(ProfileSyncServiceTypedUrlTest, ProcessUserChangeRemove) {
history::URLsDeletedDetails changes;
changes.all_history = false;
- changes.urls.insert(GURL("http://mine.com"));
+ changes.rows.push_back(history::URLRow(GURL("http://mine.com")));
scoped_refptr<ThreadNotifier> notifier(new ThreadNotifier(&history_thread_));
notifier->Notify(chrome::NOTIFICATION_HISTORY_URLS_DELETED,
content::Source<Profile>(&profile_),
diff --git a/chrome/browser/ui/webui/history_ui.cc b/chrome/browser/ui/webui/history_ui.cc
index 5735fe5..76bf780 100644
--- a/chrome/browser/ui/webui/history_ui.cc
+++ b/chrome/browser/ui/webui/history_ui.cc
@@ -429,6 +429,20 @@ history::QueryOptions BrowsingHistoryHandler::CreateMonthQueryOptions(
return options;
}
+// Helper function for Observe that determines if there are any differences
+// between the URLs noticed for deletion and the ones we are expecting.
+static bool DeletionsDiffer(const history::URLRows& deleted_rows,
+ const std::set<GURL>& urls_to_be_deleted) {
+ if (deleted_rows.size() != urls_to_be_deleted.size())
+ return true;
+ for (history::URLRows::const_iterator i = deleted_rows.begin();
+ i != deleted_rows.end(); ++i) {
+ if (urls_to_be_deleted.find(i->url()) == urls_to_be_deleted.end())
+ return true;
+ }
+ return false;
+}
+
void BrowsingHistoryHandler::Observe(
int type,
const content::NotificationSource& source,
@@ -439,11 +453,9 @@ void BrowsingHistoryHandler::Observe(
}
history::URLsDeletedDetails* deletedDetails =
content::Details<history::URLsDeletedDetails>(details).ptr();
- if (deletedDetails->urls != urls_to_be_deleted_ ||
- deletedDetails->all_history) {
- // Notify the page that someone else deleted from the history.
+ if (deletedDetails->all_history ||
+ DeletionsDiffer(deletedDetails->rows, urls_to_be_deleted_))
web_ui()->CallJavascriptFunction("historyDeleted");
- }
}
////////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/visitedlink/visitedlink_master.cc b/chrome/browser/visitedlink/visitedlink_master.cc
index 38c87ca..c80f0d5 100644
--- a/chrome/browser/visitedlink/visitedlink_master.cc
+++ b/chrome/browser/visitedlink/visitedlink_master.cc
@@ -289,10 +289,10 @@ void VisitedLinkMaster::DeleteAllURLs() {
listener_->Reset();
}
-void VisitedLinkMaster::DeleteURLs(const std::set<GURL>& urls) {
+void VisitedLinkMaster::DeleteURLs(const history::URLRows& rows) {
typedef std::set<GURL>::const_iterator SetIterator;
- if (urls.empty())
+ if (rows.empty())
return;
listener_->Reset();
@@ -300,12 +300,14 @@ void VisitedLinkMaster::DeleteURLs(const std::set<GURL>& urls) {
if (table_builder_) {
// A rebuild is in progress, save this deletion in the temporary list so
// it can be added once rebuild is complete.
- for (SetIterator i = urls.begin(); i != urls.end(); ++i) {
- if (!i->is_valid())
+ for (history::URLRows::const_iterator i = rows.begin(); i != rows.end();
+ ++i) {
+ const GURL& url(i->url());
+ if (!url.is_valid())
continue;
Fingerprint fingerprint =
- ComputeURLFingerprint(i->spec().data(), i->spec().size(), salt_);
+ ComputeURLFingerprint(url.spec().data(), url.spec().size(), salt_);
deleted_since_rebuild_.insert(fingerprint);
// If the URL was just added and now we're deleting it, it may be in the
@@ -324,11 +326,13 @@ void VisitedLinkMaster::DeleteURLs(const std::set<GURL>& urls) {
// Compute the deleted URLs' fingerprints and delete them
std::set<Fingerprint> deleted_fingerprints;
- for (SetIterator i = urls.begin(); i != urls.end(); ++i) {
- if (!i->is_valid())
+ for (history::URLRows::const_iterator i = rows.begin(); i != rows.end();
+ ++i) {
+ const GURL& url(i->url());
+ if (!url.is_valid())
continue;
deleted_fingerprints.insert(
- ComputeURLFingerprint(i->spec().data(), i->spec().size(), salt_));
+ ComputeURLFingerprint(url.spec().data(), url.spec().size(), salt_));
}
DeleteFingerprintsFromCurrentTable(deleted_fingerprints);
}
diff --git a/chrome/browser/visitedlink/visitedlink_master.h b/chrome/browser/visitedlink/visitedlink_master.h
index 1771afe..5e2afd7 100644
--- a/chrome/browser/visitedlink/visitedlink_master.h
+++ b/chrome/browser/visitedlink/visitedlink_master.h
@@ -19,6 +19,7 @@
#include "base/shared_memory.h"
#include "base/threading/sequenced_worker_pool.h"
#include "chrome/browser/history/history.h"
+#include "chrome/browser/history/history_types.h"
#include "chrome/common/visitedlink_common.h"
class GURL;
@@ -89,8 +90,8 @@ class VisitedLinkMaster : public VisitedLinkCommon {
// Adds a set of URLs to the table.
void AddURLs(const std::vector<GURL>& url);
- // Deletes the specified URLs from the table.
- void DeleteURLs(const std::set<GURL>& urls);
+ // Deletes the specified URLs from |rows| from the table.
+ void DeleteURLs(const history::URLRows& rows);
// Clears the visited links table by deleting the file from disk. Used as
// part of history clearing.
diff --git a/chrome/browser/visitedlink/visitedlink_unittest.cc b/chrome/browser/visitedlink/visitedlink_unittest.cc
index 54228bb..b6ca5c0 100644
--- a/chrome/browser/visitedlink/visitedlink_unittest.cc
+++ b/chrome/browser/visitedlink/visitedlink_unittest.cc
@@ -263,11 +263,11 @@ TEST_F(VisitedLinkTest, BigDelete) {
// Add more URLs than necessary to trigger this case.
const int kTestDeleteCount = VisitedLinkMaster::kBigDeleteThreshold + 2;
- std::set<GURL> urls_to_delete;
+ history::URLRows urls_to_delete;
for (int32 i = g_test_count; i < g_test_count + kTestDeleteCount; i++) {
GURL url(TestURL(i));
master_->AddURL(url);
- urls_to_delete.insert(url);
+ urls_to_delete.push_back(history::URLRow(url));
}
master_->DeleteURLs(urls_to_delete);
@@ -399,8 +399,8 @@ TEST_F(VisitedLinkTest, Rebuild) {
// Add one more and then delete it.
master_->AddURL(TestURL(g_test_count));
- std::set<GURL> deleted_urls;
- deleted_urls.insert(TestURL(g_test_count));
+ history::URLRows deleted_urls;
+ deleted_urls.push_back(history::URLRow(TestURL(g_test_count)));
master_->DeleteURLs(deleted_urls);
// Wait for the rebuild to complete. The task will terminate the message
@@ -447,8 +447,8 @@ TEST_F(VisitedLinkTest, Listener) {
ASSERT_EQ(i + 1, master_->GetUsedCount());
}
- std::set<GURL> deleted_urls;
- deleted_urls.insert(TestURL(0));
+ history::URLRows deleted_urls;
+ deleted_urls.push_back(history::URLRow(TestURL(0)));
// Delete an URL.
master_->DeleteURLs(deleted_urls);
// ... and all of the remaining ones.