Commit f56eaabe authored by Wez's avatar Wez Committed by Chromium LUCI CQ

[cast_api_bindings] Allow for MessagePort deletion by message handlers.

MessagePort implementations may be deleted during an OnMessage() call to
their MessagePort::Receiver.  Fix the implementations to cope with this
by early-exiting to avoid touching MessagePort fields after deletion.

Bug: b/175356780
Change-Id: Ib23e1b6a56d2ba730b471829d9680d899ff09217
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2590033
Auto-Submit: Wez <wez@chromium.org>
Commit-Queue: Kevin Marshall <kmarshall@chromium.org>
Reviewed-by: default avatarKevin Marshall <kmarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836657}
parent ffac4483
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "components/cast/message_port/message_port_fuchsia.h" #include "components/cast/message_port/message_port_fuchsia.h"
#include "base/fuchsia/fuchsia_logging.h" #include "base/fuchsia/fuchsia_logging.h"
#include "base/memory/weak_ptr.h"
#include "base/notreached.h" #include "base/notreached.h"
#include "fuchsia/base/mem_buffer_util.h" #include "fuchsia/base/mem_buffer_util.h"
#include "fuchsia/fidl/chromium/cast/cpp/fidl.h" #include "fuchsia/fidl/chromium/cast/cpp/fidl.h"
...@@ -61,6 +62,7 @@ class MessagePortFuchsiaClient : public MessagePortFuchsia { ...@@ -61,6 +62,7 @@ class MessagePortFuchsiaClient : public MessagePortFuchsia {
LOG(ERROR) << "PostMessage failed, reason: " LOG(ERROR) << "PostMessage failed, reason: "
<< static_cast<int32_t>(result.err()); << static_cast<int32_t>(result.err());
ReportPipeError(); ReportPipeError();
return;
} }
message_queue_.pop_front(); message_queue_.pop_front();
...@@ -80,7 +82,11 @@ class MessagePortFuchsiaClient : public MessagePortFuchsia { ...@@ -80,7 +82,11 @@ class MessagePortFuchsiaClient : public MessagePortFuchsia {
// Helpers for reading and writing messages on the |port_| // Helpers for reading and writing messages on the |port_|
void OnMessageReady(fuchsia::web::WebMessage message) { void OnMessageReady(fuchsia::web::WebMessage message) {
auto status = ReceiveMessageFromFidl(std::move(message)); base::WeakPtr<MessagePortFuchsia> weak_this = weak_factory_.GetWeakPtr();
auto status = ExtractAndHandleMessageFromFidl(std::move(message));
if (!weak_this)
return;
if (status) { if (status) {
LOG(WARNING) << "Received bad message, error: " LOG(WARNING) << "Received bad message, error: "
<< static_cast<int32_t>(*status); << static_cast<int32_t>(*status);
...@@ -100,6 +106,8 @@ class MessagePortFuchsiaClient : public MessagePortFuchsia { ...@@ -100,6 +106,8 @@ class MessagePortFuchsiaClient : public MessagePortFuchsia {
} }
fuchsia::web::MessagePortPtr port_; fuchsia::web::MessagePortPtr port_;
const base::WeakPtrFactory<MessagePortFuchsia> weak_factory_{this};
}; };
// A MessagePortFuchsia instantiated with an // A MessagePortFuchsia instantiated with an
...@@ -165,7 +173,11 @@ class MessagePortFuchsiaServer : public MessagePortFuchsia, ...@@ -165,7 +173,11 @@ class MessagePortFuchsiaServer : public MessagePortFuchsia,
// fuchsia::web::MessagePort implementation. // fuchsia::web::MessagePort implementation.
void PostMessage(fuchsia::web::WebMessage message, void PostMessage(fuchsia::web::WebMessage message,
PostMessageCallback callback) final { PostMessageCallback callback) final {
auto status = ReceiveMessageFromFidl(std::move(message)); base::WeakPtr<MessagePortFuchsia> weak_this = weak_factory_.GetWeakPtr();
auto status = ExtractAndHandleMessageFromFidl(std::move(message));
if (!weak_this)
return;
if (status) { if (status) {
LOG(WARNING) << "Received bad message, error: " LOG(WARNING) << "Received bad message, error: "
<< static_cast<int32_t>(*status); << static_cast<int32_t>(*status);
...@@ -192,6 +204,8 @@ class MessagePortFuchsiaServer : public MessagePortFuchsia, ...@@ -192,6 +204,8 @@ class MessagePortFuchsiaServer : public MessagePortFuchsia,
fidl::Binding<fuchsia::web::MessagePort> binding_; fidl::Binding<fuchsia::web::MessagePort> binding_;
ReceiveMessageCallback pending_receive_message_callback_; ReceiveMessageCallback pending_receive_message_callback_;
const base::WeakPtrFactory<MessagePortFuchsia> weak_factory_{this};
}; };
} // namespace } // namespace
...@@ -277,7 +291,8 @@ MessagePortFuchsia::MessagePortFuchsia(PortType port_type) ...@@ -277,7 +291,8 @@ MessagePortFuchsia::MessagePortFuchsia(PortType port_type)
MessagePortFuchsia::~MessagePortFuchsia() = default; MessagePortFuchsia::~MessagePortFuchsia() = default;
base::Optional<fuchsia::web::FrameError> base::Optional<fuchsia::web::FrameError>
MessagePortFuchsia::ReceiveMessageFromFidl(fuchsia::web::WebMessage message) { MessagePortFuchsia::ExtractAndHandleMessageFromFidl(
fuchsia::web::WebMessage message) {
DCHECK(receiver_); DCHECK(receiver_);
if (!message.has_data()) { if (!message.has_data()) {
return fuchsia::web::FrameError::NO_DATA_IN_MESSAGE; return fuchsia::web::FrameError::NO_DATA_IN_MESSAGE;
......
...@@ -61,9 +61,12 @@ class MessagePortFuchsia : public cast_api_bindings::MessagePort { ...@@ -61,9 +61,12 @@ class MessagePortFuchsia : public cast_api_bindings::MessagePort {
// Delivers a message to FIDL from |message_queue_|. // Delivers a message to FIDL from |message_queue_|.
virtual void DeliverMessageToFidl() = 0; virtual void DeliverMessageToFidl() = 0;
// Receives a |message| from FIDL into |message_queue_|. Returns a value if // Extracts the message and transferrables from |message_queue_| and invokes
// an error occurred. // |receiver_.OnMessage()| to process it. Returns a FrameError if extracting
base::Optional<fuchsia::web::FrameError> ReceiveMessageFromFidl( // or handling the message fails.
// Note that handling the message may result in |this| being deleted before
// the call returns.
base::Optional<fuchsia::web::FrameError> ExtractAndHandleMessageFromFidl(
fuchsia::web::WebMessage message); fuchsia::web::WebMessage message);
void OnZxError(zx_status_t status); void OnZxError(zx_status_t status);
......
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