From e0927d8460774bedf82e944e2baf663f52e200d1 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Wed, 20 Nov 2024 18:40:34 -0500 Subject: [PATCH] Improve logging of fatal renderer connection errors (see #3664) --- libcef/renderer/frame_impl.cc | 41 ++++++++++++++++++++++++++++------- libcef/renderer/frame_impl.h | 8 ++++++- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/libcef/renderer/frame_impl.cc b/libcef/renderer/frame_impl.cc index dec0b1ae9..73f4c1137 100644 --- a/libcef/renderer/frame_impl.cc +++ b/libcef/renderer/frame_impl.cc @@ -17,6 +17,7 @@ #endif #endif +#include "base/strings/string_number_conversions.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" #include "cef/include/cef_urlrequest.h" @@ -424,7 +425,7 @@ void CefFrameImpl::OnDetached() { browser_->FrameDetached(frame_); frame_ = nullptr; - OnDisconnect(DisconnectReason::DETACHED, std::string()); + OnDisconnect(DisconnectReason::DETACHED, 0, std::string()); browser_ = nullptr; @@ -539,24 +540,28 @@ const mojo::Remote& CefFrameImpl::GetBrowserFrame( void CefFrameImpl::OnBrowserFrameTimeout() { LOG(ERROR) << frame_debug_str_ << " connection timeout"; - OnDisconnect(DisconnectReason::CONNECT_TIMEOUT, std::string()); + OnDisconnect(DisconnectReason::CONNECT_TIMEOUT, 0, std::string()); } void CefFrameImpl::OnBrowserFrameDisconnect(uint32_t custom_reason, const std::string& description) { - OnDisconnect(DisconnectReason::BROWSER_FRAME_DISCONNECT, description); + OnDisconnect(DisconnectReason::BROWSER_FRAME_DISCONNECT, custom_reason, + description); } void CefFrameImpl::OnRenderFrameDisconnect(uint32_t custom_reason, const std::string& description) { - OnDisconnect(DisconnectReason::RENDER_FRAME_DISCONNECT, description); + OnDisconnect(DisconnectReason::RENDER_FRAME_DISCONNECT, custom_reason, + description); } // static std::string CefFrameImpl::GetDisconnectDebugString( ConnectionState connection_state, bool frame_is_valid, + bool frame_is_main, DisconnectReason reason, + uint32_t custom_reason, const std::string& description) { std::string reason_str; switch (reason) { @@ -595,16 +600,25 @@ std::string CefFrameImpl::GetDisconnectDebugString( if (!frame_is_valid) { state_str += ", FRAME_INVALID"; + } else if (frame_is_main) { + state_str += ", MAIN_FRAME"; + } else { + state_str += ", SUB_FRAME"; + } + + if (custom_reason > 0) { + state_str += ", custom_reason=" + base::NumberToString(custom_reason); } if (!description.empty()) { - state_str += ", " + description; + state_str += ", description=" + description; } return "(reason=" + reason_str + ", current_state=" + state_str + ")"; } void CefFrameImpl::OnDisconnect(DisconnectReason reason, + uint32_t custom_reason, const std::string& description) { // Ignore multiple calls in close proximity (which may occur if both // |browser_frame_| and |receiver_| disconnect). |frame_| will be nullptr @@ -621,8 +635,10 @@ void CefFrameImpl::OnDisconnect(DisconnectReason reason, 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, reason, + << GetDisconnectDebugString(connection_state, frame_is_valid, + frame_is_main, reason, custom_reason, description); browser_frame_.reset(); @@ -635,6 +651,12 @@ void CefFrameImpl::OnDisconnect(DisconnectReason reason, 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 @@ -651,7 +673,9 @@ void CefFrameImpl::OnDisconnect(DisconnectReason reason, // Trigger a crash in official builds. LOG(FATAL) << frame_debug_str_ << " connection retry failed " << GetDisconnectDebugString(connection_state, frame_is_valid, - reason, description); + frame_is_main, reason, + custom_reason, description) + << ", prior disconnects: " << browser_connect_retry_log_; } } } @@ -710,6 +734,7 @@ void CefFrameImpl::FrameAttachedAck(bool allow) { CHECK_EQ(ConnectionState::CONNECTION_PENDING, browser_connection_state_); browser_connection_state_ = ConnectionState::CONNECTION_ACKED; browser_connect_retry_ct_ = 0; + browser_connect_retry_log_.clear(); browser_connect_timer_.Stop(); if (!allow) { @@ -733,7 +758,7 @@ 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, std::string()); + OnDisconnect(DisconnectReason::BROWSER_FRAME_DETACHED, 0, std::string()); } void CefFrameImpl::SendMessage(const std::string& name, diff --git a/libcef/renderer/frame_impl.h b/libcef/renderer/frame_impl.h index b5cca460c..0d48aec39 100644 --- a/libcef/renderer/frame_impl.h +++ b/libcef/renderer/frame_impl.h @@ -138,7 +138,9 @@ class CefFrameImpl // Called if/when a disconnect occurs. This may occur due to frame navigation, // destruction, or insertion into the bfcache (when the browser-side frame // representation is destroyed and closes the connection). - void OnDisconnect(DisconnectReason reason, const std::string& description); + void OnDisconnect(DisconnectReason reason, + uint32_t custom_reason, + const std::string& description); // Send an action to the remote BrowserFrame. This will queue the action if // the remote frame is not yet attached. @@ -185,6 +187,8 @@ class CefFrameImpl // Number of times that browser reconnect has been attempted. size_t browser_connect_retry_ct_ = 0; + // Log of reasons why the reconnect failed. + std::string browser_connect_retry_log_; // Current browser connection state. enum class ConnectionState { @@ -196,7 +200,9 @@ class CefFrameImpl static std::string GetDisconnectDebugString(ConnectionState connection_state, bool frame_is_valid, + bool frame_is_main, DisconnectReason reason, + uint32_t custom_reason, const std::string& description); base::OneShotTimer browser_connect_timer_;