Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure only one IdentityManagerFactory instance gets created #8731

Merged
merged 1 commit into from
May 12, 2021

Commits on May 12, 2021

  1. Ensure only one IdentityManagerFactory instance gets created

    IdentityManagerFactory is a class meant to be instantiated in the
    browser process via base::Singleton, thus it's not expected to have
    several instance of them.
    
    However, since PR #7480[1] we're actually creating two instances for
    Brave: the BraveIdentityManagerFactory (subclass) instanced from
    ProfileSyncServiceFactory as part of a DependsOn() statement, and
    another one for every other place in the code (68 callpoints at this
    time) using DependsOn(IdentityManagerFactory::GetInstance()) to
    initialize different services.
    
    This hasn't rang any alarm until Chromium 92.0.4496.3, but in that
    release some changes were made[2] including the introduction of the
    IdentityManagerProvider class, which now DCHECKs for having only
    one instance of IdentityManagerFactory created in the browser:
    
      void SetIdentityManagerProvider(const IdentityManagerProvider& provider) {
        IdentityManagerProvider& instance = GetIdentityManagerProvider();
        DCHECK(!instance);
        instance = provider;
      }
    
    ...meaning that we will get a crash on startup and all the unit and
    browser tests crashing on DCHECK-enabled builds, revealing this issue
    that we didn't know we had until now.
    
    This patch, makes changes to ensure we have one IdentityManagerFactory
    instance only, by removing a few parts of the infrastructure added with
    PR #7480[1] and simply leveraging chromium_src overrides to implement
    the changes we need for the IdentityManager: to empty implementations
    of GetAccountsInCookieJar() and GetPrimaryAccountMutator().
    
    [1] #7480
    [2] https://chromium-review.googlesource.com/c/chromium/src/+/2824129
    
    Resolves brave/brave-browser#15659
    mariospr committed May 12, 2021
    Configuration menu
    Copy the full SHA
    d356d1f View commit details
    Browse the repository at this point in the history