Currently, both PasswordManagerClient and PasswordManagerClientHelperDelegate define

  PasswordManager* GetPasswordManager()

In PasswordManagerClient this method is non-virtual and defined in terms of a const variant (which is virtual and present in PMC).

In PasswordManagerClientHelperDelegate this method is pure virtual (and there is no const version).

The embedders' implementations of PasswordManagerClient (i.e., ChromePMC and IOSChromePMC) also both inherit PasswordManagerClientHelperDelegate. It seems silly that they currently need to define the override for PasswordManagerClientHelperDelegate::GetPasswormManager and the conveniently inherited PasswordManagerClient::GetPasswordManager is unused. Also, the override is private, so one has to upcast the implementations of PasswordManagerClient to PasswordManagerClient to be able to get the PasswordManager.

The name PasswordManagerClientHelper suggests that it is a helper for a PasswordManagerClient. So it might be reasonable to expect that the delegate is a PasswordManagerClient and so one of the solutions is to merge PasswordManagerClientHelperDelegate and PasswordManagerClient.

Or, both PasswordManagerClient and PasswordManagerClientHelperDelegate are types of an object which is capable of giving PasswordManager pointers. In that case, there should be an interface PasswordManagerDistributor, defining just the GetPasswordManager() accessor, from which both PasswordManagerClient and PasswordManagerClientHelperDelegate derive. We probably need to make that a virtual inheritance, to avoid having duplicities down in the hierarchy.

I'm Cc-ing vasilii@, who might have opinions or other ideas for this issue.