Allow a CefProcessMessage to remain valid after receipt (see issue #3126)
A reference to a received CefProcessMessage object and/or associated argument list can now be kept outside of the OnProcessMessageReceived callback. The argument list is no longer explicitly owned by the CefProcessMessage object and can be individually assigned to other CefValue types as needed (e.g. by passing to SetList, etc). Depending on client usage this could reduce the potential for unnecessary copies of the list contents. Received messages can also be sent back using SendProcessMessage (after which the CefProcessMessage would become invalid as discussed in issue #3123). This is not new behavior but we have now added explicit unit test coverage for it. This also no longer requires a copy of the argument list contents. Note that a received argument list is initially read-only for logical consistency. Assignment to another CefValue object could potentially remove the read-only status because it is not an intrinsic property of the underlying Chromium data type. This is fine because, at that point, ownership has been transfered to the new CefValue object and the original logical context (as part of the CefProcessMessage) no longer applies.
This commit is contained in:
parent
ff8f4a7217
commit
f3c513bafd
|
@ -33,7 +33,7 @@
|
||||||
// by hand. See the translator.README.txt file in the tools directory for
|
// by hand. See the translator.README.txt file in the tools directory for
|
||||||
// more information.
|
// more information.
|
||||||
//
|
//
|
||||||
// $hash=14eca959988209ba8f95037a47192fd50d64f2f1$
|
// $hash=547d03e9514a30b62d5aa11834deab87052dd6bd$
|
||||||
//
|
//
|
||||||
|
|
||||||
#ifndef CEF_INCLUDE_CAPI_CEF_CLIENT_CAPI_H_
|
#ifndef CEF_INCLUDE_CAPI_CEF_CLIENT_CAPI_H_
|
||||||
|
@ -168,8 +168,8 @@ typedef struct _cef_client_t {
|
||||||
|
|
||||||
///
|
///
|
||||||
// Called when a new message is received from a different process. Return true
|
// Called when a new message is received from a different process. Return true
|
||||||
// (1) if the message was handled or false (0) otherwise. Do not keep a
|
// (1) if the message was handled or false (0) otherwise. It is safe to keep
|
||||||
// reference to or attempt to access the message outside of this callback.
|
// a reference to |message| outside of this callback.
|
||||||
///
|
///
|
||||||
int(CEF_CALLBACK* on_process_message_received)(
|
int(CEF_CALLBACK* on_process_message_received)(
|
||||||
struct _cef_client_t* self,
|
struct _cef_client_t* self,
|
||||||
|
|
|
@ -33,7 +33,7 @@
|
||||||
// by hand. See the translator.README.txt file in the tools directory for
|
// by hand. See the translator.README.txt file in the tools directory for
|
||||||
// more information.
|
// more information.
|
||||||
//
|
//
|
||||||
// $hash=131544be2c5e916381f80854451538ad64a687a2$
|
// $hash=4ebf99611a11cc8714d710c37417fbd9f50f0618$
|
||||||
//
|
//
|
||||||
|
|
||||||
#ifndef CEF_INCLUDE_CAPI_CEF_RENDER_PROCESS_HANDLER_CAPI_H_
|
#ifndef CEF_INCLUDE_CAPI_CEF_RENDER_PROCESS_HANDLER_CAPI_H_
|
||||||
|
@ -150,8 +150,8 @@ typedef struct _cef_render_process_handler_t {
|
||||||
|
|
||||||
///
|
///
|
||||||
// Called when a new message is received from a different process. Return true
|
// Called when a new message is received from a different process. Return true
|
||||||
// (1) if the message was handled or false (0) otherwise. Do not keep a
|
// (1) if the message was handled or false (0) otherwise. It is safe to keep a
|
||||||
// reference to or attempt to access the message outside of this callback.
|
// reference to |message| outside of this callback.
|
||||||
///
|
///
|
||||||
int(CEF_CALLBACK* on_process_message_received)(
|
int(CEF_CALLBACK* on_process_message_received)(
|
||||||
struct _cef_render_process_handler_t* self,
|
struct _cef_render_process_handler_t* self,
|
||||||
|
|
|
@ -161,8 +161,8 @@ class CefClient : public virtual CefBaseRefCounted {
|
||||||
|
|
||||||
///
|
///
|
||||||
// Called when a new message is received from a different process. Return true
|
// Called when a new message is received from a different process. Return true
|
||||||
// if the message was handled or false otherwise. Do not keep a reference to
|
// if the message was handled or false otherwise. It is safe to keep a
|
||||||
// or attempt to access the message outside of this callback.
|
// reference to |message| outside of this callback.
|
||||||
///
|
///
|
||||||
/*--cef()--*/
|
/*--cef()--*/
|
||||||
virtual bool OnProcessMessageReceived(CefRefPtr<CefBrowser> browser,
|
virtual bool OnProcessMessageReceived(CefRefPtr<CefBrowser> browser,
|
||||||
|
|
|
@ -134,8 +134,8 @@ class CefRenderProcessHandler : public virtual CefBaseRefCounted {
|
||||||
|
|
||||||
///
|
///
|
||||||
// Called when a new message is received from a different process. Return true
|
// Called when a new message is received from a different process. Return true
|
||||||
// if the message was handled or false otherwise. Do not keep a reference to
|
// if the message was handled or false otherwise. It is safe to keep a
|
||||||
// or attempt to access the message outside of this callback.
|
// reference to |message| outside of this callback.
|
||||||
///
|
///
|
||||||
/*--cef()--*/
|
/*--cef()--*/
|
||||||
virtual bool OnProcessMessageReceived(CefRefPtr<CefBrowser> browser,
|
virtual bool OnProcessMessageReceived(CefRefPtr<CefBrowser> browser,
|
||||||
|
|
|
@ -526,10 +526,10 @@ void CefFrameHostImpl::SendMessage(const std::string& name,
|
||||||
if (auto client = browser->GetClient()) {
|
if (auto client = browser->GetClient()) {
|
||||||
auto& list_value = base::Value::AsListValue(arguments);
|
auto& list_value = base::Value::AsListValue(arguments);
|
||||||
CefRefPtr<CefProcessMessageImpl> message(new CefProcessMessageImpl(
|
CefRefPtr<CefProcessMessageImpl> message(new CefProcessMessageImpl(
|
||||||
name, const_cast<base::ListValue*>(&list_value), /*read_only=*/true));
|
name, std::move(const_cast<base::ListValue&>(list_value)),
|
||||||
|
/*read_only=*/true));
|
||||||
browser->GetClient()->OnProcessMessageReceived(
|
browser->GetClient()->OnProcessMessageReceived(
|
||||||
browser.get(), this, PID_RENDERER, message.get());
|
browser.get(), this, PID_RENDERER, message.get());
|
||||||
message->Detach();
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -3,6 +3,9 @@
|
||||||
// can be found in the LICENSE file.
|
// can be found in the LICENSE file.
|
||||||
|
|
||||||
#include "libcef/common/process_message_impl.h"
|
#include "libcef/common/process_message_impl.h"
|
||||||
|
|
||||||
|
#include <memory>
|
||||||
|
|
||||||
#include "libcef/common/values_impl.h"
|
#include "libcef/common/values_impl.h"
|
||||||
|
|
||||||
#include "base/logging.h"
|
#include "base/logging.h"
|
||||||
|
@ -17,30 +20,22 @@ CefProcessMessageImpl::CefProcessMessageImpl(const CefString& name,
|
||||||
CefRefPtr<CefListValue> arguments)
|
CefRefPtr<CefListValue> arguments)
|
||||||
: name_(name), arguments_(arguments) {
|
: name_(name), arguments_(arguments) {
|
||||||
DCHECK(!name_.empty());
|
DCHECK(!name_.empty());
|
||||||
DCHECK(arguments_);
|
DCHECK(arguments_ && arguments_->IsValid());
|
||||||
}
|
}
|
||||||
|
|
||||||
CefProcessMessageImpl::CefProcessMessageImpl(const CefString& name,
|
CefProcessMessageImpl::CefProcessMessageImpl(const CefString& name,
|
||||||
base::ListValue* arguments,
|
base::ListValue arguments,
|
||||||
bool read_only)
|
bool read_only)
|
||||||
: name_(name),
|
: name_(name) {
|
||||||
arguments_(
|
|
||||||
new CefListValueImpl(arguments, /*will_delete=*/false, read_only)),
|
|
||||||
should_detach_(true) {
|
|
||||||
DCHECK(!name_.empty());
|
DCHECK(!name_.empty());
|
||||||
|
|
||||||
|
auto new_obj = std::make_unique<base::ListValue>();
|
||||||
|
*new_obj = std::move(arguments);
|
||||||
|
arguments_ =
|
||||||
|
new CefListValueImpl(new_obj.release(), /*will_delete=*/true, read_only);
|
||||||
}
|
}
|
||||||
|
|
||||||
CefProcessMessageImpl::~CefProcessMessageImpl() {
|
CefProcessMessageImpl::~CefProcessMessageImpl() = default;
|
||||||
DCHECK(!should_detach_ || !arguments_->IsValid());
|
|
||||||
}
|
|
||||||
|
|
||||||
void CefProcessMessageImpl::Detach() {
|
|
||||||
DCHECK(IsValid());
|
|
||||||
DCHECK(should_detach_);
|
|
||||||
CefListValueImpl* value_impl =
|
|
||||||
static_cast<CefListValueImpl*>(arguments_.get());
|
|
||||||
ignore_result(value_impl->Detach(nullptr));
|
|
||||||
}
|
|
||||||
|
|
||||||
base::ListValue CefProcessMessageImpl::TakeArgumentList() {
|
base::ListValue CefProcessMessageImpl::TakeArgumentList() {
|
||||||
DCHECK(IsValid());
|
DCHECK(IsValid());
|
||||||
|
|
|
@ -15,22 +15,20 @@ class ListValue;
|
||||||
// CefProcessMessage implementation.
|
// CefProcessMessage implementation.
|
||||||
class CefProcessMessageImpl : public CefProcessMessage {
|
class CefProcessMessageImpl : public CefProcessMessage {
|
||||||
public:
|
public:
|
||||||
// Constructor for an owned list of arguments.
|
// Constructor for referencing existing |arguments|.
|
||||||
CefProcessMessageImpl(const CefString& name,
|
CefProcessMessageImpl(const CefString& name,
|
||||||
CefRefPtr<CefListValue> arguments);
|
CefRefPtr<CefListValue> arguments);
|
||||||
|
|
||||||
// Constructor for an unowned list of arguments.
|
// Constructor for creating a new CefListValue that takes ownership of
|
||||||
// Call Detach() when |arguments| is no longer valid.
|
// |arguments|.
|
||||||
CefProcessMessageImpl(const CefString& name,
|
CefProcessMessageImpl(const CefString& name,
|
||||||
base::ListValue* arguments,
|
base::ListValue arguments,
|
||||||
bool read_only);
|
bool read_only);
|
||||||
|
|
||||||
~CefProcessMessageImpl() override;
|
~CefProcessMessageImpl() override;
|
||||||
|
|
||||||
// Stop referencing the underlying argument list (which we never owned).
|
// Transfer ownership of the underlying argument list to the caller, or create
|
||||||
void Detach();
|
// a copy if the argument list is already owned by something else.
|
||||||
|
|
||||||
// Transfer ownership of the underlying argument list to the caller.
|
|
||||||
// TODO: Pass by reference instead of ownership if/when Mojo adds support
|
// TODO: Pass by reference instead of ownership if/when Mojo adds support
|
||||||
// for that.
|
// for that.
|
||||||
base::ListValue TakeArgumentList() WARN_UNUSED_RESULT;
|
base::ListValue TakeArgumentList() WARN_UNUSED_RESULT;
|
||||||
|
@ -45,7 +43,6 @@ class CefProcessMessageImpl : public CefProcessMessage {
|
||||||
private:
|
private:
|
||||||
const CefString name_;
|
const CefString name_;
|
||||||
CefRefPtr<CefListValue> arguments_;
|
CefRefPtr<CefListValue> arguments_;
|
||||||
const bool should_detach_ = false;
|
|
||||||
|
|
||||||
IMPLEMENT_REFCOUNTING(CefProcessMessageImpl);
|
IMPLEMENT_REFCOUNTING(CefProcessMessageImpl);
|
||||||
DISALLOW_COPY_AND_ASSIGN(CefProcessMessageImpl);
|
DISALLOW_COPY_AND_ASSIGN(CefProcessMessageImpl);
|
||||||
|
|
|
@ -398,10 +398,10 @@ void CefFrameImpl::SendMessage(const std::string& name, base::Value arguments) {
|
||||||
if (auto handler = app->GetRenderProcessHandler()) {
|
if (auto handler = app->GetRenderProcessHandler()) {
|
||||||
auto& list_value = base::Value::AsListValue(arguments);
|
auto& list_value = base::Value::AsListValue(arguments);
|
||||||
CefRefPtr<CefProcessMessageImpl> message(new CefProcessMessageImpl(
|
CefRefPtr<CefProcessMessageImpl> message(new CefProcessMessageImpl(
|
||||||
name, const_cast<base::ListValue*>(&list_value), /*read_only=*/true));
|
name, std::move(const_cast<base::ListValue&>(list_value)),
|
||||||
|
/*read_only=*/true));
|
||||||
handler->OnProcessMessageReceived(browser_, this, PID_BROWSER,
|
handler->OnProcessMessageReceived(browser_, this, PID_BROWSER,
|
||||||
message.get());
|
message.get());
|
||||||
message->Detach();
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue