diff --git a/libcef/browser/browser_frame.cc b/libcef/browser/browser_frame.cc index aacc292f0..91353b671 100644 --- a/libcef/browser/browser_frame.cc +++ b/libcef/browser/browser_frame.cc @@ -17,7 +17,12 @@ CefBrowserFrame::CefBrowserFrame( content::RenderFrameHost* render_frame_host, mojo::PendingReceiver receiver) - : FrameServiceBase(render_frame_host, std::move(receiver)) {} + : CefFrameServiceBase(render_frame_host, std::move(receiver)) { + DVLOG(1) << __func__ << ": frame " + << frame_util::GetFrameDebugString( + render_frame_host->GetGlobalFrameToken()) + << " bound"; +} CefBrowserFrame::~CefBrowserFrame() = default; @@ -62,13 +67,15 @@ void CefBrowserFrame::FrameAttached( if (auto host = GetFrameHost(/*prefer_speculative=*/true, &is_excluded)) { host->FrameAttached(std::move(render_frame), reattached); } else if (is_excluded) { - VLOG(1) << "frame " - << frame_util::GetFrameDebugString( - render_frame_host()->GetGlobalFrameToken()) - << " attach denied"; + DVLOG(1) << __func__ << ": frame " + << frame_util::GetFrameDebugString( + render_frame_host()->GetGlobalFrameToken()) + << " attach denied"; mojo::Remote render_frame_remote; render_frame_remote.Bind(std::move(render_frame)); render_frame_remote->FrameAttachedAck(/*allow=*/false); + render_frame_remote.ResetWithReason( + static_cast(frame_util::ResetReason::kExcluded), "Excluded"); } } diff --git a/libcef/browser/browser_frame.h b/libcef/browser/browser_frame.h index a21d762b8..488a23b2b 100644 --- a/libcef/browser/browser_frame.h +++ b/libcef/browser/browser_frame.h @@ -16,8 +16,7 @@ // association with the RenderFrameHost (which may be speculative, etc.), and so // that messages are always routed to the most appropriate CefFrameHostImpl // instance. Lifespan is tied to the RFH via FrameServiceBase. -class CefBrowserFrame - : public content::FrameServiceBase { +class CefBrowserFrame : public CefFrameServiceBase { public: CefBrowserFrame(content::RenderFrameHost* render_frame_host, mojo::PendingReceiver receiver); @@ -44,9 +43,6 @@ class CefBrowserFrame std::optional> regions) override; - // FrameServiceBase methods: - bool ShouldCloseOnFinishNavigation() const override { return false; } - CefRefPtr GetFrameHost(bool prefer_speculative, bool* is_excluded = nullptr) const; }; diff --git a/libcef/browser/frame_host_impl.cc b/libcef/browser/frame_host_impl.cc index 9c7e2090b..a755aaf6e 100644 --- a/libcef/browser/frame_host_impl.cc +++ b/libcef/browser/frame_host_impl.cc @@ -101,6 +101,7 @@ CefFrameHostImpl::CefFrameHostImpl(scoped_refptr browser_info, : render_frame_host->GetParent()->GetGlobalFrameToken()), render_frame_host_(render_frame_host) { DCHECK(browser_info_); + DVLOG(1) << __func__ << ": " << GetDebugString() << " created "; } CefFrameHostImpl::~CefFrameHostImpl() { @@ -502,24 +503,6 @@ bool CefFrameHostImpl::IsDetached() const { bool CefFrameHostImpl::Detach(DetachReason reason, bool is_current_main_frame) { CEF_REQUIRE_UIT(); - if (VLOG_IS_ON(1)) { - std::string reason_str; - switch (reason) { - case DetachReason::RENDER_FRAME_DELETED: - reason_str = "RENDER_FRAME_DELETED"; - break; - case DetachReason::NEW_MAIN_FRAME: - reason_str = "NEW_MAIN_FRAME"; - break; - case DetachReason::BROWSER_DESTROYED: - reason_str = "BROWSER_DESTROYED"; - break; - }; - - VLOG(1) << GetDebugString() << " detached (reason=" << reason_str - << ", is_connected=" << render_frame_.is_bound() << ")"; - } - // This method may be called multiple times (e.g. from CefBrowserInfo // SetMainFrame and RemoveFrame). bool is_first_complete_detach = false; @@ -546,15 +529,41 @@ bool CefFrameHostImpl::Detach(DetachReason reason, bool is_current_main_frame) { } if (render_frame_.is_bound()) { - render_frame_->FrameDetached(); + if (VLOG_IS_ON(1)) { + std::string reason_str; + switch (reason) { + case DetachReason::RENDER_FRAME_DELETED: + reason_str = "RENDER_FRAME_DELETED"; + break; + case DetachReason::NEW_MAIN_FRAME: + reason_str = "NEW_MAIN_FRAME"; + break; + case DetachReason::BROWSER_DESTROYED: + reason_str = "BROWSER_DESTROYED"; + break; + }; + + DVLOG(1) << __func__ << ": " << GetDebugString() + << " detached (reason=" << reason_str << ")"; + } + DetachRenderFrame(); } - render_frame_.reset(); - render_frame_host_ = nullptr; + if (render_frame_host_) { + DVLOG(1) << __func__ << ": " << GetDebugString() << " host cleared"; + render_frame_host_ = nullptr; + } return is_first_complete_detach; } +void CefFrameHostImpl::DetachRenderFrame() { + CEF_REQUIRE_UIT(); + DCHECK(render_frame_.is_bound()); + render_frame_.ResetWithReason( + static_cast(frame_util::ResetReason::kDetached), "Detached"); +} + void CefFrameHostImpl::MaybeReAttach( scoped_refptr browser_info, content::RenderFrameHost* render_frame_host, @@ -571,16 +580,12 @@ void CefFrameHostImpl::MaybeReAttach( // If |require_detached| then we expect that Detach() was called previously. CHECK(!require_detached || !render_frame_.is_bound()); - if (render_frame_host_) { + if (render_frame_.is_bound()) { // Intentionally not clearing |queued_renderer_actions_|, as we may be // changing RFH during initial browser navigation. - VLOG(1) << GetDebugString() - << " detached (reason=RENDER_FRAME_CHANGED, is_connected=" - << render_frame_.is_bound() << ")"; - if (render_frame_.is_bound()) { - render_frame_->FrameDetached(); - } - render_frame_.reset(); + DVLOG(1) << __func__ << ": " << GetDebugString() + << " detached (reason=RENDER_FRAME_CHANGED)"; + DetachRenderFrame(); } // The RFH may change but the frame token should remain the same. @@ -591,6 +596,7 @@ void CefFrameHostImpl::MaybeReAttach( browser_info_ = browser_info; } + DVLOG(1) << __func__ << ": " << GetDebugString() << " host changed"; render_frame_host_ = render_frame_host; RefreshAttributes(); @@ -689,7 +695,8 @@ void CefFrameHostImpl::FrameAttached( return; } - VLOG(1) << GetDebugString() << " " << (reattached ? "re" : "") << "connected"; + DVLOG(1) << __func__ << ": " << GetDebugString() << " " + << (reattached ? "re" : "") << "connected"; render_frame_.Bind(std::move(render_frame_remote)); render_frame_.set_disconnect_handler( diff --git a/libcef/browser/frame_host_impl.h b/libcef/browser/frame_host_impl.h index 334210198..86993f1cd 100644 --- a/libcef/browser/frame_host_impl.h +++ b/libcef/browser/frame_host_impl.h @@ -184,6 +184,8 @@ class CefFrameHostImpl : public CefFrame, public cef::mojom::BrowserFrame { void OnRenderFrameDisconnect(); + void DetachRenderFrame(); + std::string GetDebugString() const; const bool is_main_frame_; diff --git a/libcef/browser/frame_service_base.h b/libcef/browser/frame_service_base.h index ee4d4c097..b1cc5b611 100644 --- a/libcef/browser/frame_service_base.h +++ b/libcef/browser/frame_service_base.h @@ -11,6 +11,7 @@ #include "base/logging.h" #include "base/memory/raw_ptr.h" #include "base/threading/thread_checker.h" +#include "cef/libcef/common/frame_util.h" #include "content/public/browser/navigation_handle.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/web_contents.h" @@ -19,47 +20,36 @@ #include "mojo/public/cpp/bindings/receiver.h" #include "url/origin.h" -namespace content { - -// Base class for mojo interface implementations tied to a document's lifetime. -// The service will be destroyed when any of the following happens: -// 1. mojo interface connection error happened, -// 2. the RenderFrameHost was deleted, or -// 3. navigation was committed on the RenderFrameHost (not same document) and -// ShouldCloseOnFinishNavigation() returns true. -// -// WARNING: To avoid race conditions, subclasses MUST only get the origin via -// origin() instead of from |render_frame_host| passed in the constructor. -// See https://crbug.com/769189 for an example of such a race. +// Base class for mojo interface implementations tied to a RenderFrameHost +// lifetime. The service will be destroyed on mojo interface connection error +// or RFH deletion. // // Based on the old implementation of DocumentServiceBase that existed prior to // https://crrev.com/2809effa24. CEF requires the old implementation to support // bindings that outlive navigation. template -class FrameServiceBase : public Interface, public WebContentsObserver { +class CefFrameServiceBase : public Interface, + public content::WebContentsObserver { public: - FrameServiceBase(RenderFrameHost* render_frame_host, - mojo::PendingReceiver pending_receiver) - : WebContentsObserver( - WebContents::FromRenderFrameHost(render_frame_host)), + CefFrameServiceBase(content::RenderFrameHost* render_frame_host, + mojo::PendingReceiver pending_receiver) + : content::WebContentsObserver( + content::WebContents::FromRenderFrameHost(render_frame_host)), render_frame_host_(render_frame_host), - origin_(render_frame_host_->GetLastCommittedOrigin()), receiver_(this, std::move(pending_receiver)) { // |this| owns |receiver_|, so unretained is safe. receiver_.set_disconnect_handler( - base::BindOnce(&FrameServiceBase::Close, base::Unretained(this))); + base::BindOnce(&CefFrameServiceBase::Close, base::Unretained(this))); } protected: // Make the destructor private since |this| can only be deleted by Close(). - ~FrameServiceBase() override = default; - - // All subclasses should use this function to obtain the origin instead of - // trying to get it from the RenderFrameHost pointer directly. - const url::Origin& origin() const { return origin_; } + ~CefFrameServiceBase() override = default; // Returns the RenderFrameHost held by this object. - RenderFrameHost* render_frame_host() const { return render_frame_host_; } + content::RenderFrameHost* render_frame_host() const { + return render_frame_host_; + } // Subclasses can use this to check thread safety. // For example: DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -72,44 +62,23 @@ class FrameServiceBase : public Interface, public WebContentsObserver { // Use WebContents::From(render_frame_host()) instead, but please keep in mind // that the render_frame_host() might not be active. See // RenderFrameHost::IsActive() for details. - using WebContentsObserver::web_contents; + using content::WebContentsObserver::web_contents; // WebContentsObserver implementation. - void RenderFrameDeleted(RenderFrameHost* render_frame_host) final { + void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) final { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); if (render_frame_host == render_frame_host_) { - DVLOG(1) << __func__ << ": RenderFrame destroyed."; + DVLOG(1) << __func__ << ": RenderFrameHost destroyed."; + if (receiver_.is_bound()) { + receiver_.ResetWithReason( + static_cast(frame_util::ResetReason::kDeleted), + "Deleted"); + } Close(); } } - void DidFinishNavigation(NavigationHandle* navigation_handle) final { - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - if (!ShouldCloseOnFinishNavigation()) { - return; - } - - if (!navigation_handle->HasCommitted() || - navigation_handle->IsSameDocument() || - navigation_handle->IsPageActivation()) { - return; - } - - if (navigation_handle->GetRenderFrameHost() == render_frame_host_) { - // FrameServiceBase is destroyed either when RenderFrameHost is - // destroyed (covered by RenderFrameDeleted) or when a new document - // commits in the same RenderFrameHost (covered by DidFinishNavigation). - // Only committed non-same-document non-bfcache non-prerendering - // activation navigations replace a document in existing RenderFrameHost. - DVLOG(1) << __func__ << ": Close connection on navigation."; - Close(); - } - } - - // Used for CEF bindings that outlive navigation. - virtual bool ShouldCloseOnFinishNavigation() const { return true; } - // Stops observing WebContents and delete |this|. void Close() { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -117,11 +86,8 @@ class FrameServiceBase : public Interface, public WebContentsObserver { delete this; } - const raw_ptr render_frame_host_ = nullptr; - const url::Origin origin_; + const raw_ptr render_frame_host_ = nullptr; mojo::Receiver receiver_; }; -} // namespace content - #endif // CEF_LIBCEF_BROWSER_FRAME_SERVICE_BASE_H_ diff --git a/libcef/common/frame_util.h b/libcef/common/frame_util.h index 71740dd75..e9bb3a106 100644 --- a/libcef/common/frame_util.h +++ b/libcef/common/frame_util.h @@ -86,6 +86,16 @@ std::string GetFrameDebugString( std::string GetFrameDebugString( const content::GlobalRenderFrameHostToken& global_token); +// Used in combination with ResetWithReason() to report mojo connection +// disconnect reasons. |kNoReason| (0) is the default when a mojo connection +// disconnects without a specified reason. +enum class ResetReason : uint32_t { + kNoReason, + kDeleted, + kDetached, + kExcluded, +}; + } // namespace frame_util #endif // CEF_LIBCEF_COMMON_FRAME_UTIL_H_ diff --git a/libcef/common/mojom/cef.mojom b/libcef/common/mojom/cef.mojom index 75dc10bb1..b82d9d397 100644 --- a/libcef/common/mojom/cef.mojom +++ b/libcef/common/mojom/cef.mojom @@ -54,9 +54,6 @@ interface RenderFrame { // |allow| will be false if we don't want to attach the frame. FrameAttachedAck(bool allow); - // Browser process has intentionally detached. - FrameDetached(); - // Send a message to the render process. SendMessage(string name, mojo_base.mojom.ListValue arguments); diff --git a/libcef/renderer/frame_impl.cc b/libcef/renderer/frame_impl.cc index f58833c8b..fb3eb1b2c 100644 --- a/libcef/renderer/frame_impl.cc +++ b/libcef/renderer/frame_impl.cc @@ -54,11 +54,15 @@ namespace { // Maximum number of times to retry the browser connection. constexpr size_t kConnectionRetryMaxCt = 3U; -// Length of time to wait before initiating a browser connection retry. -constexpr auto kConnectionRetryDelay = base::Seconds(1); - -// Length of time to wait for the browser connection ACK before timing out. -constexpr auto kConnectionTimeout = base::Seconds(10); +// Length of time to wait before initiating a browser connection retry. The +// short value is optimized for navigation-related disconnects (time delta +// between CefFrameImpl::OnDisconnect and CefFrameHostImpl::MaybeReAttach) which +// should take << 10ms in normal circumstances (reasonably fast machine, limited +// redirects). The long value is optimized for slower machines or navigations +// with many redirects to reduce overall failure rates. See related comments in +// CefFrameImpl::OnDisconnect. +constexpr auto kConnectionRetryDelayShort = base::Milliseconds(25); +constexpr auto kConnectionRetryDelayLong = base::Seconds(3); std::string GetDebugString(blink::WebLocalFrame* frame) { return "frame " + render_frame_util::GetIdentifier(frame); @@ -423,7 +427,7 @@ void CefFrameImpl::OnDetached() { browser_->FrameDetached(frame_); frame_ = nullptr; - OnDisconnect(DisconnectReason::DETACHED, 0, std::string()); + OnDisconnect(DisconnectReason::DETACHED, 0, std::string(), MOJO_RESULT_OK); browser_ = nullptr; @@ -480,26 +484,26 @@ void CefFrameImpl::ConnectBrowserFrame(ConnectReason reason) { "RETRY %zu/%zu", browser_connect_retry_ct_, kConnectionRetryMaxCt); break; } - VLOG(1) << frame_debug_str_ << " connection request (reason=" << reason_str - << ")"; + DVLOG(1) << __func__ << ": " << frame_debug_str_ + << " connection request (reason=" << reason_str << ")"; } + browser_connect_timer_.Stop(); + // Don't attempt to connect an invalid or bfcache'd frame. If a bfcache'd // frame returns to active status a reconnect will be triggered via // OnWasShown(). if (!frame_ || attach_denied_ || blink_glue::IsInBackForwardCache(frame_)) { browser_connection_state_ = ConnectionState::DISCONNECTED; - browser_connect_timer_.Stop(); - VLOG(1) << frame_debug_str_ << " connection retry canceled (reason=" - << (frame_ ? (attach_denied_ ? "ATTACH_DENIED" : "BFCACHED") - : "INVALID") - << ")"; + DVLOG(1) << __func__ << ": " << frame_debug_str_ + << " connection retry canceled (reason=" + << (frame_ ? (attach_denied_ ? "ATTACH_DENIED" : "BFCACHED") + : "INVALID") + << ")"; return; } browser_connection_state_ = ConnectionState::CONNECTION_PENDING; - browser_connect_timer_.Start(FROM_HERE, kConnectionTimeout, this, - &CefFrameImpl::OnBrowserFrameTimeout); auto& browser_frame = GetBrowserFrame(/*expect_acked=*/false); CHECK(browser_frame); @@ -514,7 +518,7 @@ void CefFrameImpl::ConnectBrowserFrame(ConnectReason reason) { // connection. browser_frame->FrameAttached(receiver_.BindNewPipeAndPassRemote(), reattached); - receiver_.set_disconnect_with_reason_handler( + receiver_.set_disconnect_with_reason_and_result_handler( base::BindOnce(&CefFrameImpl::OnRenderFrameDisconnect, this)); } @@ -529,28 +533,25 @@ const mojo::Remote& CefFrameImpl::GetBrowserFrame( // Triggers creation of a CefBrowserFrame in the browser process. render_frame->GetBrowserInterfaceBroker().GetInterface( browser_frame_.BindNewPipeAndPassReceiver()); - browser_frame_.set_disconnect_with_reason_handler( + browser_frame_.set_disconnect_with_reason_and_result_handler( base::BindOnce(&CefFrameImpl::OnBrowserFrameDisconnect, this)); } } return browser_frame_; } -void CefFrameImpl::OnBrowserFrameTimeout() { - LOG(ERROR) << frame_debug_str_ << " connection timeout"; - OnDisconnect(DisconnectReason::CONNECT_TIMEOUT, 0, std::string()); -} - void CefFrameImpl::OnBrowserFrameDisconnect(uint32_t custom_reason, - const std::string& description) { + const std::string& description, + MojoResult error_result) { OnDisconnect(DisconnectReason::BROWSER_FRAME_DISCONNECT, custom_reason, - description); + description, error_result); } void CefFrameImpl::OnRenderFrameDisconnect(uint32_t custom_reason, - const std::string& description) { + const std::string& description, + MojoResult error_result) { OnDisconnect(DisconnectReason::RENDER_FRAME_DISCONNECT, custom_reason, - description); + description, error_result); } // static @@ -560,18 +561,13 @@ std::string CefFrameImpl::GetDisconnectDebugString( bool frame_is_main, DisconnectReason reason, uint32_t custom_reason, - const std::string& description) { + const std::string& description, + MojoResult error_result) { std::string reason_str; switch (reason) { case DisconnectReason::DETACHED: reason_str = "DETACHED"; break; - case DisconnectReason::BROWSER_FRAME_DETACHED: - reason_str = "BROWSER_FRAME_DETACHED"; - break; - case DisconnectReason::CONNECT_TIMEOUT: - reason_str = "CONNECT_TIMEOUT"; - break; case DisconnectReason::RENDER_FRAME_DISCONNECT: reason_str = "RENDER_FRAME_DISCONNECT"; break; @@ -604,7 +600,8 @@ std::string CefFrameImpl::GetDisconnectDebugString( state_str += ", SUB_FRAME"; } - if (custom_reason > 0) { + if (custom_reason != + static_cast(frame_util::ResetReason::kNoReason)) { state_str += ", custom_reason=" + base::NumberToString(custom_reason); } @@ -612,12 +609,17 @@ std::string CefFrameImpl::GetDisconnectDebugString( state_str += ", description=" + description; } + if (error_result != MOJO_RESULT_OK) { + state_str += ", error_result=" + base::NumberToString(error_result); + } + return "(reason=" + reason_str + ", current_state=" + state_str + ")"; } void CefFrameImpl::OnDisconnect(DisconnectReason reason, uint32_t custom_reason, - const std::string& description) { + const std::string& description, + MojoResult error_result) { // Ignore multiple calls in close proximity (which may occur if both // |browser_frame_| and |receiver_| disconnect). |frame_| will be nullptr // when called from/after OnDetached(). @@ -626,56 +628,108 @@ void CefFrameImpl::OnDisconnect(DisconnectReason reason, return; } - if (attach_denied_) { - VLOG(1) << frame_debug_str_ << " connection attach denied"; + // Ignore additional calls if we're already disconnected. DETACHED, + // RENDER_FRAME_DISCONNECT and/or BROWSER_FRAME_DISCONNECT may arrive in any + // order. + if (browser_connection_state_ == ConnectionState::DISCONNECTED) { return; } const auto connection_state = browser_connection_state_; const bool frame_is_valid = !!frame_; const bool frame_is_main = frame_ && frame_->IsOutermostMainFrame(); - VLOG(1) << frame_debug_str_ << " disconnected " - << GetDisconnectDebugString(connection_state, frame_is_valid, - frame_is_main, reason, custom_reason, - description); + DVLOG(1) << __func__ << ": " << frame_debug_str_ << " disconnected " + << GetDisconnectDebugString(connection_state, frame_is_valid, + frame_is_main, reason, custom_reason, + description, error_result); browser_frame_.reset(); receiver_.reset(); browser_connection_state_ = ConnectionState::DISCONNECTED; - browser_connect_timer_.Stop(); - // Only retry if the frame is still valid and the browser process has not + // True if the frame was previously bound/connected and then intentionally + // detached (Receiver::ResetWithReason called) from the browser process side. + const bool connected_and_intentionally_detached = + (reason == DisconnectReason::BROWSER_FRAME_DISCONNECT || + reason == DisconnectReason::RENDER_FRAME_DISCONNECT) && + custom_reason != + static_cast(frame_util::ResetReason::kNoReason); + + // Don't retry if the frame is invalid or if the browser process has // intentionally detached. - if (frame_ && reason != DisconnectReason::BROWSER_FRAME_DETACHED) { - if (browser_connect_retry_ct_++ < kConnectionRetryMaxCt) { - VLOG(1) << frame_debug_str_ << " connection retry scheduled"; - if (!browser_connect_retry_log_.empty()) { - browser_connect_retry_log_ += "; "; - } - browser_connect_retry_log_ += GetDisconnectDebugString( - connection_state, frame_is_valid, frame_is_main, reason, - custom_reason, description); - - // Retry after a delay in case the frame is currently navigating, being - // destroyed, or entering the bfcache. In the navigation case the retry - // will likely succeed. In the destruction case the retry will be - // ignored/canceled due to OnDetached(). In the bfcache case the status - // may not be updated immediately, so we allow the reconnect timer to - // trigger and check the status in ConnectBrowserFrame() instead. - browser_connection_state_ = ConnectionState::RECONNECT_PENDING; - browser_connect_timer_.Start( - FROM_HERE, kConnectionRetryDelay, - base::BindOnce(&CefFrameImpl::ConnectBrowserFrame, this, - ConnectReason::RETRY)); - } else { - // Trigger a crash in official builds. - LOG(FATAL) << frame_debug_str_ << " connection retry failed " - << GetDisconnectDebugString(connection_state, frame_is_valid, - frame_is_main, reason, - custom_reason, description) - << ", prior disconnects: " << browser_connect_retry_log_; - } + if (!frame_ || attach_denied_ || connected_and_intentionally_detached) { + return; } + + // True if the connection was closed (binding declined) from the browser + // process side. This can occur during navigation or if a matching + // RenderFrameHost is not currently available (like for bfcache'd frames). + // When navigating there is a race in the browser process between + // BrowserInterfaceBrokerImpl::GetInterface and RenderFrameHostImpl:: + // DidCommitNavigation. The connection will be closed if the GetInterface call + // from the renderer is still in-flight when DidCommitNavigation calls + // |broker_receiver_.reset()|. If, however, the GetInterface call arrives + // first (BrowserInterfaceBrokerImpl::GetInterface called and the + // PendingReceiver bound) then the binding will be successful and remain + // connected until the connection is closed for some other reason (like the + // Receiver being reset or the renderer process terminating). + const bool connection_binding_declined = + (reason == DisconnectReason::BROWSER_FRAME_DISCONNECT || + reason == DisconnectReason::RENDER_FRAME_DISCONNECT) && + error_result == MOJO_RESULT_FAILED_PRECONDITION; + + if (browser_connect_retry_ct_++ < kConnectionRetryMaxCt) { + DVLOG(1) << __func__ << ": " << frame_debug_str_ + << " connection retry scheduled (" << browser_connect_retry_ct_ + << "/" << kConnectionRetryMaxCt << ")"; + if (!browser_connect_retry_log_.empty()) { + browser_connect_retry_log_ += "; "; + } + browser_connect_retry_log_ += GetDisconnectDebugString( + connection_state, frame_is_valid, frame_is_main, reason, custom_reason, + description, error_result); + + // Use a shorter delay for the first retry attempt after the browser process + // intentionally declines the connection. This will improve load performance + // in normal circumstances (reasonably fast machine and navigations with + // limited redirects). + const auto retry_delay = + connection_binding_declined && browser_connect_retry_ct_ == 1 + ? kConnectionRetryDelayShort + : kConnectionRetryDelayLong; + + // Retry after a delay in case the frame is currently navigating or entering + // the bfcache. In the navigation case the retry will likely succeed. In the + // bfcache case the status may not be updated immediately, so we allow the + // reconnect timer to trigger and then check the status in + // ConnectBrowserFrame() instead. + browser_connection_state_ = ConnectionState::RECONNECT_PENDING; + browser_connect_timer_.Start( + FROM_HERE, retry_delay, + base::BindOnce(&CefFrameImpl::ConnectBrowserFrame, this, + ConnectReason::RETRY)); + return; + } + + DVLOG(1) << __func__ << ": " << frame_debug_str_ + << " connection retry limit exceeded"; + + // Don't crash on retry failures in cases where the browser process has + // intentionally declined the connection and we have never been previously + // connected. Also don't crash for sub-frame connection failures as those are + // less likely to be important functionally. We still crash for other main + // frame connection errors or in cases where a previously connected main frame + // was disconnected without first being intentionally deleted or detached. + const bool ignore_retry_failure = + (connection_binding_declined && !ever_connected_) || !frame_is_main; + + // Trigger a crash in official builds. + LOG_IF(FATAL, !ignore_retry_failure) + << frame_debug_str_ << " connection retry failed " + << GetDisconnectDebugString(connection_state, frame_is_valid, + frame_is_main, reason, custom_reason, + description, error_result) + << ", prior disconnects: " << browser_connect_retry_log_; } void CefFrameImpl::SendToBrowserFrame(const std::string& function_name, @@ -733,7 +787,9 @@ void CefFrameImpl::FrameAttachedAck(bool allow) { browser_connection_state_ = ConnectionState::CONNECTION_ACKED; browser_connect_retry_ct_ = 0; browser_connect_retry_log_.clear(); - browser_connect_timer_.Stop(); + + DVLOG(1) << __func__ << ": " << frame_debug_str_ + << " connection acked allow=" << allow; if (!allow) { // This will be followed by a connection disconnect from the browser side. @@ -744,6 +800,8 @@ void CefFrameImpl::FrameAttachedAck(bool allow) { return; } + ever_connected_ = true; + auto& browser_frame = GetBrowserFrame(); CHECK(browser_frame); @@ -753,12 +811,6 @@ void CefFrameImpl::FrameAttachedAck(bool allow) { } } -void CefFrameImpl::FrameDetached() { - // Sent from the browser process in response to CefFrameHostImpl::Detach(). - CHECK_EQ(ConnectionState::CONNECTION_ACKED, browser_connection_state_); - OnDisconnect(DisconnectReason::BROWSER_FRAME_DETACHED, 0, std::string()); -} - void CefFrameImpl::SendMessage(const std::string& name, base::Value::List arguments) { if (auto app = CefAppManager::Get()->GetApplication()) { diff --git a/libcef/renderer/frame_impl.h b/libcef/renderer/frame_impl.h index 0d48aec39..41b888c47 100644 --- a/libcef/renderer/frame_impl.h +++ b/libcef/renderer/frame_impl.h @@ -117,20 +117,17 @@ class CefFrameImpl using BrowserFrameType = mojo::Remote; const BrowserFrameType& GetBrowserFrame(bool expect_acked = true); - // Called if the BrowserFrame connection attempt times out. - void OnBrowserFrameTimeout(); - // Called if the BrowserFrame connection is disconnected. void OnBrowserFrameDisconnect(uint32_t custom_reason, - const std::string& description); + const std::string& description, + MojoResult error_result); // Called if the RenderFrame connection is disconnected. void OnRenderFrameDisconnect(uint32_t custom_reason, - const std::string& description); + const std::string& description, + MojoResult error_result); enum class DisconnectReason { DETACHED, - BROWSER_FRAME_DETACHED, - CONNECT_TIMEOUT, RENDER_FRAME_DISCONNECT, BROWSER_FRAME_DISCONNECT, }; @@ -140,7 +137,8 @@ class CefFrameImpl // representation is destroyed and closes the connection). void OnDisconnect(DisconnectReason reason, uint32_t custom_reason, - const std::string& description); + const std::string& description, + MojoResult error_result); // Send an action to the remote BrowserFrame. This will queue the action if // the remote frame is not yet attached. @@ -152,7 +150,6 @@ class CefFrameImpl // cef::mojom::RenderFrame methods: void FrameAttachedAck(bool allow) override; - void FrameDetached() override; void SendMessage(const std::string& name, base::Value::List arguments) override; void SendSharedMemoryRegion(const std::string& name, @@ -190,6 +187,8 @@ class CefFrameImpl // Log of reasons why the reconnect failed. std::string browser_connect_retry_log_; + bool ever_connected_ = false; + // Current browser connection state. enum class ConnectionState { DISCONNECTED, @@ -203,7 +202,8 @@ class CefFrameImpl bool frame_is_main, DisconnectReason reason, uint32_t custom_reason, - const std::string& description); + const std::string& description, + MojoResult error_result); base::OneShotTimer browser_connect_timer_; diff --git a/patch/patch.cfg b/patch/patch.cfg index 7e27525c1..940d1c93c 100644 --- a/patch/patch.cfg +++ b/patch/patch.cfg @@ -753,5 +753,10 @@ patches = [ # https://issues.chromium.org/issues/336738728 'name': 'v8_build', 'path': 'v8' + }, + { + # Expose Mojo Connector error state to Receiver disconnect handlers. + # https://github.com/chromiumembedded/cef/issues/3664 + 'name': 'mojo_connect_result_3664' } ] diff --git a/patch/patches/mojo_connect_result_3664.patch b/patch/patches/mojo_connect_result_3664.patch new file mode 100644 index 000000000..7ab242ed4 --- /dev/null +++ b/patch/patches/mojo_connect_result_3664.patch @@ -0,0 +1,459 @@ +diff --git ipc/ipc_mojo_bootstrap.cc ipc/ipc_mojo_bootstrap.cc +index 220f6b5fb0213..3dc6077ec69b8 100644 +--- ipc/ipc_mojo_bootstrap.cc ++++ ipc/ipc_mojo_bootstrap.cc +@@ -977,7 +977,8 @@ class ChannelAssociatedGroupController + endpoint->disconnect_reason()); + + base::AutoUnlock unlocker(lock_); +- client->NotifyError(reason); ++ // TODO(cef): Route the actual Connector error if/when needed. ++ client->NotifyError(reason, MOJO_RESULT_OK); + } else { + endpoint->task_runner()->PostTask( + FROM_HERE, +diff --git mojo/public/cpp/bindings/associated_receiver.h mojo/public/cpp/bindings/associated_receiver.h +index 76065029e088c..73a4eb914dca1 100644 +--- mojo/public/cpp/bindings/associated_receiver.h ++++ mojo/public/cpp/bindings/associated_receiver.h +@@ -46,6 +46,8 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) AssociatedReceiverBase { + void set_disconnect_handler(base::OnceClosure error_handler); + void set_disconnect_with_reason_handler( + ConnectionErrorWithReasonCallback error_handler); ++ void set_disconnect_with_reason_and_result_handler( ++ ConnectionErrorWithReasonAndResultCallback error_handler); + void reset_on_disconnect(); + + bool is_bound() const { return !!endpoint_client_; } +@@ -158,6 +160,7 @@ class AssociatedReceiver : public internal::AssociatedReceiverBase { + // Like above but when invoked |handler| will receive additional metadata + // about why the remote endpoint was closed, if provided. + using AssociatedReceiverBase::set_disconnect_with_reason_handler; ++ using AssociatedReceiverBase::set_disconnect_with_reason_and_result_handler; + + // Resets this AssociatedReceiver on disconnect. Note that this replaces any + // previously set disconnection handler. Must be called on a bound +diff --git mojo/public/cpp/bindings/associated_remote.h mojo/public/cpp/bindings/associated_remote.h +index 7e8b0144b528a..80a2ed85d1739 100644 +--- mojo/public/cpp/bindings/associated_remote.h ++++ mojo/public/cpp/bindings/associated_remote.h +@@ -153,6 +153,11 @@ class AssociatedRemote { + internal_state_.set_connection_error_with_reason_handler( + std::move(handler)); + } ++ void set_disconnect_with_reason_and_result_handler( ++ ConnectionErrorWithReasonAndResultCallback handler) { ++ internal_state_.set_connection_error_with_reason_and_result_handler( ++ std::move(handler)); ++ } + + // A convenient helper that resets this AssociatedRemote on disconnect. Note + // that this replaces any previously set disconnection handler. Must be called +diff --git mojo/public/cpp/bindings/connection_error_callback.h mojo/public/cpp/bindings/connection_error_callback.h +index 0e0cad6032f6d..9dd09d26cdcc3 100644 +--- mojo/public/cpp/bindings/connection_error_callback.h ++++ mojo/public/cpp/bindings/connection_error_callback.h +@@ -6,6 +6,7 @@ + #define MOJO_PUBLIC_CPP_BINDINGS_CONNECTION_ERROR_CALLBACK_H_ + + #include "base/functional/callback.h" ++#include "mojo/public/c/system/types.h" + + namespace mojo { + +@@ -18,6 +19,14 @@ using ConnectionErrorWithReasonCallback = + using RepeatingConnectionErrorWithReasonCallback = + base::RepeatingCallback; ++using ConnectionErrorWithReasonAndResultCallback = ++ base::OnceCallback; ++using RepeatingConnectionErrorWithReasonAndResultCallback = ++ base::RepeatingCallback; + + } // namespace mojo + +diff --git mojo/public/cpp/bindings/connector.h mojo/public/cpp/bindings/connector.h +index 332ab5c181ec5..4751eada85d07 100644 +--- mojo/public/cpp/bindings/connector.h ++++ mojo/public/cpp/bindings/connector.h +@@ -146,6 +146,11 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) Connector : public MessageReceiver { + return error_; + } + ++ MojoResult handle_ready_result() const { ++ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); ++ return handle_ready_result_; ++ } ++ + // Starts receiving on the Connector's message pipe, allowing incoming + // messages and error events to be dispatched. Once called, the Connector is + // effectively bound to `task_runner`. Initialization methods like +@@ -320,6 +325,7 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) Connector : public MessageReceiver { + std::optional peer_remoteness_tracker_; + + std::atomic error_ GUARDED_BY_CONTEXT(sequence_checker_); ++ MojoResult handle_ready_result_ = MOJO_RESULT_OK; + bool drop_writes_ = false; + bool enforce_errors_from_incoming_receiver_ = true; + +diff --git mojo/public/cpp/bindings/interface_endpoint_client.h mojo/public/cpp/bindings/interface_endpoint_client.h +index 2d796cec6e42e..dc2c0667afec9 100644 +--- mojo/public/cpp/bindings/interface_endpoint_client.h ++++ mojo/public/cpp/bindings/interface_endpoint_client.h +@@ -77,6 +77,7 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) InterfaceEndpointClient + CHECK(sequence_checker_.CalledOnValidSequence()); + error_handler_ = std::move(error_handler); + error_with_reason_handler_.Reset(); ++ error_with_reason_and_result_handler_.Reset(); + } + + void set_connection_error_with_reason_handler( +@@ -84,6 +85,15 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) InterfaceEndpointClient + CHECK(sequence_checker_.CalledOnValidSequence()); + error_with_reason_handler_ = std::move(error_handler); + error_handler_.Reset(); ++ error_with_reason_and_result_handler_.Reset(); ++ } ++ ++ void set_connection_error_with_reason_and_result_handler( ++ ConnectionErrorWithReasonAndResultCallback error_handler) { ++ CHECK(sequence_checker_.CalledOnValidSequence()); ++ error_with_reason_and_result_handler_ = std::move(error_handler); ++ error_handler_.Reset(); ++ error_with_reason_handler_.Reset(); + } + + // Returns true if an error was encountered. +@@ -155,7 +165,8 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) InterfaceEndpointClient + + // NOTE: |message| must have passed message header validation. + bool HandleIncomingMessage(Message* message); +- void NotifyError(const std::optional& reason); ++ void NotifyError(const std::optional& reason, ++ MojoResult error_result); + + // The following methods send interface control messages. + // They must only be called when the handle is not in pending association +@@ -345,6 +356,8 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) InterfaceEndpointClient + + base::OnceClosure error_handler_; + ConnectionErrorWithReasonCallback error_with_reason_handler_; ++ ConnectionErrorWithReasonAndResultCallback ++ error_with_reason_and_result_handler_; + bool encountered_error_ = false; + + const scoped_refptr task_runner_; +diff --git mojo/public/cpp/bindings/lib/associated_receiver.cc mojo/public/cpp/bindings/lib/associated_receiver.cc +index 4a0fbb27c1163..75a5c5fe70423 100644 +--- mojo/public/cpp/bindings/lib/associated_receiver.cc ++++ mojo/public/cpp/bindings/lib/associated_receiver.cc +@@ -49,6 +49,13 @@ void AssociatedReceiverBase::set_disconnect_with_reason_handler( + std::move(error_handler)); + } + ++void AssociatedReceiverBase::set_disconnect_with_reason_and_result_handler( ++ ConnectionErrorWithReasonAndResultCallback error_handler) { ++ DCHECK(is_bound()); ++ endpoint_client_->set_connection_error_with_reason_and_result_handler( ++ std::move(error_handler)); ++} ++ + void AssociatedReceiverBase::reset_on_disconnect() { + DCHECK(is_bound()); + set_disconnect_handler( +diff --git mojo/public/cpp/bindings/lib/binding_state.h mojo/public/cpp/bindings/lib/binding_state.h +index 3de514b2696ba..ac057000ea20b 100644 +--- mojo/public/cpp/bindings/lib/binding_state.h ++++ mojo/public/cpp/bindings/lib/binding_state.h +@@ -70,6 +70,13 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) BindingStateBase { + std::move(error_handler)); + } + ++ void set_connection_error_with_reason_and_result_handler( ++ ConnectionErrorWithReasonAndResultCallback error_handler) { ++ DCHECK(is_bound()); ++ endpoint_client_->set_connection_error_with_reason_and_result_handler( ++ std::move(error_handler)); ++ } ++ + bool is_bound() const { return !!router_; } + + MessagePipeHandle handle() const { +diff --git mojo/public/cpp/bindings/lib/connector.cc mojo/public/cpp/bindings/lib/connector.cc +index d9ec0189f1b9a..1d738ce04ff72 100644 +--- mojo/public/cpp/bindings/lib/connector.cc ++++ mojo/public/cpp/bindings/lib/connector.cc +@@ -438,6 +438,8 @@ void Connector::OnSyncHandleWatcherHandleReady(const char* interface_name, + void Connector::OnHandleReadyInternal(MojoResult result) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + ++ handle_ready_result_ = result; ++ + if (result == MOJO_RESULT_FAILED_PRECONDITION) { + // No more messages on the pipe and the peer is closed. + HandleError(false /* force_pipe_reset */, false /* force_async_handler */); +diff --git mojo/public/cpp/bindings/lib/interface_endpoint_client.cc mojo/public/cpp/bindings/lib/interface_endpoint_client.cc +index 8001dffa21977..87e318fd1334e 100644 +--- mojo/public/cpp/bindings/lib/interface_endpoint_client.cc ++++ mojo/public/cpp/bindings/lib/interface_endpoint_client.cc +@@ -731,7 +731,8 @@ bool InterfaceEndpointClient::HandleIncomingMessage(Message* message) { + } + + void InterfaceEndpointClient::NotifyError( +- const std::optional& reason) { ++ const std::optional& reason, ++ MojoResult error_result) { + TRACE_EVENT("toplevel", "Closed mojo endpoint", + [&](perfetto::EventContext& ctx) { + auto* info = ctx.event()->set_chrome_mojo_event_info(); +@@ -767,6 +768,14 @@ void InterfaceEndpointClient::NotifyError( + } else { + std::move(error_with_reason_handler_).Run(0, std::string()); + } ++ } else if (error_with_reason_and_result_handler_) { ++ if (reason) { ++ std::move(error_with_reason_and_result_handler_) ++ .Run(reason->custom_reason, reason->description, error_result); ++ } else { ++ std::move(error_with_reason_and_result_handler_) ++ .Run(0, std::string(), error_result); ++ } + } + } + +@@ -905,7 +914,8 @@ void InterfaceEndpointClient::OnAssociationEvent( + task_runner_->PostTask(FROM_HERE, + base::BindOnce(&InterfaceEndpointClient::NotifyError, + weak_ptr_factory_.GetWeakPtr(), +- handle_.disconnect_reason())); ++ handle_.disconnect_reason(), ++ MOJO_RESULT_OK)); + } + } + +diff --git mojo/public/cpp/bindings/lib/interface_ptr_state.h mojo/public/cpp/bindings/lib/interface_ptr_state.h +index b6b88ee9651ba..6d75cb0cbd531 100644 +--- mojo/public/cpp/bindings/lib/interface_ptr_state.h ++++ mojo/public/cpp/bindings/lib/interface_ptr_state.h +@@ -224,6 +224,15 @@ class InterfacePtrState : public InterfacePtrStateBase { + std::move(error_handler)); + } + ++ void set_connection_error_with_reason_and_result_handler( ++ ConnectionErrorWithReasonAndResultCallback error_handler) { ++ ConfigureProxyIfNecessary(); ++ ++ DCHECK(endpoint_client()); ++ endpoint_client()->set_connection_error_with_reason_and_result_handler( ++ std::move(error_handler)); ++ } ++ + void set_idle_handler(base::TimeDelta timeout, + base::RepeatingClosure handler) { + ConfigureProxyIfNecessary(); +diff --git mojo/public/cpp/bindings/lib/multiplex_router.cc mojo/public/cpp/bindings/lib/multiplex_router.cc +index c7d6a0e7b5e96..399016fbd5e71 100644 +--- mojo/public/cpp/bindings/lib/multiplex_router.cc ++++ mojo/public/cpp/bindings/lib/multiplex_router.cc +@@ -90,6 +90,12 @@ class MultiplexRouter::InterfaceEndpoint + disconnect_reason_ = disconnect_reason; + } + ++ MojoResult error_result() const { return error_result_; } ++ void set_error_result(MojoResult error_result) { ++ router_->AssertLockAcquired(); ++ error_result_ = error_result; ++ } ++ + base::SequencedTaskRunner* task_runner() const { return task_runner_.get(); } + + InterfaceEndpointClient* client() const { return client_; } +@@ -245,6 +251,7 @@ class MultiplexRouter::InterfaceEndpoint + bool handle_created_; + + std::optional disconnect_reason_; ++ MojoResult error_result_ = MOJO_RESULT_OK; + + // The task runner on which |client_|'s methods can be called. + scoped_refptr task_runner_; +@@ -842,6 +849,8 @@ void MultiplexRouter::OnPipeConnectionError(bool force_async_dispatch) { + for (uint64_t request_id : request_ids) + endpoint->client()->ForgetAsyncRequest(request_id); + ++ endpoint->set_error_result(connector_.handle_ready_result()); ++ + tasks_.push_back(Task::CreateNotifyErrorTask(endpoint.get())); + } + +@@ -1032,7 +1041,7 @@ bool MultiplexRouter::ProcessNotifyErrorTask( + // It is safe to call into |client| without the lock. Because |client| is + // always accessed on the same sequence, including DetachEndpointClient(). + MayAutoUnlock unlocker(&lock_); +- client->NotifyError(disconnect_reason); ++ client->NotifyError(disconnect_reason, endpoint->error_result()); + } + return true; + } +diff --git mojo/public/cpp/bindings/receiver.h mojo/public/cpp/bindings/receiver.h +index 8d51fbad3832e..a2eff11227539 100644 +--- mojo/public/cpp/bindings/receiver.h ++++ mojo/public/cpp/bindings/receiver.h +@@ -106,6 +106,12 @@ class Receiver { + internal_state_.set_connection_error_with_reason_handler( + std::move(error_handler)); + } ++ void set_disconnect_with_reason_and_result_handler( ++ ConnectionErrorWithReasonAndResultCallback error_handler) { ++ DCHECK(is_bound()); ++ internal_state_.set_connection_error_with_reason_and_result_handler( ++ std::move(error_handler)); ++ } + + // Resets this Receiver to an unbound state. An unbound Receiver will NEVER + // schedule method calls or disconnection notifications, and any pending tasks +diff --git mojo/public/cpp/bindings/receiver_set.cc mojo/public/cpp/bindings/receiver_set.cc +index ede8e5973b576..6f22981831ede 100644 +--- mojo/public/cpp/bindings/receiver_set.cc ++++ mojo/public/cpp/bindings/receiver_set.cc +@@ -69,9 +69,10 @@ void ReceiverSetState::Entry::DidDispatchOrReject() { + } + + void ReceiverSetState::Entry::OnDisconnect(uint32_t custom_reason_code, +- const std::string& description) { ++ const std::string& description, ++ MojoResult error_result) { + WillDispatch(); +- state_.OnDisconnect(id_, custom_reason_code, description); ++ state_.OnDisconnect(id_, custom_reason_code, description, error_result); + } + + ReceiverSetState::ReceiverSetState() = default; +@@ -81,12 +82,21 @@ ReceiverSetState::~ReceiverSetState() = default; + void ReceiverSetState::set_disconnect_handler(base::RepeatingClosure handler) { + disconnect_handler_ = std::move(handler); + disconnect_with_reason_handler_.Reset(); ++ disconnect_with_reason_and_result_handler_.Reset(); + } + + void ReceiverSetState::set_disconnect_with_reason_handler( + RepeatingConnectionErrorWithReasonCallback handler) { + disconnect_with_reason_handler_ = std::move(handler); + disconnect_handler_.Reset(); ++ disconnect_with_reason_and_result_handler_.Reset(); ++} ++ ++void ReceiverSetState::set_disconnect_with_reason_and_result_handler( ++ RepeatingConnectionErrorWithReasonAndResultCallback handler) { ++ disconnect_with_reason_and_result_handler_ = std::move(handler); ++ disconnect_handler_.Reset(); ++ disconnect_with_reason_handler_.Reset(); + } + + ReportBadMessageCallback ReceiverSetState::GetBadMessageCallback() { +@@ -159,7 +169,8 @@ void ReceiverSetState::SetDispatchContext(void* context, + + void ReceiverSetState::OnDisconnect(ReceiverId id, + uint32_t custom_reason_code, +- const std::string& description) { ++ const std::string& description, ++ MojoResult error_result) { + auto it = entries_.find(id); + CHECK(it != entries_.end(), base::NotFatalUntil::M130); + +@@ -171,6 +182,10 @@ void ReceiverSetState::OnDisconnect(ReceiverId id, + disconnect_handler_.Run(); + else if (disconnect_with_reason_handler_) + disconnect_with_reason_handler_.Run(custom_reason_code, description); ++ else if (disconnect_with_reason_and_result_handler_) { ++ disconnect_with_reason_and_result_handler_.Run(custom_reason_code, ++ description, error_result); ++ } + } + + } // namespace mojo +diff --git mojo/public/cpp/bindings/receiver_set.h mojo/public/cpp/bindings/receiver_set.h +index 41b31247e9e50..ecd699772381b 100644 +--- mojo/public/cpp/bindings/receiver_set.h ++++ mojo/public/cpp/bindings/receiver_set.h +@@ -72,7 +72,8 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) ReceiverSetState { + virtual void* GetContext() = 0; + virtual void InstallDispatchHooks( + std::unique_ptr filter, +- RepeatingConnectionErrorWithReasonCallback disconnect_handler) = 0; ++ RepeatingConnectionErrorWithReasonAndResultCallback ++ disconnect_handler) = 0; + virtual void FlushForTesting() = 0; + virtual void ResetWithReason(uint32_t custom_reason_code, + const std::string& description) = 0; +@@ -94,7 +95,8 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) ReceiverSetState { + void WillDispatch(); + void DidDispatchOrReject(); + void OnDisconnect(uint32_t custom_reason_code, +- const std::string& description); ++ const std::string& description, ++ MojoResult error_result); + + // RAW_PTR_EXCLUSION: Binary size increase. + RAW_PTR_EXCLUSION ReceiverSetState& state_; +@@ -130,6 +132,8 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) ReceiverSetState { + void set_disconnect_handler(base::RepeatingClosure handler); + void set_disconnect_with_reason_handler( + RepeatingConnectionErrorWithReasonCallback handler); ++ void set_disconnect_with_reason_and_result_handler( ++ RepeatingConnectionErrorWithReasonAndResultCallback handler); + + ReportBadMessageCallback GetBadMessageCallback(); + ReceiverId Add(std::unique_ptr receiver, +@@ -142,11 +146,14 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) ReceiverSetState { + void SetDispatchContext(void* context, ReceiverId receiver_id); + void OnDisconnect(ReceiverId id, + uint32_t custom_reason_code, +- const std::string& description); ++ const std::string& description, ++ MojoResult error_result); + + private: + base::RepeatingClosure disconnect_handler_; + RepeatingConnectionErrorWithReasonCallback disconnect_with_reason_handler_; ++ RepeatingConnectionErrorWithReasonAndResultCallback ++ disconnect_with_reason_and_result_handler_; + ReceiverId next_receiver_id_ = 0; + EntryMap entries_; + raw_ptr current_context_ = nullptr; +@@ -489,11 +496,12 @@ class ReceiverSetBase { + const void* GetContext() const override { return &context_; } + void* GetContext() override { return &context_; } + +- void InstallDispatchHooks(std::unique_ptr filter, +- RepeatingConnectionErrorWithReasonCallback +- disconnect_handler) override { ++ void InstallDispatchHooks( ++ std::unique_ptr filter, ++ RepeatingConnectionErrorWithReasonAndResultCallback ++ disconnect_handler) override { + receiver_.SetFilter(std::move(filter)); +- receiver_.set_disconnect_with_reason_handler( ++ receiver_.set_disconnect_with_reason_and_result_handler( + std::move(disconnect_handler)); + } + +diff --git mojo/public/cpp/bindings/remote.h mojo/public/cpp/bindings/remote.h +index e912da6086552..7119a8d35f242 100644 +--- mojo/public/cpp/bindings/remote.h ++++ mojo/public/cpp/bindings/remote.h +@@ -155,6 +155,11 @@ class Remote { + internal_state_.set_connection_error_with_reason_handler( + std::move(handler)); + } ++ void set_disconnect_with_reason_and_result_handler( ++ ConnectionErrorWithReasonAndResultCallback handler) { ++ internal_state_.set_connection_error_with_reason_and_result_handler( ++ std::move(handler)); ++ } + + // A convenient helper that resets this Remote on disconnect. Note that this + // replaces any previously set disconnection handler. Must be called on a