0

Upgrade DCHECK to DUMP_WILL_BE_CHECK in DocumentService.

1. Binding a null PendingReceiver is always unintended. There is no
   point in creating a DocumentService with a null receiver.
2. The supported ways of destroying a DocumentService always reset the
   receiver before deleting the service itself. This is used as a proxy
   for signaling that the DocumentService instance itself has been
   unregistered correctly. A still-bound receiver indicates that
   unregistration has not happened, and that memory safety violations
   will likely follow.

Change-Id: I48500e5bb30efe2442e246c05f466454aa2cb82a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5027338
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225727}
This commit is contained in:
Daniel Cheng
2023-11-16 21:33:03 +00:00
committed by Chromium LUCI CQ
parent b315710fcf
commit ce5d2fb6b5

@ -70,6 +70,9 @@ class DocumentService : public Interface, public internal::DocumentServiceBase {
mojo::PendingReceiver<Interface> pending_receiver)
: DocumentServiceBase(render_frame_host),
receiver_(this, std::move(pending_receiver)) {
// This is a developer error; it does not make sense to bind a
// DocumentService with a null PendingReceiver.
DUMP_WILL_BE_CHECK(receiver_.is_bound());
// |this| owns |receiver_|, so base::Unretained is safe.
receiver_.set_disconnect_handler(base::BindOnce(
[](DocumentService* document_service) {
@ -83,7 +86,7 @@ class DocumentService : public Interface, public internal::DocumentServiceBase {
~DocumentService() override {
// To avoid potential destruction order issues, implementations must use one
// of the *AndDeleteThis() methods below instead of writing `delete this`.
DCHECK(!receiver_.is_bound());
DUMP_WILL_BE_CHECK(!receiver_.is_bound());
}
// Subclasses may end their lifetime early by calling this method; `delete