Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4415)

Unified Diff: chrome/browser/translate/cld_data_harness.h

Issue 461633002: Refactor language detection logic to allow non-static CLD data sources. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Make some of the harness factory methods private Created 6 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/translate/chrome_translate_client.cc ('k') | chrome/browser/translate/cld_data_harness.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/translate/cld_data_harness.h
diff --git a/chrome/browser/translate/cld_data_harness.h b/chrome/browser/translate/cld_data_harness.h
index e9b6f3210c3bdfa1ebc32ce8eb252f27c37f5b56..1ca3cb5e0a35d663436499d9b60b75ece2b2fc20 100644
--- a/chrome/browser/translate/cld_data_harness.h
+++ b/chrome/browser/translate/cld_data_harness.h
@@ -12,16 +12,24 @@
namespace test {
// A utility class that sets up CLD dynamic data upon calling Init() and cleans
-// it up when destroyed.
+// it up when destroyed. Note that the Init() method will also configure the
+// CLD data source using translate::CldDataSource::Set(), overriding any
+// previously-set value unconditionally!
+//
// Test data lives under: src/chrome/test/data/cld2_component
//
// This class is intended to be instantiated within IN_PROC_BROWSER_TEST_F
// test fixtures; it uses ASSERT macros for correctness, so that tests will
-// fail gracefully in error conditions. Sample use:
+// fail gracefully in error conditions. Test code should generally use a
+// CldDataHarnessFactory to create CldDataHarness objects since this allows
+// the tests to run with whatever configuration is appropriate for the platform;
+// If that's not enough power, the testing code can set the factory itself.
+//
+// Sample usage:
//
// IN_PROC_BROWSER_TEST_F(BrowserTest, PageLanguageDetection) {
// scoped_ptr<test::CldDataHarness> cld_data_scope =
-// test::CreateCldDataHarness();
+// test::CldDataHarnessFactory::Get()->CreateCldDataHarness();
// ASSERT_NO_FATAL_FAILURE(cld_data_scope->Init());
// // ... your code that depends on language detection goes here
// }
@@ -34,7 +42,9 @@ namespace test {
// class MyTestClass : public InProcessBrowserTest {
// public:
// MyTestClass() :
-// cld_data_scope(test::CreateCldDataHarness()) {
+// cld_data_scope(
+// test::CldDataHarnessFactory::Get->CreateCldDataHarness()) {
+// // (your additional setup code here)
// }
// virtual void SetUpOnMainThread() override {
// cld_data_scope->Init();
@@ -46,16 +56,39 @@ namespace test {
//
class CldDataHarness {
public:
+ CldDataHarness() {}
+
// Reverses the work done by the Init() method: any files and/or directories
// that would be created by Init() (whether it was called or not) are
// immediately deleted.
// If dynamic data is not currently available for any reason, this method has
// no effect.
+ // The default implementation does nothing.
virtual ~CldDataHarness() {}
// Call this method, wrapping it in ASSERT_NO_FATAL_FAILURE, to initialize
// the harness and trigger test failure if initialization fails.
- virtual void Init() = 0;
+ // IMPORTANT: This method will unconditionally set the CLD data source using
+ // translate::CldDataSource::Set(...). Any previously-configured CLD data
+ // source will be lost. This helps ensure a consistent test environment where
+ // the configured data source matches definitely matches the harness.
+ virtual void Init() {}
+
+ // Create and return a new instance of a data harness whose Init() method
+ // will configure the "static" CldDataSource.
+ static scoped_ptr<CldDataHarness> CreateStaticDataHarness();
+
+ // Create and return a new instance of a data harness whose Init() method
+ // will configure the "standalone" CldDataSource.
+ // Unlike NONE() and STATIC(), this data hardness will perform work to allow
+ // CLD to load data from a file.
+ static scoped_ptr<CldDataHarness> CreateStandaloneDataHarness();
+
+ // Create and return a new instance of a data harness whose Init() method
+ // will configure the "component" CldDataSource.
+ // Unlike NONE() and STATIC(), this data hardness will perform work to allow
+ // CLD to load data from a file.
+ static scoped_ptr<CldDataHarness> CreateComponentDataHarness();
protected:
// Returns the version number of the Component Updater "extension" in the
@@ -69,11 +102,10 @@ class CldDataHarness {
// directory. Within, there is a real copy of the CLD2 dynamic data that can
// be used in testing scenarios without accessing the network.
void GetTestDataSourceDirectory(base::FilePath* out_path);
-};
-// Static factory function that returns a data harness defined by the
-// implementation, which must be a subclass of CldDataHarness.
-scoped_ptr<CldDataHarness> CreateCldDataHarness();
+ private:
+ DISALLOW_COPY_AND_ASSIGN(CldDataHarness);
+};
} // namespace test
« no previous file with comments | « chrome/browser/translate/chrome_translate_client.cc ('k') | chrome/browser/translate/cld_data_harness.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698