ProfileSyncService: Handle null SupervisedUserService

chrome/browser/sync/chrome_sync_client.cc

 // Copyright 2015 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 "chrome/browser/sync/chrome_sync_client.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 429 matching lines @@
       return SupervisedUserSettingsServiceFactory::GetForProfile(profile_)->
           AsWeakPtr();
 #if !defined(OS_ANDROID)
     case syncer::SUPERVISED_USERS:
       return SupervisedUserSyncServiceFactory::GetForProfile(profile_)->
           AsWeakPtr();
     case syncer::SUPERVISED_USER_SHARED_SETTINGS:
       return SupervisedUserSharedSettingsServiceFactory::GetForBrowserContext(
           profile_)->AsWeakPtr();
 #endif  // !defined(OS_ANDROID)
-    case syncer::SUPERVISED_USER_WHITELISTS:
-      return SupervisedUserServiceFactory::GetForProfile(profile_)
-          ->GetWhitelistService()
-          ->AsWeakPtr();
+    case syncer::SUPERVISED_USER_WHITELISTS: {
+      // SupervisedUserServiceFactory depends on ProfileSyncServiceFactory

[skym, 2017/05/01 17:24:03]

I'd rephrase this to talk about the dep in the other direction.

// Unlike other types here, ProfileSyncServiceFactory does not declare a
DependsOn the SupervisedUserServiceFactory (in order to avoid circular
dependency), which means we cannot assume it is still alive.

[Marc Treib, 2017/05/02 08:27:36]

+1

[rishiag, 2017/05/05 17:22:02]

Done.

+      // so SupervisedUserService might be destroyed before ProfileSyncService.
+      SupervisedUserService* supervised_user_service =
+          SupervisedUserServiceFactory::GetForProfile(profile_);

[Marc Treib, 2017/05/02 08:27:36]

SupervisedUserServiceFactory::GetForProfile will create the service if it
doesn't exist. I don't know what will happen if this gets called after the SUS
has already been destroyed, but I assume nothing good?
We should probably add a GetForProfileIfExists method on the SUSFactory.

[rishiag, 2017/05/05 17:22:02]

Done.

+      if (supervised_user_service)
+        return supervised_user_service->GetWhitelistService()->AsWeakPtr();
+      return base::WeakPtr<syncer::SyncableService>();
+    }
 #endif  // BUILDFLAG(ENABLE_SUPERVISED_USERS)
     case syncer::ARTICLES: {
       dom_distiller::DomDistillerService* service =
           dom_distiller::DomDistillerServiceFactory::GetForBrowserContext(
               profile_);
       if (service)
         return service->GetSyncableService()->AsWeakPtr();
       return base::WeakPtr<syncer::SyncableService>();
     }
     case syncer::SESSIONS: {
@@ skipping 36 matching lines @@
     case syncer::AUTOFILL:
       return autofill::AutocompleteSyncBridge::FromWebDataService(
           web_data_service_.get());
 #if defined(OS_CHROMEOS)
     case syncer::PRINTERS:
       return chromeos::PrintersManagerFactory::GetForBrowserContext(profile_)
           ->GetSyncBridge()
           ->AsWeakPtr();
 #endif  // defined(OS_CHROMEOS)
     case syncer::TYPED_URLS:
-      // TODO(gangwu):implement TypedURLSyncBridge and return real
+      // TODO(gangwu): Implement TypedURLSyncBridge and return real
       // TypedURLSyncBridge here.
       return base::WeakPtr<syncer::ModelTypeSyncBridge>();
     default:
       NOTREACHED();
       return base::WeakPtr<syncer::ModelTypeSyncBridge>();
   }
 }
 
 scoped_refptr<syncer::ModelSafeWorker>
 ChromeSyncClient::CreateModelWorkerForGroup(syncer::ModelSafeGroup group) {
@@ skipping 185 matching lines @@
   sync_service->RegisterDataTypeController(
       base::MakeUnique<SupervisedUserSyncDataTypeController>(
           syncer::SUPERVISED_USER_SETTINGS, error_callback, this, profile_));
   sync_service->RegisterDataTypeController(
       base::MakeUnique<SupervisedUserSyncDataTypeController>(
           syncer::SUPERVISED_USER_WHITELISTS, error_callback, this, profile_));
 #endif  // BUILDFLAG(ENABLE_SUPERVISED_USERS)
 }
 
 }  // namespace browser_sync