diff options
-rw-r--r-- | chrome/browser/net/sdch_browsertest.cc | 68 | ||||
-rw-r--r-- | net/base/sdch_manager.cc | 12 | ||||
-rw-r--r-- | net/base/sdch_manager_unittest.cc | 56 | ||||
-rw-r--r-- | net/base/sdch_observer.h | 19 | ||||
-rw-r--r-- | net/filter/sdch_filter_unittest.cc | 11 | ||||
-rw-r--r-- | net/sdch/sdch_owner.cc | 13 | ||||
-rw-r--r-- | net/sdch/sdch_owner.h | 11 | ||||
-rw-r--r-- | net/sdch/sdch_owner_unittest.cc | 10 |
8 files changed, 131 insertions, 69 deletions
diff --git a/chrome/browser/net/sdch_browsertest.cc b/chrome/browser/net/sdch_browsertest.cc index 16bf982..7a1cb7b 100644 --- a/chrome/browser/net/sdch_browsertest.cc +++ b/chrome/browser/net/sdch_browsertest.cc @@ -261,9 +261,41 @@ class SdchResponseHandler { base::WeakPtrFactory<SdchResponseHandler> weak_ptr_factory_; }; +class TestSdchObserver : public net::SdchObserver { + public: + TestSdchObserver() : manager_(nullptr), fetch_count_(0) {} + ~TestSdchObserver() override { + if (manager_) { + manager_->RemoveObserver(this); + } + } + + void Observe(net::SdchManager* manager) { + DCHECK(!manager_); + manager_ = manager; + manager_->AddObserver(this); + } + + // SdchObserver + void OnDictionaryAdded(const GURL& /* dictionary_url */, + const std::string& /* server_hash */) override {} + void OnDictionaryRemoved(const std::string& /* server_hash */) override {} + void OnGetDictionary(const GURL& /* request_url */, + const GURL& /* dictionary_url */) override { + fetch_count_++; + } + void OnDictionaryUsed(const std::string& /* server_hash */) override {} + void OnClearDictionaries() override {} + + int fetch_count() const { return fetch_count_; } + + private: + net::SdchManager* manager_; + int fetch_count_; +}; + class SdchBrowserTest : public InProcessBrowserTest, - public net::URLFetcherDelegate, - public net::SdchObserver { + public net::URLFetcherDelegate { public: static const char kTestHost[]; @@ -548,9 +580,9 @@ class SdchBrowserTest : public InProcessBrowserTest, net::SdchManager* manager( context_getter->GetURLRequestContext()->sdch_manager()); - DCHECK(fetch_counts_.end() != fetch_counts_.find(manager)); + DCHECK(observers_.end() != observers_.find(manager)); - *result = fetch_counts_[manager]; + *result = observers_[manager].fetch_count(); } // InProcessBrowserTest @@ -595,36 +627,16 @@ class SdchBrowserTest : public InProcessBrowserTest, net::SdchManager* manager = context_getter->GetURLRequestContext()->sdch_manager(); - DCHECK(fetch_counts_.end() == fetch_counts_.find(manager)); + DCHECK(observers_.end() == observers_.find(manager)); - fetch_counts_[manager] = 0; - manager->AddObserver(this); + observers_[manager].Observe(manager); } void UnsubscribeFromAllSdchNotifications() { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - - for (auto it = fetch_counts_.begin(); it != fetch_counts_.end(); ++it) - it->first->RemoveObserver(this); - - fetch_counts_.clear(); - } - - // SdchObserver - void OnDictionaryUsed(net::SdchManager* manager, - const std::string& server_hash) override {} - - void OnGetDictionary(net::SdchManager* manager, - const GURL& request_url, - const GURL& dictionary_url) override { - DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - DLOG(ERROR) << "Retrieving count of notifications from manager " << manager; - DCHECK(fetch_counts_.end() != fetch_counts_.find(manager)); - ++fetch_counts_[manager]; + observers_.clear(); } - void OnClearDictionaries(net::SdchManager* manager) override {} - // URLFetcherDelegate void OnURLFetchComplete(const net::URLFetcher* source) override { url_fetch_complete_ = true; @@ -644,7 +656,7 @@ class SdchBrowserTest : public InProcessBrowserTest, Browser* incognito_browser_; // IO Thread access only. - std::map<net::SdchManager*, int> fetch_counts_; + std::map<net::SdchManager*, TestSdchObserver> observers_; }; const char SdchBrowserTest::kTestHost[] = "our.test.host.com"; diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc index c43c8b0..cd702f7 100644 --- a/net/base/sdch_manager.cc +++ b/net/base/sdch_manager.cc @@ -103,7 +103,7 @@ void SdchManager::ClearData() { blacklisted_domains_.clear(); allow_latency_experiment_.clear(); dictionaries_.clear(); - FOR_EACH_OBSERVER(SdchObserver, observers_, OnClearDictionaries(this)); + FOR_EACH_OBSERVER(SdchObserver, observers_, OnClearDictionaries()); } // static @@ -220,14 +220,14 @@ SdchProblemCode SdchManager::OnGetDictionary(const GURL& request_url, FOR_EACH_OBSERVER(SdchObserver, observers_, - OnGetDictionary(this, request_url, dictionary_url)); + OnGetDictionary(request_url, dictionary_url)); return SDCH_OK; } void SdchManager::OnDictionaryUsed(const std::string& server_hash) { FOR_EACH_OBSERVER(SdchObserver, observers_, - OnDictionaryUsed(this, server_hash)); + OnDictionaryUsed(server_hash)); } SdchProblemCode SdchManager::CanFetchDictionary( @@ -448,6 +448,9 @@ SdchProblemCode SdchManager::AddSdchDictionary( if (server_hash_p) *server_hash_p = server_hash; + FOR_EACH_OBSERVER(SdchObserver, observers_, + OnDictionaryAdded(dictionary_url, server_hash)); + return SDCH_OK; } @@ -457,6 +460,9 @@ SdchProblemCode SdchManager::RemoveSdchDictionary( return SDCH_DICTIONARY_HASH_NOT_FOUND; dictionaries_.erase(server_hash); + + FOR_EACH_OBSERVER(SdchObserver, observers_, OnDictionaryRemoved(server_hash)); + return SDCH_OK; } diff --git a/net/base/sdch_manager_unittest.cc b/net/base/sdch_manager_unittest.cc index ba3248e..9bb0e7e 100644 --- a/net/base/sdch_manager_unittest.cc +++ b/net/base/sdch_manager_unittest.cc @@ -29,10 +29,18 @@ static const char kTestVcdiffDictionary[] = "DictionaryFor" class MockSdchObserver : public SdchObserver { public: MockSdchObserver() - : dictionary_used_notifications_(0), + : dictionary_added_notifications_(0), + dictionary_removed_notifications_(0), + dictionary_used_notifications_(0), get_dictionary_notifications_(0), clear_dictionaries_notifications_(0) {} + int dictionary_added_notifications() const { + return dictionary_added_notifications_; + } + int dictionary_removed_notifications() const { + return dictionary_removed_notifications_; + } std::string last_server_hash() const { return last_server_hash_; } int dictionary_used_notifications() const { return dictionary_used_notifications_; @@ -50,24 +58,34 @@ class MockSdchObserver : public SdchObserver { } // SdchObserver implementation - void OnDictionaryUsed(SdchManager* manager, - const std::string& server_hash) override { + void OnDictionaryAdded(const GURL& dictionary_url, + const std::string& server_hash) override { + last_server_hash_ = server_hash; + last_dictionary_url_ = dictionary_url; + ++dictionary_added_notifications_; + } + void OnDictionaryRemoved(const std::string& server_hash) override { + last_server_hash_ = server_hash; + ++dictionary_removed_notifications_; + } + void OnDictionaryUsed(const std::string& server_hash) override { last_server_hash_ = server_hash; ++dictionary_used_notifications_; } - void OnGetDictionary(SdchManager* manager, - const GURL& request_url, + void OnGetDictionary(const GURL& request_url, const GURL& dictionary_url) override { ++get_dictionary_notifications_; last_dictionary_request_url_ = request_url; last_dictionary_url_ = dictionary_url; } - void OnClearDictionaries(SdchManager* manager) override { + void OnClearDictionaries() override { ++clear_dictionaries_notifications_; } private: + int dictionary_added_notifications_; + int dictionary_removed_notifications_; int dictionary_used_notifications_; int get_dictionary_notifications_; int clear_dictionaries_notifications_; @@ -628,19 +646,37 @@ TEST_F(SdchManagerTest, SdchDictionaryUsed) { std::string server_hash; SdchManager::GenerateHash(dictionary_text, &client_hash, &server_hash); EXPECT_TRUE(AddSdchDictionary(dictionary_text, target_gurl)); - EXPECT_EQ("xyzzy", observer.last_server_hash()); EXPECT_EQ(1, observer.dictionary_used_notifications()); EXPECT_TRUE(sdch_manager()->GetDictionarySet(target_gurl)); - EXPECT_EQ("xyzzy", observer.last_server_hash()); EXPECT_EQ(1, observer.dictionary_used_notifications()); sdch_manager()->RemoveObserver(&observer); EXPECT_EQ(1, observer.dictionary_used_notifications()); - EXPECT_EQ("xyzzy", observer.last_server_hash()); sdch_manager()->OnDictionaryUsed("plugh"); EXPECT_EQ(1, observer.dictionary_used_notifications()); - EXPECT_EQ("xyzzy", observer.last_server_hash()); +} + +TEST_F(SdchManagerTest, AddRemoveNotifications) { + MockSdchObserver observer; + sdch_manager()->AddObserver(&observer); + + std::string dictionary_domain("x.y.z.google.com"); + GURL target_gurl("http://" + dictionary_domain); + std::string dictionary_text(NewSdchDictionary(dictionary_domain)); + std::string client_hash; + std::string server_hash; + SdchManager::GenerateHash(dictionary_text, &client_hash, &server_hash); + EXPECT_TRUE(AddSdchDictionary(dictionary_text, target_gurl)); + EXPECT_EQ(1, observer.dictionary_added_notifications()); + EXPECT_EQ(target_gurl, observer.last_dictionary_url()); + EXPECT_EQ(server_hash, observer.last_server_hash()); + + EXPECT_EQ(SDCH_OK, sdch_manager()->RemoveSdchDictionary(server_hash)); + EXPECT_EQ(1, observer.dictionary_removed_notifications()); + EXPECT_EQ(server_hash, observer.last_server_hash()); + + sdch_manager()->RemoveObserver(&observer); } } // namespace net diff --git a/net/base/sdch_observer.h b/net/base/sdch_observer.h index b74a090..b2311de 100644 --- a/net/base/sdch_observer.h +++ b/net/base/sdch_observer.h @@ -22,9 +22,14 @@ class NET_EXPORT SdchObserver { public: virtual ~SdchObserver(); - // TODO(rdsmith): Add Added/Removed signals. These are only needed if - // we end up with an implementation in which more than one observer - // generates Add/Removed events; otherwise, tracking can be done internally. + // A dictionary has been added to the observed manager. + virtual void OnDictionaryAdded(const GURL& dictionary_url, + const std::string& server_hash) = 0; + + // A dictionary has been removed from the observed manager. Note that this is + // not called when dictionaries are cleared, so observers that want to track + // the loaded dictionary set need to observe OnClearDictionaries as well. + virtual void OnDictionaryRemoved(const std::string& server_hash) = 0; // TODO(rdsmith): Add signal that an Avail-Dictionary header was generated. // Should be added if/when an observer wants to use it to fine-tune @@ -38,17 +43,15 @@ class NET_EXPORT SdchObserver { // references to deleted dictionaries. Observers must handle this case. // TODO(rdsmith): Should this notification indicate how much // compression the dictionary provided? - virtual void OnDictionaryUsed(SdchManager* manager, - const std::string& server_hash) = 0; + virtual void OnDictionaryUsed(const std::string& server_hash) = 0; // A "Get-Dictionary" header has been seen. - virtual void OnGetDictionary(SdchManager* manager, - const GURL& request_url, + virtual void OnGetDictionary(const GURL& request_url, const GURL& dictionary_url) = 0; // Notification that SDCH has received a request to clear all // its dictionaries. - virtual void OnClearDictionaries(SdchManager* manager) = 0; + virtual void OnClearDictionaries() = 0; }; } // namespace net diff --git a/net/filter/sdch_filter_unittest.cc b/net/filter/sdch_filter_unittest.cc index 4601d25..a83e63f 100644 --- a/net/filter/sdch_filter_unittest.cc +++ b/net/filter/sdch_filter_unittest.cc @@ -1236,8 +1236,7 @@ class SimpleSdchObserver : public SdchObserver { ~SimpleSdchObserver() override { manager_->RemoveObserver(this); } // SdchObserver - void OnDictionaryUsed(SdchManager* manager, - const std::string& server_hash) override { + void OnDictionaryUsed(const std::string& server_hash) override { dictionary_used_++; last_server_hash_ = server_hash; } @@ -1245,10 +1244,12 @@ class SimpleSdchObserver : public SdchObserver { int dictionary_used_calls() const { return dictionary_used_; } std::string last_server_hash() const { return last_server_hash_; } - void OnGetDictionary(SdchManager* /* manager */, - const GURL& /* request_url */, + void OnDictionaryAdded(const GURL& /* dictionary_url */, + const std::string& /* server_hash */) override {} + void OnDictionaryRemoved(const std::string& /* server_hash */) override {} + void OnGetDictionary(const GURL& /* request_url */, const GURL& /* dictionary_url */) override {} - void OnClearDictionaries(SdchManager* /* manager */) override {} + void OnClearDictionaries() override {} private: int dictionary_used_; diff --git a/net/sdch/sdch_owner.cc b/net/sdch/sdch_owner.cc index 226604a..a3ef30e 100644 --- a/net/sdch/sdch_owner.cc +++ b/net/sdch/sdch_owner.cc @@ -484,8 +484,12 @@ void SdchOwner::OnDictionaryFetched(base::Time last_used, load_times_[server_hash] = clock_->Now(); } -void SdchOwner::OnDictionaryUsed(SdchManager* manager, - const std::string& server_hash) { +void SdchOwner::OnDictionaryAdded(const GURL& dictionary_url, + const std::string& server_hash) { } + +void SdchOwner::OnDictionaryRemoved(const std::string& server_hash) { } + +void SdchOwner::OnDictionaryUsed(const std::string& server_hash) { base::Time now(clock_->Now()); base::DictionaryValue* pref_dictionary_map = GetPersistentStoreDictionaryMap(pref_store_); @@ -538,8 +542,7 @@ void SdchOwner::OnDictionaryUsed(SdchManager* manager, specific_dictionary_map->SetInteger(kDictionaryUseCountKey, use_count + 1); } -void SdchOwner::OnGetDictionary(SdchManager* manager, - const GURL& request_url, +void SdchOwner::OnGetDictionary(const GURL& request_url, const GURL& dictionary_url) { #if defined(OS_CHROMEOS) // For debugging http://crbug.com/454198; remove when resolved. @@ -581,7 +584,7 @@ void SdchOwner::OnGetDictionary(SdchManager* manager, base::Unretained(this), base::Time(), 0)); } -void SdchOwner::OnClearDictionaries(SdchManager* manager) { +void SdchOwner::OnClearDictionaries() { total_dictionary_bytes_ = 0; fetcher_->Cancel(); diff --git a/net/sdch/sdch_owner.h b/net/sdch/sdch_owner.h index 04b83a1..ec5d82d 100644 --- a/net/sdch/sdch_owner.h +++ b/net/sdch/sdch_owner.h @@ -56,12 +56,13 @@ class NET_EXPORT SdchOwner : public SdchObserver, public PrefStore::Observer { void SetMinSpaceForDictionaryFetch(size_t min_space_for_dictionary_fetch); // SdchObserver implementation. - void OnDictionaryUsed(SdchManager* manager, - const std::string& server_hash) override; - void OnGetDictionary(SdchManager* manager, - const GURL& request_url, + void OnDictionaryAdded(const GURL& dictionary_url, + const std::string& server_hash) override; + void OnDictionaryRemoved(const std::string& server_hash) override; + void OnDictionaryUsed(const std::string& server_hash) override; + void OnGetDictionary(const GURL& request_url, const GURL& dictionary_url) override; - void OnClearDictionaries(SdchManager* manager) override; + void OnClearDictionaries() override; // PrefStore::Observer implementation. void OnPrefValueChanged(const std::string& key) override; diff --git a/net/sdch/sdch_owner_unittest.cc b/net/sdch/sdch_owner_unittest.cc index e522d18..788a992 100644 --- a/net/sdch/sdch_owner_unittest.cc +++ b/net/sdch/sdch_owner_unittest.cc @@ -299,7 +299,7 @@ class SdchOwnerTest : public testing::Test { } void SignalGetDictionaryAndClearJobs(GURL request_url, GURL dictionary_url) { - sdch_owner().OnGetDictionary(&sdch_manager_, request_url, dictionary_url); + sdch_owner().OnGetDictionary(request_url, dictionary_url); WaitForNoJobs(); } @@ -561,7 +561,7 @@ TEST_F(SdchOwnerTest, UseChangesEviction) { EXPECT_TRUE(DictionaryPresentInManager(server_hash_d3)); // Use the oldest dictionary. - sdch_owner().OnDictionaryUsed(&sdch_manager(), server_hash_d3); + sdch_owner().OnDictionaryUsed(server_hash_d3); // The addition of a new dictionary should succeed, evicting only the // newer stale one. @@ -600,8 +600,8 @@ TEST_F(SdchOwnerTest, UsePreventsAddition) { EXPECT_TRUE(DictionaryPresentInManager(server_hash_d3)); // Use the older dictionaries. - sdch_owner().OnDictionaryUsed(&sdch_manager(), server_hash_d2); - sdch_owner().OnDictionaryUsed(&sdch_manager(), server_hash_d3); + sdch_owner().OnDictionaryUsed(server_hash_d2); + sdch_owner().OnDictionaryUsed(server_hash_d3); // The addition of a new dictionary should fail, not evicting anything. std::string server_hash_d4; @@ -871,7 +871,7 @@ TEST_F(SdchOwnerPersistenceTest, UsingDictionaryUpdatesUseCount) { ResetOwner(false); ASSERT_TRUE(CompleteLoadFromURL(url, "0", true)); - owner_->OnDictionaryUsed(manager_.get(), hash); + owner_->OnDictionaryUsed(hash); int new_count; { |