SigninManagerBase::IsSigninAllowed is used from the following files:

$ git grep -l '\<IsSigninAllowed\>'|grep -v components/signin
chrome/browser/android/signin/signin_manager_android.cc
chrome/browser/signin/signin_promo_util.cc
chrome/browser/ui/chrome_pages.cc
chrome/browser/ui/views/profiles/profile_chooser_view.cc
chrome/browser/ui/webui/app_launcher_login_handler.cc
chrome/browser/ui/webui/settings/people_handler.cc
chrome/browser/ui/webui/signin/signin_utils_desktop.cc

The preference is referenced from those files:

$ git grep -l '\<prefs::kSigninAllowed\>'|grep -v components/signin
chrome/browser/android/signin/signin_manager_android.cc
chrome/browser/policy/configuration_policy_handler_list_factory.cc
chrome/browser/profiles/profile_manager.cc
chrome/browser/signin/account_consistency_mode_manager.cc
chrome/browser/signin/account_consistency_mode_manager_unittest.cc
chrome/browser/supervised_user/supervised_user_pref_store.cc
chrome/browser/ui/browser_command_controller.cc
chrome/browser/ui/browser_command_controller_unittest.cc
chrome/browser/ui/webui/profile_info_watcher.cc
chrome/browser/ui/webui/settings/people_handler.cc
chrome/browser/ui/webui/signin/dice_turn_sync_on_helper_unittest.cc
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc

Some of those usages are from globals which would make API option 1. difficult to implement.

See also https://bugs.chromium.org/p/chromium/issues/detail?id=889903 for similar API for signing out that already has setter in SigninManager and that is handled in the embedder.

@sdefresne: Are you planning to work on this one yourself anytime soon? Seems to be blocking quite a few other tasks, and I was wondering whether it would make sense for me to work on this one, or if you'd rather do it yourself (and the lack of a Proj-Servicification-VendorBug tag makes me think of the later).

I am currently working on a design for refactoring those APIs. I understand that this is blocking many migration bugs, and thus it is one of my highest priority bug.

> I am currently working on a design for refactoring those APIs. I understand that this is blocking many migration bugs, and thus it is one of my highest priority bug.

Thanks @sdefresne for the heads-up. Looking forward to the new APIs!

After discussing this offline with msarda@, the plan is to move the preference out of SigninManager and to leave the responsibility to the embedder. So we would end up with SetSigninAllowed() only API that would be invoked by an object created by the factory.

So all code in chrome/ should be converted away from SigninManagerBase::IsSigninAllowed() and instead directly use the preference.

So converting the following:

  bool is_signin_allowed =
      SigninManagerFactory::GetForProfile(profile)->IsSigninAllowed()

to:

  bool is_signin_allowed =
      profile->GetPrefs()->GetBoolean(prefs::kSigninAllowed);

Hey Sylvain,

Can you expand on what "fixed" means here? Does it mean that all the blocked bugs can now be converted away from using SigninManager at all via the approach described in c#20? Is there any further work to do on the IdentityManager side? Thanks!

It means that the blocked bug can now be worked on (by converting to use the preference). An API will exists to propagate the bool from the preference to the SigninManager (via PrimaryAccountMutator most likely) and maybe some API to query the value (but more for completeness, using the preference is the recommended way to get to the pref in embedder code).

Oh! I haven't seen this until today, thanks!