wifi_sync: allow WifiCredentialSyncableServiceFactory to ignore LoginState

WifiCredentialSyncableServiceFactory normally uses
chromeos::LoginState to associate a SyncableService with a Shill
profile. For this to work, LoginState must have been set properly,
before someone tries to use the Factory to create a SyncableService.

The precondition is met in production mode. However, in sync
integration tests, it is difficult to arrange for LoginState to be
configured properly. The problem is that the sync integration test
framework creates multiple SyncableServices, without providing the
test case the ability to change configuration between the construction
of the SyncableServices.

Acccomodate the way sync integration test work, by allowing the
WifiCredentialSyncableServiceFactory to ignore LoginState. When
LoginState is ignored, the factory will, instead, use the
BrowserContext to associate the new SyncableService with a Shill
profile. This works for sync integration tests, because each
SyncableService created by the test framework has a separate
BrowserContext.

BUG=chromium:431435
TEST=components_unittests --gtest_filter="Wifi*"

Review URL: https://codereview.chromium.org/876833002

Cr-Commit-Position: refs/heads/master@{#314981}

components/wifi_sync/DEPS

@@ -5,6 +5,7 @@ include_rules = [
   "+components/keyed_service/content",
   "+components/keyed_service/core",
   "+components/onc",
+  "+content/public/browser",
   "+sync/api",
   "+sync/internal_api/public/attachments",
   "+sync/protocol",

components/wifi_sync/wifi_credential_syncable_service_factory.cc

@@ -4,12 +4,17 @@
 
 #include "components/wifi_sync/wifi_credential_syncable_service_factory.h"
 
+#include <string>
+
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/wifi_sync/wifi_config_delegate.h"
 #include "components/wifi_sync/wifi_credential_syncable_service.h"
+#include "content/public/browser/browser_context.h"
 
 #if defined(OS_CHROMEOS)
+#include "base/files/file_path.h"
 #include "chromeos/login/login_state.h"
 #include "chromeos/network/network_handler.h"
 #include "components/wifi_sync/wifi_config_delegate_chromeos.h"
@@ -19,26 +24,28 @@ namespace wifi_sync {
 
 namespace {
 
-scoped_ptr<WifiConfigDelegate> BuildConfigDelegate(
-    content::BrowserContext* context) {
 #if defined(OS_CHROMEOS)
-  const chromeos::LoginState* login_state = chromeos::LoginState::Get();
-  DCHECK(login_state->IsUserLoggedIn());
-  DCHECK(!login_state->primary_user_hash().empty());
-  // TODO(quiche): Verify that |context| is the primary user's context.
-
-  // Note: NetworkHandler is a singleton that is managed by
-  // ChromeBrowserMainPartsChromeos, and destroyed after all
-  // KeyedService instances are destroyed.
-  chromeos::NetworkHandler* network_handler = chromeos::NetworkHandler::Get();
-  return make_scoped_ptr(new WifiConfigDelegateChromeOs(
-      login_state->primary_user_hash(),
-      network_handler->managed_network_configuration_handler()));
-#else
-  NOTREACHED();
-  return nullptr;
-#endif
+// Returns a string identifying a ChromeOS network settings profile,
+// by that profile's UserHash property. This value may be communicated
+// to the ChromeOS connection manager ("Shill"), but must not be
+// exposed to any untrusted code (e.g., via web APIs).
+std::string GetUserHash(content::BrowserContext* context,
+                        bool use_login_state) {
+  if (use_login_state) {
+    const chromeos::LoginState* login_state = chromeos::LoginState::Get();
+    DCHECK(login_state->IsUserLoggedIn());
+    DCHECK(!login_state->primary_user_hash().empty());
+    // TODO(quiche): Verify that |context| is the primary user's context.
+    return login_state->primary_user_hash();
+  } else {
+    // In WiFi credential sync tests, LoginState is not
+    // available. Instead, those tests set their Shill profiles'
+    // UserHashes based on the corresponding BrowserContexts' storage
+    // paths.
+    return context->GetPath().BaseName().value();
+  }
 }
+#endif
 
 }  // namespace
 
@@ -71,7 +78,27 @@ KeyedService* WifiCredentialSyncableServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   // TODO(quiche): Figure out if this behaves properly for multi-profile.
   // crbug.com/430681.
-  return new WifiCredentialSyncableService(BuildConfigDelegate(context));
+#if defined(OS_CHROMEOS)
+  return new WifiCredentialSyncableService(
+      BuildWifiConfigDelegateChromeOs(context));
+#else
+  NOTREACHED();
+  return nullptr;
+#endif
+}
+
+#if defined(OS_CHROMEOS)
+scoped_ptr<WifiConfigDelegate>
+WifiCredentialSyncableServiceFactory::BuildWifiConfigDelegateChromeOs(
+    content::BrowserContext* context) const {
+  // Note: NetworkHandler is a singleton that is managed by
+  // ChromeBrowserMainPartsChromeos, and destroyed after all
+  // KeyedService instances are destroyed.
+  chromeos::NetworkHandler* network_handler = chromeos::NetworkHandler::Get();
+  return make_scoped_ptr(new WifiConfigDelegateChromeOs(
+      GetUserHash(context, !ignore_login_state_for_test_),
+      network_handler->managed_network_configuration_handler()));
 }
+#endif
 
 }  // namespace wifi_sync

components/wifi_sync/wifi_credential_syncable_service_factory.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_WIFI_SYNC_WIFI_CREDENTIAL_SYNCABLE_SERVICE_FACTORY_H_
 
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "components/keyed_service/content/browser_context_keyed_service_factory.h"
 
@@ -15,6 +16,7 @@ class BrowserContext;
 
 namespace wifi_sync {
 
+class WifiConfigDelegate;
 class WifiCredentialSyncableService;
 
 // Singleton that owns all WifiCredentialSyncableServices and
@@ -29,12 +31,15 @@ class WifiCredentialSyncableServiceFactory
   static WifiCredentialSyncableService* GetForBrowserContext(
       content::BrowserContext* browser_context);
 
-  // Returns the singleton instance. As this class has no public
-  // instance methods, this function is not generally useful for
-  // external callers. This function is public only so that the
-  // Singleton template can reference it.
+  // Returns the singleton instance.
   static WifiCredentialSyncableServiceFactory* GetInstance();
 
+#if defined(OS_CHROMEOS)
+  void set_ignore_login_state_for_test(bool new_value) {
+    ignore_login_state_for_test_ = new_value;
+  }
+#endif
+
  private:
   friend struct DefaultSingletonTraits<WifiCredentialSyncableServiceFactory>;
 
@@ -45,6 +50,22 @@ class WifiCredentialSyncableServiceFactory
   KeyedService* BuildServiceInstanceFor(
       content::BrowserContext* context) const override;
 
+#if defined(OS_CHROMEOS)
+  // Returns a scoped pointer to a WifiConfigDelegate, which can be
+  // used to configure the ChromeOS Wi-Fi settings associated with
+  // |context|.
+  scoped_ptr<WifiConfigDelegate> BuildWifiConfigDelegateChromeOs(
+      content::BrowserContext* context) const;
+#endif
+
+#if defined(OS_CHROMEOS)
+  // Whether or not we should use LoginState to associate a new
+  // SyncableService with a Shill profile. Should be set to true in
+  // sync integration tests, where it is not possible to control
+  // LoginState at the time SyncableServices are constructed.
+  bool ignore_login_state_for_test_ = false;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(WifiCredentialSyncableServiceFactory);
 };