diff options
35 files changed, 253 insertions, 139 deletions
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn index 2676245..2395bec 100644 --- a/chrome/browser/BUILD.gn +++ b/chrome/browser/BUILD.gn @@ -38,7 +38,6 @@ static_library("browser") { "//chrome/browser/history:in_memory_url_index_cache_proto", "//chrome/browser/net:cert_logger_proto", "//chrome/browser/net:probe_message_proto", - "//chrome/browser/search/suggestions/proto", "//chrome/browser/ui", "//chrome/common", "//chrome/common/net", @@ -65,6 +64,7 @@ static_library("browser") { "//components/signin/core/browser", "//components/startup_metric_utils", "//components/strings", + "//components/suggestions", "//components/translate/core/browser", "//components/translate/core/common", "//components/url_fixer", diff --git a/chrome/browser/DEPS b/chrome/browser/DEPS index 011ba1f..21f190f 100644 --- a/chrome/browser/DEPS +++ b/chrome/browser/DEPS @@ -55,6 +55,7 @@ include_rules = [ "+components/signin", "+components/startup_metric_utils", "+components/storage_monitor", + "+components/suggestions", "+components/sync_driver", "+components/translate/content/browser", "+components/translate/content/common", diff --git a/chrome/browser/android/most_visited_sites.cc b/chrome/browser/android/most_visited_sites.cc index 16aed52..b3ff790 100644 --- a/chrome/browser/android/most_visited_sites.cc +++ b/chrome/browser/android/most_visited_sites.cc @@ -22,10 +22,10 @@ #include "chrome/browser/history/top_sites.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_android.h" -#include "chrome/browser/search/suggestions/suggestions_service.h" #include "chrome/browser/search/suggestions/suggestions_service_factory.h" #include "chrome/browser/search/suggestions/suggestions_source.h" #include "chrome/browser/thumbnails/thumbnail_list_source.h" +#include "components/suggestions/suggestions_service.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_source.h" #include "content/public/browser/url_data_source.h" diff --git a/chrome/browser/android/most_visited_sites.h b/chrome/browser/android/most_visited_sites.h index 8e1137c..39ecceb 100644 --- a/chrome/browser/android/most_visited_sites.h +++ b/chrome/browser/android/most_visited_sites.h @@ -12,7 +12,7 @@ #include "base/memory/weak_ptr.h" #include "chrome/browser/history/history_types.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" +#include "components/suggestions/proto/suggestions.pb.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" diff --git a/chrome/browser/search/suggestions/suggestions_service_factory.cc b/chrome/browser/search/suggestions/suggestions_service_factory.cc index 2e90965..a6978f4 100644 --- a/chrome/browser/search/suggestions/suggestions_service_factory.cc +++ b/chrome/browser/search/suggestions/suggestions_service_factory.cc @@ -8,20 +8,21 @@ #include "base/prefs/pref_service.h" #include "chrome/browser/profiles/incognito_helpers.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/search/suggestions/blacklist_store.h" -#include "chrome/browser/search/suggestions/image_manager.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" -#include "chrome/browser/search/suggestions/suggestions_service.h" -#include "chrome/browser/search/suggestions/suggestions_store.h" #include "chrome/browser/search/suggestions/thumbnail_manager.h" -#include "chrome/common/pref_names.h" #include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/leveldb_proto/proto_database.h" #include "components/leveldb_proto/proto_database_impl.h" #include "components/pref_registry/pref_registry_syncable.h" +#include "components/suggestions/blacklist_store.h" +#include "components/suggestions/image_manager.h" +#include "components/suggestions/proto/suggestions.pb.h" +#include "components/suggestions/suggestions_service.h" +#include "components/suggestions/suggestions_store.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/browser_thread.h" +using content::BrowserThread; + namespace suggestions { // static @@ -55,8 +56,8 @@ content::BrowserContext* SuggestionsServiceFactory::GetBrowserContextToUse( KeyedService* SuggestionsServiceFactory::BuildServiceInstanceFor( content::BrowserContext* profile) const { scoped_refptr<base::SequencedTaskRunner> background_task_runner = - content::BrowserThread::GetBlockingPool()->GetSequencedTaskRunner( - content::BrowserThread::GetBlockingPool()->GetSequenceToken()); + BrowserThread::GetBlockingPool()->GetSequencedTaskRunner( + BrowserThread::GetBlockingPool()->GetSequenceToken()); Profile* the_profile = static_cast<Profile*>(profile); scoped_ptr<SuggestionsStore> suggestions_store( diff --git a/chrome/browser/search/suggestions/suggestions_source.cc b/chrome/browser/search/suggestions/suggestions_source.cc index 98bd51a..cb0e17e 100644 --- a/chrome/browser/search/suggestions/suggestions_source.cc +++ b/chrome/browser/search/suggestions/suggestions_source.cc @@ -14,9 +14,9 @@ #include "base/strings/string_piece.h" #include "base/strings/string_util.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/search/suggestions/suggestions_service.h" #include "chrome/browser/search/suggestions/suggestions_service_factory.h" #include "chrome/common/url_constants.h" +#include "components/suggestions/suggestions_service.h" #include "net/base/escape.h" #include "ui/gfx/codec/png_codec.h" #include "ui/gfx/image/image_skia.h" diff --git a/chrome/browser/search/suggestions/suggestions_source.h b/chrome/browser/search/suggestions/suggestions_source.h index f3cd4a5..ec6a8b7 100644 --- a/chrome/browser/search/suggestions/suggestions_source.h +++ b/chrome/browser/search/suggestions/suggestions_source.h @@ -11,7 +11,7 @@ #include "base/basictypes.h" #include "base/callback.h" #include "base/memory/weak_ptr.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" +#include "components/suggestions/proto/suggestions.pb.h" #include "content/public/browser/url_data_source.h" #include "url/gurl.h" diff --git a/chrome/browser/search/suggestions/thumbnail_manager.h b/chrome/browser/search/suggestions/thumbnail_manager.h index 948931a..9d45aa7 100644 --- a/chrome/browser/search/suggestions/thumbnail_manager.h +++ b/chrome/browser/search/suggestions/thumbnail_manager.h @@ -15,9 +15,9 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h" -#include "chrome/browser/search/suggestions/image_manager.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" #include "components/leveldb_proto/proto_database.h" +#include "components/suggestions/image_manager.h" +#include "components/suggestions/proto/suggestions.pb.h" #include "ui/gfx/image/image_skia.h" #include "url/gurl.h" diff --git a/chrome/browser/search/suggestions/thumbnail_manager_browsertest.cc b/chrome/browser/search/suggestions/thumbnail_manager_browsertest.cc index c3f24638..2269634 100644 --- a/chrome/browser/search/suggestions/thumbnail_manager_browsertest.cc +++ b/chrome/browser/search/suggestions/thumbnail_manager_browsertest.cc @@ -7,12 +7,12 @@ #include "base/files/file_path.h" #include "base/run_loop.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" #include "chrome/browser/search/suggestions/thumbnail_manager.h" #include "chrome/browser/ui/browser.h" #include "chrome/test/base/in_process_browser_test.h" #include "components/leveldb_proto/proto_database.h" #include "components/leveldb_proto/testing/fake_db.h" +#include "components/suggestions/proto/suggestions.pb.h" #include "content/public/test/test_utils.h" #include "net/base/load_flags.h" #include "net/test/spawned_test_server/spawned_test_server.h" diff --git a/chrome/browser/search/suggestions/thumbnail_manager_unittest.cc b/chrome/browser/search/suggestions/thumbnail_manager_unittest.cc index 7182dd8..dd0543b 100644 --- a/chrome/browser/search/suggestions/thumbnail_manager_unittest.cc +++ b/chrome/browser/search/suggestions/thumbnail_manager_unittest.cc @@ -5,11 +5,11 @@ #include <string> #include "base/memory/scoped_ptr.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" #include "chrome/browser/search/suggestions/thumbnail_manager.h" #include "chrome/test/base/testing_profile.h" #include "components/leveldb_proto/proto_database.h" #include "components/leveldb_proto/testing/fake_db.h" +#include "components/suggestions/proto/suggestions.pb.h" #include "content/public/test/test_browser_thread_bundle.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 06635da..5093cf4 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1142,17 +1142,10 @@ 'browser/search/most_visited_iframe_source.h', 'browser/search/search.cc', 'browser/search/search.h', - 'browser/search/suggestions/blacklist_store.cc', - 'browser/search/suggestions/blacklist_store.h', - 'browser/search/suggestions/image_manager.h', - 'browser/search/suggestions/suggestions_service.cc', - 'browser/search/suggestions/suggestions_service.h', 'browser/search/suggestions/suggestions_service_factory.cc', 'browser/search/suggestions/suggestions_service_factory.h', 'browser/search/suggestions/suggestions_source.cc', 'browser/search/suggestions/suggestions_source.h', - 'browser/search/suggestions/suggestions_store.cc', - 'browser/search/suggestions/suggestions_store.h', 'browser/search/suggestions/thumbnail_manager.cc', 'browser/search/suggestions/thumbnail_manager.h', 'browser/search_engines/chrome_template_url_service_client.cc', @@ -2860,7 +2853,6 @@ 'common_net', 'in_memory_url_index_cache_proto', 'probe_message_proto', - 'suggestions_proto', '../components/components.gyp:autocomplete', '../components/components.gyp:autofill_core_browser', '../components/components.gyp:bookmarks_browser', @@ -2894,6 +2886,7 @@ '../components/components.gyp:search', '../components/components.gyp:search_engines', '../components/components.gyp:search_provider_logos', + '../components/components.gyp:suggestions', '../components/components.gyp:signin_core_browser', '../components/components.gyp:startup_metric_utils', '../components/components.gyp:sync_driver', @@ -3459,18 +3452,6 @@ 'includes': [ '../build/protoc.gypi', ], }, { - # Protobuf compiler / generator for the suggestions service proto. - # GN version: //chrome/browser/search/suggestions/proto - 'target_name': 'suggestions_proto', - 'type': 'static_library', - 'sources': [ 'browser/search/suggestions/proto/suggestions.proto', ], - 'variables': { - 'proto_in_dir': 'browser/search/suggestions/proto', - 'proto_out_dir': 'chrome/browser/search/suggestions/proto', - }, - 'includes': [ '../build/protoc.gypi', ], - }, - { # Protobuf compiler / generator for Probe Message. # GN version: //chrome/browser/net:probe_message_proto 'target_name': 'probe_message_proto', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index e1fc9e0..7d5a14e 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1274,9 +1274,6 @@ 'browser/search/most_visited_iframe_source_unittest.cc', 'browser/search/search_android_unittest.cc', 'browser/search/search_unittest.cc', - 'browser/search/suggestions/blacklist_store_unittest.cc', - 'browser/search/suggestions/suggestions_service_unittest.cc', - 'browser/search/suggestions/suggestions_store_unittest.cc', 'browser/search/suggestions/thumbnail_manager_unittest.cc', 'browser/search_engines/default_search_pref_migration_unittest.cc', 'browser/search_engines/search_provider_install_data_unittest.cc', diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 63ff260..14bb5b5 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -1219,14 +1219,6 @@ extern const char kEasyUnlockPairing[] = "easy_unlock.pairing"; // A cache of zero suggest results using JSON serialized into a string. const char kZeroSuggestCachedResults[] = "zerosuggest.cachedresults"; -// A cache of suggestions represented as a serialized SuggestionsProfile -// protobuf. -const char kSuggestionsData[] = "suggestions.data"; - -// A cache of a suggestions blacklist, represented as a serialized -// SuggestionsBlacklist protobuf. -const char kSuggestionsBlacklist[] = "suggestions.blacklist"; - // *************** LOCAL STATE *************** // These are attached to the machine/installation diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index eca4a3a..72ef8b6 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -392,9 +392,6 @@ extern const char kEasyUnlockPairing[]; extern const char kZeroSuggestCachedResults[]; -extern const char kSuggestionsData[]; -extern const char kSuggestionsBlacklist[]; - // Local state prefs. Please add Profile prefs above instead. extern const char kCertRevocationCheckingEnabled[]; extern const char kCertRevocationCheckingRequiredLocalAnchors[]; diff --git a/components/components.gyp b/components/components.gyp index 6def0a0..c14de82 100644 --- a/components/components.gyp +++ b/components/components.gyp @@ -49,6 +49,7 @@ 'search_provider_logos.gypi', 'signin.gypi', 'startup_metric_utils.gypi', + 'suggestions.gypi', 'translate.gypi', 'url_fixer.gypi', 'url_matcher.gypi', diff --git a/components/components_tests.gyp b/components/components_tests.gyp index e0117f9..b031db5 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -186,6 +186,9 @@ 'storage_monitor/storage_monitor_mac_unittest.mm', 'storage_monitor/storage_monitor_unittest.cc', 'storage_monitor/storage_monitor_win_unittest.cc', + 'suggestions/blacklist_store_unittest.cc', + 'suggestions/suggestions_service_unittest.cc', + 'suggestions/suggestions_store_unittest.cc', 'sync_driver/non_ui_data_type_controller_unittest.cc', 'sync_driver/data_type_manager_impl_unittest.cc', 'sync_driver/generic_change_processor_unittest.cc', @@ -367,6 +370,9 @@ 'components.gyp:signin_core_browser_test_support', '../google_apis/google_apis.gyp:google_apis_test_support', + # Dependencies of suggestions + 'components.gyp:suggestions', + # Dependencies of sync_driver 'components.gyp:sync_driver_test_support', @@ -767,7 +773,7 @@ 'components.gyp:autofill_content_browser', 'components.gyp:dom_distiller_content', 'components.gyp:dom_distiller_core', - 'components.gyp:pref_registry_test_support', + 'components.gyp:pref_registry_test_support', 'components_resources.gyp:components_resources', '../content/content.gyp:content_common', '../content/content.gyp:content_gpu', diff --git a/components/suggestions.gypi b/components/suggestions.gypi new file mode 100644 index 0000000..0dae348 --- /dev/null +++ b/components/suggestions.gypi @@ -0,0 +1,42 @@ +# Copyright 2014 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +{ + 'targets': [ + { + # GN version: //components/suggestions + 'target_name': 'suggestions', + 'type': 'static_library', + 'include_dirs': [ + '..', + ], + 'dependencies': [ + '../base/base.gyp:base', + '../net/net.gyp:net', + '../ui/gfx/gfx.gyp:gfx', + '../url/url.gyp:url_lib', + 'components.gyp:keyed_service_core', + 'components.gyp:pref_registry', + 'components.gyp:variations', + ], + 'sources': [ + 'suggestions/blacklist_store.cc', + 'suggestions/blacklist_store.h', + 'suggestions/image_manager.h', + 'suggestions/proto/suggestions.proto', + 'suggestions/suggestions_pref_names.cc', + 'suggestions/suggestions_pref_names.h', + 'suggestions/suggestions_service.cc', + 'suggestions/suggestions_service.h', + 'suggestions/suggestions_store.cc', + 'suggestions/suggestions_store.h', + ], + 'variables': { + 'proto_in_dir': 'suggestions/proto', + 'proto_out_dir': 'components/suggestions/proto', + }, + 'includes': [ '../build/protoc.gypi' ], + }, + ], +} diff --git a/components/suggestions/BUILD.gn b/components/suggestions/BUILD.gn new file mode 100644 index 0000000..1fe2ff9 --- /dev/null +++ b/components/suggestions/BUILD.gn @@ -0,0 +1,28 @@ +# Copyright 2014 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +static_library("suggestions") { + sources = [ + "blacklist_store.cc", + "blacklist_store.h", + "image_manager.h", + "suggestions_pref_names.cc", + "suggestions_pref_names.h", + "suggestions_service.cc", + "suggestions_service.h", + "suggestions_store.cc", + "suggestions_store.h", + ] + + deps = [ + "//base", + "//components/keyed_service/core", + "//components/pref_registry", + "//components/suggestions/proto", + "//components/variations", + "//net", + "//ui/gfx", + "//url", + ] +} diff --git a/components/suggestions/DEPS b/components/suggestions/DEPS new file mode 100644 index 0000000..e3304f3 --- /dev/null +++ b/components/suggestions/DEPS @@ -0,0 +1,8 @@ +include_rules = [ + "+components/keyed_service/core", + "+components/pref_registry", + "+components/variations", + "+net", + "+ui", + "+url", +] diff --git a/components/suggestions/OWNERS b/components/suggestions/OWNERS new file mode 100644 index 0000000..07a31a62 --- /dev/null +++ b/components/suggestions/OWNERS @@ -0,0 +1 @@ +mathp@chromium.org diff --git a/chrome/browser/search/suggestions/blacklist_store.cc b/components/suggestions/blacklist_store.cc index 8520972..1e92869 100644 --- a/chrome/browser/search/suggestions/blacklist_store.cc +++ b/components/suggestions/blacklist_store.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/search/suggestions/blacklist_store.h" +#include "components/suggestions/blacklist_store.h" #include <set> #include <string> @@ -10,8 +10,8 @@ #include "base/base64.h" #include "base/metrics/histogram.h" #include "base/prefs/pref_service.h" -#include "chrome/common/pref_names.h" #include "components/pref_registry/pref_registry_syncable.h" +#include "components/suggestions/suggestions_pref_names.h" namespace suggestions { diff --git a/chrome/browser/search/suggestions/blacklist_store.h b/components/suggestions/blacklist_store.h index fd4a8a0..072e3f6 100644 --- a/chrome/browser/search/suggestions/blacklist_store.h +++ b/components/suggestions/blacklist_store.h @@ -2,11 +2,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_SEARCH_SUGGESTIONS_BLACKLIST_STORE_H_ -#define CHROME_BROWSER_SEARCH_SUGGESTIONS_BLACKLIST_STORE_H_ +#ifndef COMPONENTS_SUGGESTIONS_BLACKLIST_STORE_H_ +#define COMPONENTS_SUGGESTIONS_BLACKLIST_STORE_H_ #include "base/macros.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" +#include "components/suggestions/proto/suggestions.pb.h" #include "url/gurl.h" class PrefService; @@ -68,4 +68,4 @@ class BlacklistStore { } // namespace suggestions -#endif // CHROME_BROWSER_SEARCH_SUGGESTIONS_BLACKLIST_STORE_H_ +#endif // COMPONENTS_SUGGESTIONS_BLACKLIST_STORE_H_ diff --git a/chrome/browser/search/suggestions/blacklist_store_unittest.cc b/components/suggestions/blacklist_store_unittest.cc index b449ccf..399875e 100644 --- a/chrome/browser/search/suggestions/blacklist_store_unittest.cc +++ b/components/suggestions/blacklist_store_unittest.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/search/suggestions/blacklist_store.h" +#include "components/suggestions/blacklist_store.h" #include <set> #include <string> @@ -11,9 +11,8 @@ #include "base/metrics/histogram_samples.h" #include "base/metrics/statistics_recorder.h" #include "base/test/statistics_delta_reader.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" #include "components/pref_registry/testing_pref_service_syncable.h" -#include "content/public/test/test_browser_thread_bundle.h" +#include "components/suggestions/proto/suggestions.pb.h" #include "testing/gtest/include/gtest/gtest.h" using user_prefs::TestingPrefServiceSyncable; @@ -52,10 +51,27 @@ void ValidateSuggestions(const SuggestionsProfile& expected, } // namespace -TEST(BlacklistStoreTest, BasicInteractions) { - TestingPrefServiceSyncable prefs; - BlacklistStore::RegisterProfilePrefs(prefs.registry()); - BlacklistStore blacklist_store(&prefs); +class BlacklistStoreTest : public testing::Test { + public: + BlacklistStoreTest() + : pref_service_(new user_prefs::TestingPrefServiceSyncable) {} + + virtual void SetUp() OVERRIDE { + BlacklistStore::RegisterProfilePrefs(pref_service()->registry()); + } + + user_prefs::TestingPrefServiceSyncable* pref_service() { + return pref_service_.get(); + } + + private: + scoped_ptr<user_prefs::TestingPrefServiceSyncable> pref_service_; + + DISALLOW_COPY_AND_ASSIGN(BlacklistStoreTest); +}; + +TEST_F(BlacklistStoreTest, BasicInteractions) { + BlacklistStore blacklist_store(pref_service()); // Create suggestions with A, B and C. C and D will be added to the blacklist. std::set<std::string> suggested_urls; @@ -87,25 +103,19 @@ TEST(BlacklistStoreTest, BasicInteractions) { ValidateSuggestions(original_suggestions, suggestions); } -TEST(BlacklistStoreTest, BlacklistTwiceSuceeds) { - TestingPrefServiceSyncable prefs; - BlacklistStore::RegisterProfilePrefs(prefs.registry()); - BlacklistStore blacklist_store(&prefs); +TEST_F(BlacklistStoreTest, BlacklistTwiceSuceeds) { + BlacklistStore blacklist_store(pref_service()); EXPECT_TRUE(blacklist_store.BlacklistUrl(GURL(kTestUrlA))); EXPECT_TRUE(blacklist_store.BlacklistUrl(GURL(kTestUrlA))); } -TEST(BlacklistStoreTest, RemoveUnknownUrlSucceeds) { - TestingPrefServiceSyncable prefs; - BlacklistStore::RegisterProfilePrefs(prefs.registry()); - BlacklistStore blacklist_store(&prefs); +TEST_F(BlacklistStoreTest, RemoveUnknownUrlSucceeds) { + BlacklistStore blacklist_store(pref_service()); EXPECT_TRUE(blacklist_store.RemoveUrl(GURL(kTestUrlA))); } -TEST(BlacklistStoreTest, GetFirstUrlFromBlacklist) { - TestingPrefServiceSyncable prefs; - BlacklistStore::RegisterProfilePrefs(prefs.registry()); - BlacklistStore blacklist_store(&prefs); +TEST_F(BlacklistStoreTest, GetFirstUrlFromBlacklist) { + BlacklistStore blacklist_store(pref_service()); // Expect GetFirstUrlFromBlacklist fails when blacklist empty. GURL retrieved; @@ -122,14 +132,12 @@ TEST(BlacklistStoreTest, GetFirstUrlFromBlacklist) { retrieved_string == std::string(kTestUrlB)); } -TEST(BlacklistStoreLogTest, LogsBlacklistSize) { - content::TestBrowserThreadBundle bundle; +TEST_F(BlacklistStoreTest, LogsBlacklistSize) { base::StatisticsDeltaReader statistics_delta_reader; // Create a first store - blacklist is empty at this point. - TestingPrefServiceSyncable prefs; - BlacklistStore::RegisterProfilePrefs(prefs.registry()); - scoped_ptr<BlacklistStore> blacklist_store(new BlacklistStore(&prefs)); + scoped_ptr<BlacklistStore> blacklist_store( + new BlacklistStore(pref_service())); scoped_ptr<base::HistogramSamples> samples( statistics_delta_reader.GetHistogramSamplesSinceCreation( "Suggestions.LocalBlacklistSize")); @@ -141,7 +149,7 @@ TEST(BlacklistStoreLogTest, LogsBlacklistSize) { EXPECT_TRUE(blacklist_store->BlacklistUrl(GURL(kTestUrlB))); // Create a new BlacklistStore and verify the counts. - blacklist_store.reset(new BlacklistStore(&prefs)); + blacklist_store.reset(new BlacklistStore(pref_service())); samples = statistics_delta_reader.GetHistogramSamplesSinceCreation( "Suggestions.LocalBlacklistSize"); EXPECT_EQ(2, samples->TotalCount()); diff --git a/chrome/browser/search/suggestions/image_manager.h b/components/suggestions/image_manager.h index 96a07d4..8c09fdf 100644 --- a/chrome/browser/search/suggestions/image_manager.h +++ b/components/suggestions/image_manager.h @@ -2,12 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_MANAGER_H_ -#define CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_MANAGER_H_ +#ifndef COMPONENTS_SUGGESTIONS_IMAGE_MANAGER_H_ +#define COMPONENTS_SUGGESTIONS_IMAGE_MANAGER_H_ #include "base/basictypes.h" #include "base/callback.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" +#include "components/suggestions/proto/suggestions.pb.h" #include "ui/gfx/image/image_skia.h" #include "url/gurl.h" @@ -35,4 +35,4 @@ class ImageManager { } // namespace suggestions -#endif // CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_MANAGER_H_ +#endif // COMPONENTS_SUGGESTIONS_IMAGE_MANAGER_H_ diff --git a/chrome/browser/search/suggestions/proto/BUILD.gn b/components/suggestions/proto/BUILD.gn index 1a6018b..1a6018b 100644 --- a/chrome/browser/search/suggestions/proto/BUILD.gn +++ b/components/suggestions/proto/BUILD.gn diff --git a/chrome/browser/search/suggestions/proto/suggestions.proto b/components/suggestions/proto/suggestions.proto index aaf5ef6..aaf5ef6 100644 --- a/chrome/browser/search/suggestions/proto/suggestions.proto +++ b/components/suggestions/proto/suggestions.proto diff --git a/components/suggestions/suggestions_pref_names.cc b/components/suggestions/suggestions_pref_names.cc new file mode 100644 index 0000000..346a33e --- /dev/null +++ b/components/suggestions/suggestions_pref_names.cc @@ -0,0 +1,19 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/suggestions/suggestions_pref_names.h" + +namespace suggestions { +namespace prefs { + +// A cache of a suggestions blacklist, represented as a serialized +// SuggestionsBlacklist protobuf. +const char kSuggestionsBlacklist[] = "suggestions.blacklist"; + +// A cache of suggestions represented as a serialized SuggestionsProfile +// protobuf. +const char kSuggestionsData[] = "suggestions.data"; + +} // namespace prefs +} // namespace suggestions diff --git a/components/suggestions/suggestions_pref_names.h b/components/suggestions/suggestions_pref_names.h new file mode 100644 index 0000000..68ca42e --- /dev/null +++ b/components/suggestions/suggestions_pref_names.h @@ -0,0 +1,19 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_SUGGESTIONS_SUGGESTIONS_PREF_NAMES_H_ +#define COMPONENTS_SUGGESTIONS_SUGGESTIONS_PREF_NAMES_H_ + +namespace suggestions { +namespace prefs { + +// Alphabetical list of preference names specific to the Suggestions +// component. Keep alphabetized, and document each in the .cc file. +extern const char kSuggestionsBlacklist[]; +extern const char kSuggestionsData[]; + +} // namespace prefs +} // namespace suggestions + +#endif // COMPONENTS_SUGGESTIONS_SUGGESTIONS_PREF_NAMES_H_ diff --git a/chrome/browser/search/suggestions/suggestions_service.cc b/components/suggestions/suggestions_service.cc index 60e7dbe..181a3d8 100644 --- a/chrome/browser/search/suggestions/suggestions_service.cc +++ b/components/suggestions/suggestions_service.cc @@ -2,23 +2,23 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/search/suggestions/suggestions_service.h" +#include "components/suggestions/suggestions_service.h" #include <sstream> #include <string> #include "base/memory/scoped_ptr.h" +#include "base/message_loop/message_loop_proxy.h" #include "base/metrics/histogram.h" #include "base/metrics/sparse_histogram.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/time/time.h" -#include "chrome/browser/search/suggestions/blacklist_store.h" -#include "chrome/browser/search/suggestions/suggestions_store.h" #include "components/pref_registry/pref_registry_syncable.h" +#include "components/suggestions/blacklist_store.h" +#include "components/suggestions/suggestions_store.h" #include "components/variations/variations_associated_data.h" #include "components/variations/variations_http_header_provider.h" -#include "content/public/browser/browser_thread.h" #include "net/base/escape.h" #include "net/base/load_flags.h" #include "net/base/net_errors.h" @@ -31,7 +31,6 @@ #include "url/gurl.h" using base::CancelableClosure; -using content::BrowserThread; namespace suggestions { @@ -156,7 +155,7 @@ bool SuggestionsService::IsControlGroup() { void SuggestionsService::FetchSuggestionsData( SuggestionsService::ResponseCallback callback) { - DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + DCHECK(thread_checker_.CalledOnValidThread()); FetchSuggestionsDataNoTimeout(callback); @@ -164,14 +163,14 @@ void SuggestionsService::FetchSuggestionsData( // after some time. Cancels the previous such task, if one existed. pending_timeout_closure_.reset(new CancelableClosure(base::Bind( &SuggestionsService::OnRequestTimeout, weak_ptr_factory_.GetWeakPtr()))); - BrowserThread::PostDelayedTask( - BrowserThread::UI, FROM_HERE, pending_timeout_closure_->callback(), + base::MessageLoopProxy::current()->PostDelayedTask( + FROM_HERE, pending_timeout_closure_->callback(), base::TimeDelta::FromMilliseconds(request_timeout_ms_)); } void SuggestionsService::FetchSuggestionsDataNoTimeout( SuggestionsService::ResponseCallback callback) { - DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + DCHECK(thread_checker_.CalledOnValidThread()); if (pending_request_.get()) { // Request already exists, so just add requestor to queue. waiting_requestors_.push_back(callback); @@ -193,7 +192,7 @@ void SuggestionsService::GetPageThumbnail( void SuggestionsService::BlacklistURL( const GURL& candidate_url, const SuggestionsService::ResponseCallback& callback) { - DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + DCHECK(thread_checker_.CalledOnValidThread()); waiting_requestors_.push_back(callback); // Blacklist locally, for immediate effect. @@ -204,7 +203,6 @@ void SuggestionsService::BlacklistURL( // If there's an ongoing request, let it complete. if (pending_request_.get()) return; - IssueRequest(BuildBlacklistRequestURL(blacklist_url_prefix_, candidate_url)); } @@ -255,14 +253,13 @@ net::URLFetcher* SuggestionsService::CreateSuggestionsRequest(const GURL& url) { } void SuggestionsService::OnRequestTimeout() { - DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + DCHECK(thread_checker_.CalledOnValidThread()); ServeFromCache(); } void SuggestionsService::OnURLFetchComplete(const net::URLFetcher* source) { - DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_EQ(pending_request_.get(), source); - // We no longer need the timeout closure. Delete it whether or not it has run. // If it hasn't, this cancels it. pending_timeout_closure_.reset(); @@ -346,7 +343,7 @@ void SuggestionsService::FilterAndServe(SuggestionsProfile* suggestions) { } void SuggestionsService::ScheduleBlacklistUpload(bool last_request_successful) { - DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + DCHECK(thread_checker_.CalledOnValidThread()); UpdateBlacklistDelay(last_request_successful); @@ -356,14 +353,14 @@ void SuggestionsService::ScheduleBlacklistUpload(bool last_request_successful) { base::Closure blacklist_cb = base::Bind(&SuggestionsService::UploadOneFromBlacklist, weak_ptr_factory_.GetWeakPtr()); - BrowserThread::PostDelayedTask( - BrowserThread::UI, FROM_HERE, blacklist_cb, + base::MessageLoopProxy::current()->PostDelayedTask( + FROM_HERE, blacklist_cb, base::TimeDelta::FromSeconds(blacklist_delay_sec_)); } } void SuggestionsService::UploadOneFromBlacklist() { - DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + DCHECK(thread_checker_.CalledOnValidThread()); // If there's an ongoing request, let it complete. if (pending_request_.get()) return; @@ -377,7 +374,7 @@ void SuggestionsService::UploadOneFromBlacklist() { } void SuggestionsService::UpdateBlacklistDelay(bool last_request_successful) { - DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + DCHECK(thread_checker_.CalledOnValidThread()); if (last_request_successful) { blacklist_delay_sec_ = kBlacklistDefaultDelaySec; diff --git a/chrome/browser/search/suggestions/suggestions_service.h b/components/suggestions/suggestions_service.h index 635ae5d..ee1ed92 100644 --- a/chrome/browser/search/suggestions/suggestions_service.h +++ b/components/suggestions/suggestions_service.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_SEARCH_SUGGESTIONS_SUGGESTIONS_SERVICE_H_ -#define CHROME_BROWSER_SEARCH_SUGGESTIONS_SUGGESTIONS_SERVICE_H_ +#ifndef COMPONENTS_SUGGESTIONS_SUGGESTIONS_SERVICE_H_ +#define COMPONENTS_SUGGESTIONS_SUGGESTIONS_SERVICE_H_ #include <string> #include <vector> @@ -14,10 +14,11 @@ #include "base/gtest_prod_util.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" +#include "base/threading/thread_checker.h" #include "base/time/time.h" -#include "chrome/browser/search/suggestions/image_manager.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" #include "components/keyed_service/core/keyed_service.h" +#include "components/suggestions/image_manager.h" +#include "components/suggestions/proto/suggestions.pb.h" #include "net/url_request/url_fetcher_delegate.h" #include "ui/gfx/image/image_skia.h" #include "url/gurl.h" @@ -49,10 +50,11 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate { public: typedef base::Callback<void(const SuggestionsProfile&)> ResponseCallback; - SuggestionsService(net::URLRequestContextGetter* url_request_context, - scoped_ptr<SuggestionsStore> suggestions_store, - scoped_ptr<ImageManager> thumbnail_manager, - scoped_ptr<BlacklistStore> blacklist_store); + SuggestionsService( + net::URLRequestContextGetter* url_request_context, + scoped_ptr<SuggestionsStore> suggestions_store, + scoped_ptr<ImageManager> thumbnail_manager, + scoped_ptr<BlacklistStore> blacklist_store); virtual ~SuggestionsService(); // Whether this service is enabled. @@ -135,6 +137,8 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate { int blacklist_delay() const { return blacklist_delay_sec_; } void set_blacklist_delay(int delay) { blacklist_delay_sec_ = delay; } + base::ThreadChecker thread_checker_; + // The cache for the suggestions. scoped_ptr<SuggestionsStore> suggestions_store_; @@ -182,4 +186,4 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate { } // namespace suggestions -#endif // CHROME_BROWSER_SEARCH_SUGGESTIONS_SUGGESTIONS_SERVICE_H_ +#endif // COMPONENTS_SUGGESTIONS_SUGGESTIONS_SERVICE_H_ diff --git a/chrome/browser/search/suggestions/suggestions_service_unittest.cc b/components/suggestions/suggestions_service_unittest.cc index 395eb4d..0d082ea 100644 --- a/chrome/browser/search/suggestions/suggestions_service_unittest.cc +++ b/components/suggestions/suggestions_service_unittest.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/search/suggestions/suggestions_service.h" +#include "components/suggestions/suggestions_service.h" #include <map> #include <sstream> @@ -10,17 +10,16 @@ #include "base/bind.h" #include "base/memory/scoped_ptr.h" +#include "base/message_loop/message_loop.h" #include "base/metrics/field_trial.h" #include "base/prefs/pref_service.h" #include "base/strings/utf_string_conversions.h" -#include "chrome/browser/search/suggestions/blacklist_store.h" -#include "chrome/browser/search/suggestions/image_manager.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" -#include "chrome/browser/search/suggestions/suggestions_service_factory.h" -#include "chrome/browser/search/suggestions/suggestions_store.h" +#include "components/suggestions/blacklist_store.h" +#include "components/suggestions/image_manager.h" +#include "components/suggestions/proto/suggestions.pb.h" +#include "components/suggestions/suggestions_store.h" #include "components/variations/entropy_provider.h" #include "components/variations/variations_associated_data.h" -#include "content/public/test/test_browser_thread_bundle.h" #include "net/http/http_response_headers.h" #include "net/http/http_status_code.h" #include "net/url_request/test_url_fetcher_factory.h" @@ -148,8 +147,8 @@ class SuggestionsServiceTest : public testing::Test { virtual ~SuggestionsServiceTest() {} virtual void SetUp() OVERRIDE { - request_context_ = - new net::TestURLRequestContextGetter(base::MessageLoopProxy::current()); + request_context_ = new net::TestURLRequestContextGetter( + io_message_loop_.message_loop_proxy()); } // Enables the "ChromeSuggestions.Group1" field trial. @@ -233,20 +232,21 @@ class SuggestionsServiceTest : public testing::Test { &SuggestionsServiceTest::CheckSuggestionsData, base::Unretained(this))); if (!interleaved_requests) - base::MessageLoop::current()->RunUntilIdle(); // Let request complete. + io_message_loop_.RunUntilIdle(); // Let request complete. // Send the request a second time. suggestions_service->FetchSuggestionsDataNoTimeout(base::Bind( &SuggestionsServiceTest::CheckSuggestionsData, base::Unretained(this))); // (Testing only) wait until suggestion fetch is complete. - base::MessageLoop::current()->RunUntilIdle(); + io_message_loop_.RunUntilIdle(); // Ensure that CheckSuggestionsData() ran twice. EXPECT_EQ(2, suggestions_data_check_count_); } protected: + base::MessageLoopForIO io_message_loop_; net::FakeURLFetcherFactory factory_; // Only used if the SuggestionsService is built with mocks. Not owned. MockSuggestionsStore* mock_suggestions_store_; @@ -255,7 +255,6 @@ class SuggestionsServiceTest : public testing::Test { scoped_refptr<net::TestURLRequestContextGetter> request_context_; private: - content::TestBrowserThreadBundle thread_bundle_; scoped_ptr<base::FieldTrialList> field_trial_list_; scoped_refptr<base::FieldTrial> field_trial_; @@ -309,7 +308,7 @@ TEST_F(SuggestionsServiceTest, FetchSuggestionsDataRequestError) { base::Unretained(this))); // (Testing only) wait until suggestion fetch is complete. - base::MessageLoop::current()->RunUntilIdle(); + io_message_loop_.RunUntilIdle(); // Ensure that ExpectEmptySuggestionsProfile ran once. EXPECT_EQ(1, suggestions_empty_data_count_); @@ -344,7 +343,7 @@ TEST_F(SuggestionsServiceTest, FetchSuggestionsDataResponseNotOK) { base::Unretained(this))); // (Testing only) wait until suggestion fetch is complete. - base::MessageLoop::current()->RunUntilIdle(); + io_message_loop_.RunUntilIdle(); // Ensure that ExpectEmptySuggestionsProfile ran once. EXPECT_EQ(1, suggestions_empty_data_count_); @@ -385,7 +384,7 @@ TEST_F(SuggestionsServiceTest, BlacklistURL) { base::Unretained(this))); // (Testing only) wait until blacklist request is complete. - base::MessageLoop::current()->RunUntilIdle(); + io_message_loop_.RunUntilIdle(); // Ensure that CheckSuggestionsData() ran once. EXPECT_EQ(1, suggestions_data_check_count_); @@ -441,7 +440,11 @@ TEST_F(SuggestionsServiceTest, BlacklistURLFails) { net::HTTP_OK, net::URLRequestStatus::SUCCESS); // (Testing only) wait until both requests are complete. + io_message_loop_.RunUntilIdle(); + // ... Other task gets posted to the message loop. base::MessageLoop::current()->RunUntilIdle(); + // ... And completes. + io_message_loop_.RunUntilIdle(); // Ensure that CheckSuggestionsData() ran once. EXPECT_EQ(1, suggestions_data_check_count_); diff --git a/chrome/browser/search/suggestions/suggestions_store.cc b/components/suggestions/suggestions_store.cc index e57569d..1d1147b 100644 --- a/chrome/browser/search/suggestions/suggestions_store.cc +++ b/components/suggestions/suggestions_store.cc @@ -2,14 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/search/suggestions/suggestions_store.h" +#include "components/suggestions/suggestions_store.h" #include <string> #include "base/base64.h" #include "base/prefs/pref_service.h" -#include "chrome/common/pref_names.h" #include "components/pref_registry/pref_registry_syncable.h" +#include "components/suggestions/suggestions_pref_names.h" namespace suggestions { diff --git a/chrome/browser/search/suggestions/suggestions_store.h b/components/suggestions/suggestions_store.h index cda5cd5c..c39f662 100644 --- a/chrome/browser/search/suggestions/suggestions_store.h +++ b/components/suggestions/suggestions_store.h @@ -2,11 +2,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_SEARCH_SUGGESTIONS_SUGGESTIONS_STORE_H_ -#define CHROME_BROWSER_SEARCH_SUGGESTIONS_SUGGESTIONS_STORE_H_ +#ifndef COMPONENTS_SUGGESTIONS_SUGGESTIONS_STORE_H_ +#define COMPONENTS_SUGGESTIONS_SUGGESTIONS_STORE_H_ #include "base/macros.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" +#include "components/suggestions/proto/suggestions.pb.h" class PrefService; @@ -52,4 +52,4 @@ class SuggestionsStore { } // namespace suggestions -#endif // CHROME_BROWSER_SEARCH_SUGGESTIONS_SUGGESTIONS_STORE_H_ +#endif // COMPONENTS_SUGGESTIONS_SUGGESTIONS_STORE_H_ diff --git a/chrome/browser/search/suggestions/suggestions_store_unittest.cc b/components/suggestions/suggestions_store_unittest.cc index 1181171..0893013 100644 --- a/chrome/browser/search/suggestions/suggestions_store_unittest.cc +++ b/components/suggestions/suggestions_store_unittest.cc @@ -2,12 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/search/suggestions/suggestions_store.h" +#include "components/suggestions/suggestions_store.h" -#include "chrome/browser/search/suggestions/proto/suggestions.pb.h" -#include "chrome/test/base/testing_pref_service_syncable.h" +#include "components/pref_registry/testing_pref_service_syncable.h" +#include "components/suggestions/proto/suggestions.pb.h" #include "testing/gtest/include/gtest/gtest.h" +using user_prefs::TestingPrefServiceSyncable; + namespace suggestions { namespace { diff --git a/components/test/run_all_unittests.cc b/components/test/run_all_unittests.cc index 0e6dafa..ca4e064 100644 --- a/components/test/run_all_unittests.cc +++ b/components/test/run_all_unittests.cc @@ -4,6 +4,7 @@ #include "base/bind.h" #include "base/memory/scoped_ptr.h" +#include "base/metrics/statistics_recorder.h" #include "base/path_service.h" #include "base/test/launcher/unit_test_launcher.h" #include "base/test/test_suite.h" @@ -35,6 +36,12 @@ class ComponentsTestSuite : public base::TestSuite { private: virtual void Initialize() OVERRIDE { base::TestSuite::Initialize(); + + // Initialize the histograms subsystem, so that any histograms hit in tests + // are correctly registered with the statistics recorder and can be queried + // by tests. + base::StatisticsRecorder::Initialize(); + #if !defined(OS_IOS) gfx::GLSurface::InitializeOneOffForTests(); #endif |