wifi_sync: implement ACTION_ADD in WifiCredentialSyncableService

components/wifi_sync/wifi_credential_syncable_service_factory.cc

Index: components/wifi_sync/wifi_credential_syncable_service_factory.cc
diff --git a/components/wifi_sync/wifi_credential_syncable_service_factory.cc b/components/wifi_sync/wifi_credential_syncable_service_factory.cc
index dbc2fec526097c424271b341804ce25433fe412b..451748fce24fb448bf8f72f095787ba4f96f60a5 100644
--- a/components/wifi_sync/wifi_credential_syncable_service_factory.cc
+++ b/components/wifi_sync/wifi_credential_syncable_service_factory.cc
@@ -4,11 +4,49 @@
 
 #include "components/wifi_sync/wifi_credential_syncable_service_factory.h"
 
+#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_credential_syncable_service.h"
 
+#if defined(OS_CHROMEOS)
+#include "chromeos/login/login_state.h"
+#include "chromeos/network/network_handler.h"
+#include "components/wifi_sync/wifi_config_delegate_chromeos.h"
+#endif
+
 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.
+
+  // It is safe for us to pass WifiConfigDelegateChromeOs a raw
+  // pointer to the ManagedNetworkConfigurationHandler instance,
+  // because: 1) The scoped_ptr that we return will be owned by a
+  // KeyedService, 2) KeyedServices are destroyed before the
+  // chromeos::internal::DBusServices singleton, 3) The DBusServices
+  // singleton owns the chromeos::NetworkHandler singleton, and 4) The
+  // NetworkHandler singleton owns the
+  // ManagedNetworkConfigurationHandler singleton.

[stevenjb, 2015/01/14 00:12:49]

This comment is overly complicated. All you need to say is something like:
"Note: NetworkHandler is a singleton that is managed by
ChromeBrowserMainPartsChromeos and destroyed after all KeyedService instances
are destroyed".

(In particular, the relationship between NetworkHandler, DBusServices, and
ChromeBrowserMainPartsChromeos is an external detail that might change at some
point).

[mukesh agrawal, 2015/01/20 21:13:15]

Done.

+  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
+  NOTIMPLEMENTED();

[erikwright (departed), 2015/01/13 19:21:59]

Do you intend for this to CHECK on other platforms? Presumably you don't expect
it to ever be called?

[mukesh agrawal, 2015/01/20 21:13:15]

True -- I don't expect this to be called at all on other platforms. So I've
changed this to NOTREACHED().

(While NOTREACHED is a DCHECK rather than a CHECK, I don't see any reason to
make this particular NOTREACHED a CHECK. Let me know if you disagree.)

+  return nullptr;
+#endif
+}
+
+}  // namespace
+
 // static
 WifiCredentialSyncableService*
 WifiCredentialSyncableServiceFactory::GetForBrowserContext(
@@ -38,7 +76,7 @@ KeyedService* WifiCredentialSyncableServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   // TODO(quiche): Figure out if this behaves properly for multi-profile.
   // crbug.com/430681.
-  return new WifiCredentialSyncableService();
+  return new WifiCredentialSyncableService(BuildConfigDelegate(context));
 }
 
 }  // namespace wifi_sync