From 5b27b90ab37e70f6926735fe23868adb083336b1 Mon Sep 17 00:00:00 2001 From: Nate Chapin Date: Tue, 2 Mar 2021 19:06:17 +0000 Subject: [PATCH] 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 Reviewed-by: Aaron Colwell Commit-Queue: Nate Chapin Cr-Commit-Position: refs/heads/master@{#859066} --- .../renderer_host/modal_close_listener_host.cc | 9 +++------ .../renderer_host/modal_close_listener_host.h | 2 -- .../modalclosewatcher/modal_close_watcher.idl | 2 +- .../global-interface-listing-expected.txt | 2 +- .../wpt_internal/modal-close-watcher/basic.html | 13 +++++++++++++ 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/content/browser/renderer_host/modal_close_listener_host.cc b/content/browser/renderer_host/modal_close_listener_host.cc index 99a16936ef9d6..350472e7d888c 100644 --- a/content/browser/renderer_host/modal_close_listener_host.cc +++ b/content/browser/renderer_host/modal_close_listener_host.cc @@ -16,11 +16,12 @@ ModalCloseListenerHost::~ModalCloseListenerHost() = default; void ModalCloseListenerHost::SetListener( mojo::PendingRemote 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 diff --git a/content/browser/renderer_host/modal_close_listener_host.h b/content/browser/renderer_host/modal_close_listener_host.h index 6787339f39513..7cbef98a21bf5 100644 --- a/content/browser/renderer_host/modal_close_listener_host.h +++ b/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; - void Disconnect(); - mojo::Remote modal_close_listener_; RENDER_DOCUMENT_HOST_USER_DATA_KEY_DECL(); DISALLOW_COPY_AND_ASSIGN(ModalCloseListenerHost); diff --git a/third_party/blink/renderer/modules/modalclosewatcher/modal_close_watcher.idl b/third_party/blink/renderer/modules/modalclosewatcher/modal_close_watcher.idl index 25c45816b3a35..146f18cd6352b 100644 --- a/third_party/blink/renderer/modules/modalclosewatcher/modal_close_watcher.idl +++ b/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(); diff --git a/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt b/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt index 7ba66d0a887f3..04140c2f5775c 100644 --- a/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt +++ b/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 diff --git a/third_party/blink/web_tests/wpt_internal/modal-close-watcher/basic.html b/third_party/blink/web_tests/wpt_internal/modal-close-watcher/basic.html index 7e3d3f3903240..2e80b1ba97ba1 100644 --- a/third_party/blink/web_tests/wpt_internal/modal-close-watcher/basic.html +++ b/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");