[Sync] Stop accessing BrowserContextKeyedServiceFactory on non-UI thread.

chrome/browser/extensions/api/storage/settings_sync_util.cc

Index: chrome/browser/extensions/api/storage/settings_sync_util.cc
diff --git a/chrome/browser/extensions/api/storage/settings_sync_util.cc b/chrome/browser/extensions/api/storage/settings_sync_util.cc
index e29718c43f4a593c99230337ba1b709fa4e24205..e8fee891fab29471cc4d497f9f2622fb826307cc 100644
--- a/chrome/browser/extensions/api/storage/settings_sync_util.cc
+++ b/chrome/browser/extensions/api/storage/settings_sync_util.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/extensions/api/storage/settings_sync_util.h"
 
+#include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/api/storage/sync_value_store_cache.h"
@@ -13,7 +14,10 @@
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/api/storage/storage_frontend.h"
 
+using base::WeakPtr;
 using content::BrowserThread;
+using syncer::ModelType;
+using syncer::SyncableService;
 
 namespace extensions {
 
@@ -44,13 +48,20 @@ void PopulateAppSettingSpecifics(
       extension_id, key, value, specifics->mutable_extension_setting());
 }
 
+WeakPtr<SyncableService> GetSyncableService(WeakPtr<SyncValueStoreCache> cache,
+                                            ModelType type) {
+  // WeakPtr must be explicitly checked instead of using Bind magic because we

[pavely, 2017/03/27 19:38:25]

I don't think there is any Bind magic here. If cache was invalid,
cache->GetSyncableService() would DCHECK or SEGFAULT.

[skym, 2017/03/27 21:45:26]

Removed the confusing comment.

+  // need to return a default value if the WeakPtr doesn't hold anything.
+  return cache.get() ? cache->GetSyncableService(type)->AsWeakPtr()
+                     : WeakPtr<SyncableService>();
+}
+
 }  // namespace
 
-syncer::SyncData CreateData(
-    const std::string& extension_id,
-    const std::string& key,
-    const base::Value& value,
-    syncer::ModelType type) {
+syncer::SyncData CreateData(const std::string& extension_id,
+                            const std::string& key,
+                            const base::Value& value,
+                            ModelType type) {
   sync_pb::EntitySpecifics specifics;
   switch (type) {
     case syncer::EXTENSION_SETTINGS:
@@ -77,32 +88,29 @@ syncer::SyncData CreateData(
       extension_id + "/" + key, key, specifics);
 }
 
-syncer::SyncChange CreateAdd(
-    const std::string& extension_id,
-    const std::string& key,
-    const base::Value& value,
-    syncer::ModelType type) {
+syncer::SyncChange CreateAdd(const std::string& extension_id,
+                             const std::string& key,
+                             const base::Value& value,
+                             ModelType type) {
   return syncer::SyncChange(
       FROM_HERE,
       syncer::SyncChange::ACTION_ADD,
       CreateData(extension_id, key, value, type));
 }
 
-syncer::SyncChange CreateUpdate(
-    const std::string& extension_id,
-    const std::string& key,
-    const base::Value& value,
-    syncer::ModelType type) {
+syncer::SyncChange CreateUpdate(const std::string& extension_id,
+                                const std::string& key,
+                                const base::Value& value,
+                                ModelType type) {
   return syncer::SyncChange(
       FROM_HERE,
       syncer::SyncChange::ACTION_UPDATE,
       CreateData(extension_id, key, value, type));
 }
 
-syncer::SyncChange CreateDelete(
-    const std::string& extension_id,
-    const std::string& key,
-    syncer::ModelType type) {
+syncer::SyncChange CreateDelete(const std::string& extension_id,
+                                const std::string& key,
+                                ModelType type) {
   base::DictionaryValue no_value;
   return syncer::SyncChange(
       FROM_HERE,
@@ -110,14 +118,15 @@ syncer::SyncChange CreateDelete(
       CreateData(extension_id, key, no_value, type));
 }
 
-syncer::SyncableService* GetSyncableService(content::BrowserContext* context,
-                                            syncer::ModelType type) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+syncer::SyncClient::ServiceProvider GetSyncableServiceProvider(
+    content::BrowserContext* context,
+    ModelType type) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(type == syncer::APP_SETTINGS || type == syncer::EXTENSION_SETTINGS);
   StorageFrontend* frontend = StorageFrontend::Get(context);
   SyncValueStoreCache* sync_cache = static_cast<SyncValueStoreCache*>(
       frontend->GetValueStoreCache(settings_namespace::SYNC));
-  return sync_cache->GetSyncableService(type);
+  return base::Bind(&GetSyncableService, sync_cache->AsWeakPtr(), type);
 }
 
 }  // namespace settings_sync_util