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

components/translate/content/browser/browser_cld_data_provider_factory.cc

+// 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/translate/content/browser/browser_cld_data_provider_factory.h"
+
+#include "base/lazy_instance.h"
+#include "components/translate/content/browser/browser_cld_data_provider.h"
+
+namespace {
+translate::BrowserCldDataProviderFactory* g_instance = NULL;
+

[picksi1, 2014/09/29 15:59:08]

This looks familiar! Is there a common pattern here & in the earlier use that
can be pulled out somehow?

[Andrew Hayden (chromium.org), 2014/10/30 14:25:15]

Similarly, this is now cleaned up.

+// The default factory
+struct WrappedDefault {
+  WrappedDefault() {
+    value = new translate::BrowserCldDataProviderFactory();
+  }
+  translate::BrowserCldDataProviderFactory* value;
+};
+base::LazyInstance<WrappedDefault>::Leaky g_wrapped_default =
+    LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
+
+namespace translate {
+
+BrowserCldDataProviderFactory::BrowserCldDataProviderFactory() {
+}
+
+BrowserCldDataProviderFactory::~BrowserCldDataProviderFactory() {
+}
+
+BrowserCldDataProvider* BrowserCldDataProviderFactory::NewProvider(
+      content::WebContents* web_contents) {
+  return new BrowserCldDataProvider();
+}
+
+bool BrowserCldDataProviderFactory::IsInitialized() {
+  return g_instance != NULL;
+}
+
+void BrowserCldDataProviderFactory::Set(
+    BrowserCldDataProviderFactory* instance, bool overwrite) {
+  if (overwrite || g_instance == NULL) {
+    g_instance = instance;

[picksi1, 2014/09/29 15:59:08]

Now I understand what the bool does! It might be better to have two functions,
one called 'SetFactoryInstance' (?) and one called 'SetFactoryInstanceIfNull'(?)
to be explicit.

Also might be more readable/future proof to use IsInitialized() instead of
checking g_instance directly.

[Andrew Hayden (chromium.org), 2014/10/30 14:25:15]

Done

+  }
+}
+
+BrowserCldDataProviderFactory* BrowserCldDataProviderFactory::Get() {
+  if (g_instance == NULL) {
+    // No provider set. OK for test code and narrow use cases, but bad if it is
+    // a real browser. Since there's no good way to differentiate these use
+    // cases at runtime, don't log a warning here.
+    return g_wrapped_default.Get().value;

[picksi1, 2014/09/29 15:59:08]

Can we lazily initialize (to a default?) in here?

[Andrew Hayden (chromium.org), 2014/10/30 14:25:15]

That's what we're doing. Cleaner now.

+  }
+  return g_instance;
+}
+
+}  // namespace translate