Commit 1ad77242 authored by Derek Cheng's avatar Derek Cheng Committed by Commit Bot

[Presentation API] Fix race condition where Mojo pipes aren't closed.

Race condition introduced in:
https://chromium-review.googlesource.com/c/chromium/src/+/724724

The crash is caused by a race condition, where the the renderer attempted
to register another PresentationController to PresentationServiceImpl
while there is still a (soon-to-be invalid) one already. When we moved
the PresentationController implementation from PresentationDispatcher to
blink::PresentationController, we are now creating/destoying the
PresentationController across navigation (instead of having it
long-lived in the PresentationDispatcher / RenderFrameImpl).

The fix is to close all message pipes / Reset() in
PresentationServiceImpl when a Mojo connection error is detected.
This way, the PresentationServiceImpl will be in a clean state when
the renderer connects to it again.

This also fixes PresentationReceiver's behavior of obtaining a
connection to PresentationService and immediately dropping it after
calling SetReceiver(), which would let to Reset() getting called with
this patch.

To merge back to 66 (if possible) and 67.

Bug: 832176
Change-Id: Ic7cd2601a107024143936fa9e1ae197505e4cf64
Reviewed-on: https://chromium-review.googlesource.com/1011289Reviewed-by: default avatarmark a. foltz <mfoltz@chromium.org>
Reviewed-by: default avatarDerek Cheng <imcheng@chromium.org>
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551057}
parent 2aaf6945
...@@ -68,6 +68,9 @@ PresentationServiceImpl::PresentationServiceImpl( ...@@ -68,6 +68,9 @@ PresentationServiceImpl::PresentationServiceImpl(
if (auto* delegate = GetPresentationServiceDelegate()) if (auto* delegate = GetPresentationServiceDelegate())
delegate->AddObserver(render_process_id_, render_frame_id_, this); delegate->AddObserver(render_process_id_, render_frame_id_, this);
bindings_.set_connection_error_handler(base::BindRepeating(
&PresentationServiceImpl::OnConnectionError, base::Unretained(this)));
} }
PresentationServiceImpl::~PresentationServiceImpl() { PresentationServiceImpl::~PresentationServiceImpl() {
...@@ -118,6 +121,8 @@ void PresentationServiceImpl::SetController( ...@@ -118,6 +121,8 @@ void PresentationServiceImpl::SetController(
return; return;
} }
controller_ = std::move(controller); controller_ = std::move(controller);
controller_.set_connection_error_handler(base::BindOnce(
&PresentationServiceImpl::OnConnectionError, base::Unretained(this)));
} }
void PresentationServiceImpl::SetReceiver( void PresentationServiceImpl::SetReceiver(
...@@ -138,6 +143,8 @@ void PresentationServiceImpl::SetReceiver( ...@@ -138,6 +143,8 @@ void PresentationServiceImpl::SetReceiver(
} }
receiver_ = std::move(receiver); receiver_ = std::move(receiver);
receiver_.set_connection_error_handler(base::BindOnce(
&PresentationServiceImpl::OnConnectionError, base::Unretained(this)));
receiver_delegate_->RegisterReceiverConnectionAvailableCallback( receiver_delegate_->RegisterReceiverConnectionAvailableCallback(
base::Bind(&PresentationServiceImpl::OnReceiverConnectionAvailable, base::Bind(&PresentationServiceImpl::OnReceiverConnectionAvailable,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
...@@ -387,6 +394,10 @@ bool PresentationServiceImpl::FrameMatches( ...@@ -387,6 +394,10 @@ bool PresentationServiceImpl::FrameMatches(
render_frame_host->GetRoutingID() == render_frame_id_; render_frame_host->GetRoutingID() == render_frame_id_;
} }
void PresentationServiceImpl::OnConnectionError() {
Reset();
}
PresentationServiceDelegate* PresentationServiceDelegate*
PresentationServiceImpl::GetPresentationServiceDelegate() { PresentationServiceImpl::GetPresentationServiceDelegate() {
return receiver_delegate_ return receiver_delegate_
......
...@@ -239,6 +239,10 @@ class CONTENT_EXPORT PresentationServiceImpl ...@@ -239,6 +239,10 @@ class CONTENT_EXPORT PresentationServiceImpl
// Returns true if this object is associated with |render_frame_host|. // Returns true if this object is associated with |render_frame_host|.
bool FrameMatches(content::RenderFrameHost* render_frame_host) const; bool FrameMatches(content::RenderFrameHost* render_frame_host) const;
// Invoked on Mojo connection error. Closes all Mojo message pipes held by
// |this|.
void OnConnectionError();
// Returns |controller_delegate| if current frame is controller frame; Returns // Returns |controller_delegate| if current frame is controller frame; Returns
// |receiver_delegate| if current frame is receiver frame. // |receiver_delegate| if current frame is receiver frame.
PresentationServiceDelegate* GetPresentationServiceDelegate(); PresentationServiceDelegate* GetPresentationServiceDelegate();
......
...@@ -66,13 +66,12 @@ ScriptPromise PresentationReceiver::connectionList(ScriptState* script_state) { ...@@ -66,13 +66,12 @@ ScriptPromise PresentationReceiver::connectionList(ScriptState* script_state) {
void PresentationReceiver::Init() { void PresentationReceiver::Init() {
DCHECK(!receiver_binding_.is_bound()); DCHECK(!receiver_binding_.is_bound());
mojom::blink::PresentationServicePtr presentation_service;
auto* interface_provider = GetFrame()->Client()->GetInterfaceProvider(); auto* interface_provider = GetFrame()->Client()->GetInterfaceProvider();
interface_provider->GetInterface(mojo::MakeRequest(&presentation_service)); interface_provider->GetInterface(mojo::MakeRequest(&presentation_service_));
mojom::blink::PresentationReceiverPtr receiver_ptr; mojom::blink::PresentationReceiverPtr receiver_ptr;
receiver_binding_.Bind(mojo::MakeRequest(&receiver_ptr)); receiver_binding_.Bind(mojo::MakeRequest(&receiver_ptr));
presentation_service->SetReceiver(std::move(receiver_ptr)); presentation_service_->SetReceiver(std::move(receiver_ptr));
} }
void PresentationReceiver::OnReceiverTerminated() { void PresentationReceiver::OnReceiverTerminated() {
......
...@@ -79,6 +79,7 @@ class MODULES_EXPORT PresentationReceiver final ...@@ -79,6 +79,7 @@ class MODULES_EXPORT PresentationReceiver final
Member<PresentationConnectionList> connection_list_; Member<PresentationConnectionList> connection_list_;
mojo::Binding<mojom::blink::PresentationReceiver> receiver_binding_; mojo::Binding<mojom::blink::PresentationReceiver> receiver_binding_;
mojom::blink::PresentationServicePtr presentation_service_;
WebPresentationClient* client_; WebPresentationClient* client_;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment