summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/net/sdch_browsertest.cc68
-rw-r--r--net/base/sdch_manager.cc12
-rw-r--r--net/base/sdch_manager_unittest.cc56
-rw-r--r--net/base/sdch_observer.h19
-rw-r--r--net/filter/sdch_filter_unittest.cc11
-rw-r--r--net/sdch/sdch_owner.cc13
-rw-r--r--net/sdch/sdch_owner.h11
-rw-r--r--net/sdch/sdch_owner_unittest.cc10
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;
{