mirror of
				https://bitbucket.org/chromiumembedded/cef
				synced 2025-06-05 21:39:12 +02:00 
			
		
		
		
	Reduce the frequency of connection-related renderer crashes (see #3664)
- Use ResetWithReason to report intentional browser side disconnects of existing Mojo connections. Don't retry for those disconnects. - Add set_disconnect_with_reason_and_result_handler in Chromium/Mojo to expose the MojoResult code for failed connections, allowing identification of connections that are intentionally unbound on the browser side. - Optimize initial reconnect delay for known disconnect cases such as navigation-related and bfcache changes. - Remove connection timeout and increase total connection deadline by 100% to further reduce crash rates on slower machines. - Only fail fatally for main frames (not sub-frames) in cases where the connection fails or disconnects for unknown reasons. - Improve connection debug logging when running with `--enable-logging --vmodule=*frame*=1 --log-file=C:\temp\log.txt`
This commit is contained in:
		| @@ -17,7 +17,12 @@ | ||||
| CefBrowserFrame::CefBrowserFrame( | ||||
|     content::RenderFrameHost* render_frame_host, | ||||
|     mojo::PendingReceiver<cef::mojom::BrowserFrame> 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<cef::mojom::RenderFrame> render_frame_remote; | ||||
|     render_frame_remote.Bind(std::move(render_frame)); | ||||
|     render_frame_remote->FrameAttachedAck(/*allow=*/false); | ||||
|     render_frame_remote.ResetWithReason( | ||||
|         static_cast<uint32_t>(frame_util::ResetReason::kExcluded), "Excluded"); | ||||
|   } | ||||
| } | ||||
|  | ||||
|   | ||||
| @@ -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<cef::mojom::BrowserFrame> { | ||||
| class CefBrowserFrame : public CefFrameServiceBase<cef::mojom::BrowserFrame> { | ||||
|  public: | ||||
|   CefBrowserFrame(content::RenderFrameHost* render_frame_host, | ||||
|                   mojo::PendingReceiver<cef::mojom::BrowserFrame> receiver); | ||||
| @@ -44,9 +43,6 @@ class CefBrowserFrame | ||||
|       std::optional<std::vector<cef::mojom::DraggableRegionEntryPtr>> regions) | ||||
|       override; | ||||
|  | ||||
|   // FrameServiceBase methods: | ||||
|   bool ShouldCloseOnFinishNavigation() const override { return false; } | ||||
|  | ||||
|   CefRefPtr<CefFrameHostImpl> GetFrameHost(bool prefer_speculative, | ||||
|                                            bool* is_excluded = nullptr) const; | ||||
| }; | ||||
|   | ||||
| @@ -101,6 +101,7 @@ CefFrameHostImpl::CefFrameHostImpl(scoped_refptr<CefBrowserInfo> 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<uint32_t>(frame_util::ResetReason::kDetached), "Detached"); | ||||
| } | ||||
|  | ||||
| void CefFrameHostImpl::MaybeReAttach( | ||||
|     scoped_refptr<CefBrowserInfo> 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( | ||||
|   | ||||
| @@ -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_; | ||||
|   | ||||
| @@ -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 <typename Interface> | ||||
| class FrameServiceBase : public Interface, public WebContentsObserver { | ||||
| class CefFrameServiceBase : public Interface, | ||||
|                             public content::WebContentsObserver { | ||||
|  public: | ||||
|   FrameServiceBase(RenderFrameHost* render_frame_host, | ||||
|                    mojo::PendingReceiver<Interface> pending_receiver) | ||||
|       : WebContentsObserver( | ||||
|             WebContents::FromRenderFrameHost(render_frame_host)), | ||||
|   CefFrameServiceBase(content::RenderFrameHost* render_frame_host, | ||||
|                       mojo::PendingReceiver<Interface> 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<uint32_t>(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<RenderFrameHost> render_frame_host_ = nullptr; | ||||
|   const url::Origin origin_; | ||||
|   const raw_ptr<content::RenderFrameHost> render_frame_host_ = nullptr; | ||||
|   mojo::Receiver<Interface> receiver_; | ||||
| }; | ||||
|  | ||||
| }  // namespace content | ||||
|  | ||||
| #endif  // CEF_LIBCEF_BROWSER_FRAME_SERVICE_BASE_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_ | ||||
|   | ||||
| @@ -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); | ||||
|  | ||||
|   | ||||
| @@ -56,11 +56,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); | ||||
| @@ -425,7 +429,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; | ||||
|  | ||||
| @@ -482,26 +486,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); | ||||
| @@ -516,7 +520,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)); | ||||
| } | ||||
|  | ||||
| @@ -531,28 +535,25 @@ const mojo::Remote<cef::mojom::BrowserFrame>& 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 | ||||
| @@ -562,18 +563,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; | ||||
| @@ -606,7 +602,8 @@ std::string CefFrameImpl::GetDisconnectDebugString( | ||||
|     state_str += ", SUB_FRAME"; | ||||
|   } | ||||
|  | ||||
|   if (custom_reason > 0) { | ||||
|   if (custom_reason != | ||||
|       static_cast<uint32_t>(frame_util::ResetReason::kNoReason)) { | ||||
|     state_str += ", custom_reason=" + base::NumberToString(custom_reason); | ||||
|   } | ||||
|  | ||||
| @@ -614,12 +611,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(). | ||||
| @@ -628,56 +630,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<uint32_t>(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, | ||||
| @@ -735,7 +789,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. | ||||
| @@ -746,6 +802,8 @@ void CefFrameImpl::FrameAttachedAck(bool allow) { | ||||
|     return; | ||||
|   } | ||||
|  | ||||
|   ever_connected_ = true; | ||||
|  | ||||
|   auto& browser_frame = GetBrowserFrame(); | ||||
|   CHECK(browser_frame); | ||||
|  | ||||
| @@ -755,12 +813,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()) { | ||||
|   | ||||
| @@ -117,20 +117,17 @@ class CefFrameImpl | ||||
|   using BrowserFrameType = mojo::Remote<cef::mojom::BrowserFrame>; | ||||
|   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_; | ||||
|  | ||||
|   | ||||
| @@ -746,5 +746,10 @@ patches = [ | ||||
|     # linux: Fix cannot allocate memory in static TLS block in dlopen libcef.so | ||||
|     # https://github.com/chromiumembedded/cef/issues/3616 | ||||
|     'name': 'third_party_sentencepiece_3616' | ||||
|   }, | ||||
|   { | ||||
|     # Expose Mojo Connector error state to Receiver disconnect handlers. | ||||
|     # https://github.com/chromiumembedded/cef/issues/3664 | ||||
|     'name': 'mojo_connect_result_3664' | ||||
|   } | ||||
| ] | ||||
|   | ||||
							
								
								
									
										459
									
								
								patch/patches/mojo_connect_result_3664.patch
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										459
									
								
								patch/patches/mojo_connect_result_3664.patch
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,459 @@ | ||||
| diff --git ipc/ipc_mojo_bootstrap.cc ipc/ipc_mojo_bootstrap.cc | ||||
| index da2bfab0e9064..449a36cbff63f 100644 | ||||
| --- ipc/ipc_mojo_bootstrap.cc | ||||
| +++ ipc/ipc_mojo_bootstrap.cc | ||||
| @@ -990,7 +990,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<void(uint32_t /* custom_reason */, | ||||
|                                   const std::string& /* description */)>; | ||||
| +using ConnectionErrorWithReasonAndResultCallback = | ||||
| +    base::OnceCallback<void(uint32_t /* custom_reason */, | ||||
| +                            const std::string& /* description */, | ||||
| +                            MojoResult /* error_result */)>; | ||||
| +using RepeatingConnectionErrorWithReasonAndResultCallback = | ||||
| +    base::RepeatingCallback<void(uint32_t /* custom_reason */, | ||||
| +                                 const std::string& /* description */, | ||||
| +                                 MojoResult /* error_result */)>; | ||||
|   | ||||
|  }  // 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<HandleSignalTracker> peer_remoteness_tracker_; | ||||
|   | ||||
|    std::atomic<bool> 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<DisconnectReason>& reason); | ||||
| +  void NotifyError(const std::optional<DisconnectReason>& 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<base::SequencedTaskRunner> 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<DisconnectReason>& reason) { | ||||
| +    const std::optional<DisconnectReason>& 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<DisconnectReason> disconnect_reason_; | ||||
| +  MojoResult error_result_ = MOJO_RESULT_OK; | ||||
|   | ||||
|    // The task runner on which |client_|'s methods can be called. | ||||
|    scoped_refptr<base::SequencedTaskRunner> 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<MessageFilter> 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<ReceiverState> 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<void, DanglingUntriaged> 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<MessageFilter> filter, | ||||
| -                              RepeatingConnectionErrorWithReasonCallback | ||||
| -                                  disconnect_handler) override { | ||||
| +    void InstallDispatchHooks( | ||||
| +        std::unique_ptr<MessageFilter> 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 | ||||
| @@ -1,8 +1,8 @@ | ||||
| diff --git content/browser/renderer_host/render_frame_host_impl.cc content/browser/renderer_host/render_frame_host_impl.cc | ||||
| index 90cd61d803a58..f41b26538e4ec 100644 | ||||
| index 6223e072246c2..16e112ca96554 100644 | ||||
| --- content/browser/renderer_host/render_frame_host_impl.cc | ||||
| +++ content/browser/renderer_host/render_frame_host_impl.cc | ||||
| @@ -9153,6 +9153,16 @@ void RenderFrameHostImpl::CreateNewWindow( | ||||
| @@ -9130,6 +9130,16 @@ void RenderFrameHostImpl::CreateNewWindow( | ||||
|      return; | ||||
|    } | ||||
|   | ||||
| @@ -19,7 +19,7 @@ index 90cd61d803a58..f41b26538e4ec 100644 | ||||
|    // Otherwise, consume user activation before we proceed. In particular, it is | ||||
|    // important to do this before we return from the |opener_suppressed| case | ||||
|    // below. | ||||
| @@ -11495,6 +11505,7 @@ void RenderFrameHostImpl::CommitNavigation( | ||||
| @@ -11472,6 +11482,7 @@ void RenderFrameHostImpl::CommitNavigation( | ||||
|    auto browser_calc_origin_to_commit = | ||||
|        navigation_request->GetOriginToCommitWithDebugInfo(); | ||||
|    if (!process_lock.is_error_page() && !is_mhtml_subframe && | ||||
| @@ -27,3 +27,11 @@ index 90cd61d803a58..f41b26538e4ec 100644 | ||||
|        !policy->CanAccessOrigin( | ||||
|            GetProcess()->GetID(), browser_calc_origin_to_commit.first.value(), | ||||
|            ChildProcessSecurityPolicyImpl::AccessType::kCanCommitNewOrigin)) { | ||||
| @@ -15455,6 +15466,7 @@ void RenderFrameHostImpl::DidCommitNavigation( | ||||
|   | ||||
|    if (interface_params) { | ||||
|      if (broker_receiver_.is_bound()) { | ||||
| +      DVLOG(1) << __func__ << ": broker receiver reset"; | ||||
|        broker_receiver_.reset(); | ||||
|      } | ||||
|      BindBrowserInterfaceBrokerReceiver( | ||||
|   | ||||
		Reference in New Issue
	
	Block a user