SigninViewControllerDelegate uses WebContentsModalDialogManager is a peculiar way.

WebContentsModalDialogManager owns the dialogs run on a WebContents, and itself is tied lifetime-wise to that WebContents. However, SinginViewControllerDelegate makes a WebContents, binds a WebContentsModalDialogManager to it (making the WebContentsModalDialogManager share the WebContents's lifetime), makes a modal dialog, and then makes the WebContents be the contents of the dialog.

The ownership picture is then: WebContents owns the WebContentsModalDialogManager (which is tied to the lifetime of the WebContents), which owns a dialog, which owns the WebContents starting this all.

This ouroboros, as you'd imagine, has lifetime fragility issues.

In fixing a crash ( bug 760507 ), I added a check for visibility (https://crrev.com/c/653800). However, in the case of closing a SigninViewControllerDelegate, that check hit a dangling delegate pointer and crashed. That led to a hilarious series of bugs (bug 771142,  bug 771872 ,  bug 772745 ) that got worse and worse as I misunderstood the ownership issues of SigninViewControllerDelegate and broke it more and more.

Those are fixed, and there is a bandaid over the crash in bug 771142. However, that bandaid avoids the crash by short-circuiting early and avoiding touching the dangling delegate.

There _shouldn't be_ dangling delegate pointers in the code. Right now, the whole teardown scenario of SigninViewControllerDelegate is incredibly fragile.

If indeed putting a WebContents into a dialog owned by a WebContentsModalDialogManager attached to that very WebContents is intended to be a supported use case, the lifetime issues need to be officially addressed with better mechanisms which need to be retrofitted into SigninViewControllerDelegate. The situation we're in right now, where the "obviously" correct thing to do is completely wrong, is not a good situation to be in.