Currently the public detach method in password controller is called from three places:
1. webStateDestroyed callback in PasswordTabHelper class.
2. dealloc method of PasswordController class.
3. webStateDestroyed callback in PasswordController class.

Thus, this method will be called 3 times when a tab is closed, which is kind of duplication.

The detach call in PasswordTabHelper class should be safely removed, since they are observing the same webState.
If webStateDestroyed is guaranteed to be called (Is this true?), there is no need to repeat that in dealloc again.
Also it is not recommended to send message to self in dealloc method, per http://go/objc-style#avoid-messaging-the-current-object-within-initializers-and-dealloc

What's more, the public detach method is harmful because it introduces the possibility of an additional state between a normal webState and a destroyed webState. That is to say, if detach method is called before webStateDestroyed is called, the webState ivar will be set to nil and all observers will be removed, but it is NOT yet destroyed. The current code in password controller have an isWebStateDestroyed property to indicate this situation, when some logic like passwordManager->OnPasswordFormSubmitted() is still able to proceed even if the webState is already set to nil. 

It will be nice if we can remove the public detach method and move the logic into webStateDestroyed callback of Password Controller directly. So that all the complicity coming with it can be simplified.