Refactor language detection logic to allow non-static CLD data sources.

components/translate/content/common/cld_data_source.h

Index: components/translate/content/common/cld_data_source.h
diff --git a/components/translate/content/common/cld_data_source.h b/components/translate/content/common/cld_data_source.h
index 07a56b0437a6894c0ff105608f86ff68a9126033..77dea394a2de1b1194d38f962c0c7cb6529c4aed 100644
--- a/components/translate/content/common/cld_data_source.h
+++ b/components/translate/content/common/cld_data_source.h
@@ -7,12 +7,22 @@
 
 #include <string>
 
+#include "base/files/file_path.h"
+#include "base/macros.h"
+#include "base/synchronization/lock.h"
+
 namespace translate {
 
 // Provides high-level functionality related to a CLD Data Source.
 class CldDataSource {

[Takashi Toyoshima, 2014/11/06 14:34:29]

See also my comments on CldDataHarnessFactory.

[Andrew Hayden (chromium.org), 2014/11/10 14:06:35]

Acknowledged.

 
  public:
+  // Generally not used by Chromium code, but available for embedders to
+  // configure additional data sources as subclasses.
+  // Chromium code should use the getters instead (GetStaticDataSource(),
+  // GetStandaloneDataSource(), and GetComponentDataSource()).
+  CldDataSource();
+  virtual ~CldDataSource() {}
 
   // Returns the symbolic name of the data source. In the Chromium
   // open-source tree, the following data sources exist:
@@ -24,24 +34,60 @@ class CldDataSource {
   //              implementations.
   //
   // Other implementations based upon Chromium may provide CLD differently and
-  // may have other names. This method is primarily provided for those
-  // non-Chromium implementations; Chromium implementations should use the
-  // boolean methods in this class instead:
-  // ShouldRegisterForComponentUpdates()
-  // ShouldUseStandaloneDataFile()
-  static std::string GetName();
-
-  // Returns true if the data source needs to receive updates from the
-  // Component Updater.
-  // This is only true if the data source name is "component", but makes caller
-  // logic more generic.
-  static bool ShouldRegisterForComponentUpdates();
-
-  // Returns true if the data source needs to have the path to the CLD
-  // data file configured immediately because it is bundled with Chromium.
-  // This is only true if the data source name is "standalone", but makes
-  // caller logic more generic.
-  static bool ShouldUseStandaloneDataFile();
+  // may have other names.
+  // This method is threadsafe.
+  virtual std::string GetName();
+
+  // For data sources that support a separate CLD data file, configures the path
+  // of that data file.
+  //
+  // The 'component' and 'standalone' data sources need this method to be called
+  // in order to locate the CLD data on disk.
+  // If the data source doesn't need or doesn't support such configuration, this
+  // function is a no-op. This is the case for, e.g., the static data source.
+  // This method is threadsafe.
+  void SetCldDataFilePath(const base::FilePath& path);
+
+  // Returns the path most recently set by SetCldDataFilePath. The initial value
+  // prior to any such call is the empty path. If the data source doesn't
+  // support a data file, returns the empty path.
+  // This method is threadsafe.
+  base::FilePath GetCldDataFilePath();
+
+  // Sets the default data source for this process, i.e. the data source to be
+  // used unless the embedder calls Set(CldDatasource*). This is the method
+  // that normal (i.e., non-test) Chromium code should use; embedders can and
+  // should use the unconditional Set(CldDataSource*) method instead. If a
+  // default data source has already been set, this method does nothing.
+  static void SetDefault(CldDataSource* data_source);

[Takashi Toyoshima, 2014/11/06 14:34:29]

Do you really need both SetDefault() and Set()?

[Andrew Hayden (chromium.org), 2014/11/10 14:06:35]

Yes, for the same reasons we need them everywhere else. The embedder needs to be
able to override the default, but each platform needs to be able to configure a
default on its own.

[Takashi Toyoshima, 2014/11/11 07:16:36]

Now I understand. Thanks.

+
+  // Unconditionally sets the data source for this process, overwriting any
+  // previously-configured default. Normal Chromium code should never use this
+  // method; it is provided for embedders to inject a data source from outside
+  // of the Chromium code base. Test code can also use this method to force the
+  // runtime to have a desired behavior.
+  static void Set(CldDataSource* data_source);
+
+  // Returns the data source for this process. Guaranteed to never be null.
+  // If no instance has been set, this returns the same object obtained by
+  // calling GetStaticDataSource(), which is always safe but may fail to
+  // function if the CLD data is not *actually* statically linked.
+  static CldDataSource* Get();

[Takashi Toyoshima, 2014/11/06 14:34:29]

Can you avoid using these getters for raw object pointers?
I think you just use it like CldDataSource::Get()->GetName(). They can be
replaced with CldDataSource::GetUsingDataSourceName() or something.

[Andrew Hayden (chromium.org), 2014/11/10 14:06:35]

It would be nice, yes, but there would be tradeoffs. GetName(),
SetCldDataFilePath(), GetCldDataFilePath() all are instance methods right now.
To completely hide the pointer, we would have to make them static and they would
internally reference the global instance and set the data upon it. I'm willing
to make the change, but I'm not sure it will actually look much better.
Obviously we have to switch to using Is[type]DataSource like you suggest below.
I'll do this in a separate patchset, stand by.

+
+  // Fetch the global instance of the "static" data source.
+  static CldDataSource* GetStaticDataSource();

[Takashi Toyoshima, 2014/11/06 14:34:29]

Also, if your use case is only "Get() == GetStaticDataSource()" and so on, it's
safe to add "bool IsUsingDataSourceStatic()".

+
+  // Fetch the global instance of the "standalone" data source.
+  static CldDataSource* GetStandaloneDataSource();
+
+  // Fetch the global instance of the "component" data source.
+  static CldDataSource* GetComponentDataSource();
+
+ private:
+  base::FilePath m_cached_filepath;  // Guarded by m_file_lock
+  base::Lock m_file_lock;  // Guards m_cached_filepath
+
+  DISALLOW_COPY_AND_ASSIGN(CldDataSource);
 };
 
 }  // namespace translate