GitLab

Currently, if a DialogDelegate's Accept() or Cancel() method leads back
into its Close(), two of the registered callbacks can be run, which is a
violation of DialogDelegate's contract and causes confusing workarounds
in client code. This often happens in code like this:

void SomeBubble::OnDialogAccepted() {
  NavigateTo(kSomeURL);
}

where NavigateTo causes a page navigation synchronously, which causes
the involved bubble to be closed.

Instead:

* In the near term, DialogDelegate should ensure that it never delivers
  more than one of these callbacks
* In the far term, DialogDelegate should enforce that callers do not
  re-enter Close() from inside Accept() or Cancel()

The latter will likely need to wait for the Accept/Cancel/Close callback
refactor to be finished.

This change also removes the remnants of the GlobalErrorBubbleView unit test
suite, which were exercising this behavior deliberately. These tests
were providing very little actual coverage and I have been gradually
deleting them as I refactor GlobalError anyway.

Bug: 1011446
Change-Id: Ifd76330c0f79d04ce2e5741c5a1977159ee598d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2042252
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739097}