Add missing EventTarget inherit for ModalCloseWatcher

Also, the new test case uncovered a race condition where the
ModalCloseWatcher may try to disconnect and reconnect its mojo
pipe before the host in the browser process has a chance to run
its disconnect handler. Fix that, too.

Bug: 1182772
Change-Id: Icfaa143669803a0293c3497765a48c15448cc4d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2723246
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Aaron Colwell <acolwell@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#859066}

content/browser/renderer_host/modal_close_listener_host.cc

@@ -16,11 +16,12 @@ ModalCloseListenerHost::~ModalCloseListenerHost() = default;
 
 void ModalCloseListenerHost::SetListener(
     mojo::PendingRemote<blink::mojom::ModalCloseListener> listener) {
+  if (modal_close_listener_.is_bound())
+    modal_close_listener_.reset();
   modal_close_listener_.Bind(std::move(listener));
   // The renderer resets the message pipe when no ModalCloseWatchers are
   // waiting for a Signal().
-  modal_close_listener_.set_disconnect_handler(base::BindOnce(
-      &ModalCloseListenerHost::Disconnect, base::Unretained(this)));
+  modal_close_listener_.reset_on_disconnect();
 }
 
 bool ModalCloseListenerHost::SignalIfActive() {
@@ -30,10 +31,6 @@ bool ModalCloseListenerHost::SignalIfActive() {
   return true;
 }
 
-void ModalCloseListenerHost::Disconnect() {
-  modal_close_listener_.reset();
-}
-
 RENDER_DOCUMENT_HOST_USER_DATA_KEY_IMPL(ModalCloseListenerHost)
 
 }  // namespace content

content/browser/renderer_host/modal_close_listener_host.h

@@ -27,8 +27,6 @@ class CONTENT_EXPORT ModalCloseListenerHost
   explicit ModalCloseListenerHost(RenderFrameHost* render_frame_host);
   friend class RenderDocumentHostUserData<ModalCloseListenerHost>;
 
-  void Disconnect();
-
   mojo::Remote<blink::mojom::ModalCloseListener> modal_close_listener_;
   RENDER_DOCUMENT_HOST_USER_DATA_KEY_DECL();
   DISALLOW_COPY_AND_ASSIGN(ModalCloseListenerHost);

third_party/blink/renderer/modules/modalclosewatcher/modal_close_watcher.idl

@@ -5,7 +5,7 @@
 [
   Exposed=Window,
   RuntimeEnabled=ModalCloseWatcher
-] interface ModalCloseWatcher {
+] interface ModalCloseWatcher : EventTarget {
   [CallWith=ScriptState, RaisesException] constructor();
 
   void signalClosed();

third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt

@@ -5348,7 +5348,7 @@ interface MimeTypeArray
     method constructor
     method item
     method namedItem
-interface ModalCloseWatcher
+interface ModalCloseWatcher : EventTarget
     attribute @@toStringTag
     getter onbeforeclose
     getter onclose

third_party/blink/web_tests/wpt_internal/modal-close-watcher/basic.html

@@ -198,4 +198,17 @@
   watcher.onclose = () => { watcher.signalClosed(); }
   watcher.signalClosed();
 }, "signalClose inside onclose should not trigger an infinite loop");
+
+test(() => {
+  let watcher = new ModalCloseWatcher();
+  let onbeforeclose_called = false;
+  let onclose_called = false;
+  watcher.addEventListener("beforeclose", () => onbeforeclose_called = true);
+  watcher.addEventListener("close", () => onclose_called = true);
+
+  watcher.signalClosed();
+
+  assert_true(onbeforeclose_called);
+  assert_true(onclose_called);
+}, "signalClose with events added via addEventListener");
 </script>