Skip to content

Commit

Permalink
Refactor some ResourceMessageReply usages.
Browse files Browse the repository at this point in the history 
There are some usages where these are allocated on the heap.
Mechanically refactor the ones where the scoped_ptr is like a nullable reference by adding a bool pending field.
Hand rewrite some of the previous factoring in PepperExtensionsCommonMessageFilter.

BUG=none

Review URL: https://codereview.chromium.org/231883002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@263730 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
bbudge@chromium.org committed Apr 14, 2014
1 parent 03fbc5b commit a850702
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 109 deletions.
84 changes: 34 additions & 50 deletions chrome/browser/renderer_host/pepper/pepper_extensions_common_message_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,26 +127,45 @@ int32_t PepperExtensionsCommonMessageFilter::OnPost(
ppapi::host::HostMessageContext* context,
const std::string& request_name,
base::ListValue& args) {
if (HandleRequest(context, request_name, &args, false))
return PP_OK;
else
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

if (!EnsureDispatcherOwnerInitialized())
return PP_ERROR_FAILED;

ExtensionHostMsg_Request_Params params;
PopulateParams(request_name, &args, false, &params);

dispatcher_owner_->dispatcher()->Dispatch(
params, dispatcher_owner_->render_frame_host()->GetRenderViewHost());
// There will be no callback so return PP_OK.
return PP_OK;
}

int32_t PepperExtensionsCommonMessageFilter::OnCall(
ppapi::host::HostMessageContext* context,
const std::string& request_name,
base::ListValue& args) {
if (HandleRequest(context, request_name, &args, true))
return PP_OK_COMPLETIONPENDING;
else
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

if (!EnsureDispatcherOwnerInitialized())
return PP_ERROR_FAILED;

ExtensionHostMsg_Request_Params params;
PopulateParams(request_name, &args, true, &params);

dispatcher_owner_->dispatcher()->DispatchWithCallback(
params,
dispatcher_owner_->render_frame_host(),
base::Bind(&PepperExtensionsCommonMessageFilter::OnCallCompleted,
this,
context->MakeReplyMessageContext()));
return PP_OK_COMPLETIONPENDING;
}

void PepperExtensionsCommonMessageFilter::EnsureDispatcherOwnerInitialized() {
bool PepperExtensionsCommonMessageFilter::EnsureDispatcherOwnerInitialized() {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
if (dispatcher_owner_initialized_)
return;
return (!!dispatcher_owner_);
dispatcher_owner_initialized_ = true;

DCHECK(!dispatcher_owner_);
Expand All @@ -156,16 +175,17 @@ void PepperExtensionsCommonMessageFilter::EnsureDispatcherOwnerInitialized() {
content::WebContents::FromRenderFrameHost(frame_host);

if (!document_url_.SchemeIs(extensions::kExtensionScheme))
return;
return false;

ProfileManager* profile_manager = g_browser_process->profile_manager();
if (!profile_manager)
return;
return false;
Profile* profile = profile_manager->GetProfile(profile_directory_);

// It will be automatically destroyed when |view_host| goes away.
dispatcher_owner_ =
new DispatcherOwner(this, profile, frame_host, web_contents);
return true;
}

void PepperExtensionsCommonMessageFilter::DetachDispatcherOwner() {
Expand All @@ -192,59 +212,23 @@ void PepperExtensionsCommonMessageFilter::PopulateParams(
params->user_gesture = false;
}

void PepperExtensionsCommonMessageFilter::OnExtensionFunctionCompleted(
scoped_ptr<ppapi::host::ReplyMessageContext> reply_context,
void PepperExtensionsCommonMessageFilter::OnCallCompleted(
ppapi::host::ReplyMessageContext reply_context,
ExtensionFunction::ResponseType type,
const base::ListValue& results,
const std::string& /* error */) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

// Ignore responses resulted from calls to OnPost().
if (!reply_context) {
DCHECK_EQ(0u, results.GetSize());
return;
}

if (type == ExtensionFunction::BAD_MESSAGE) {
// The input arguments were not validated at the plugin side, so don't kill
// the plugin process when we see a message with invalid arguments.
// TODO(yzshen): It is nicer to also do the validation at the plugin side.
type = ExtensionFunction::FAILED;
}

reply_context->params.set_result(
reply_context.params.set_result(
type == ExtensionFunction::SUCCEEDED ? PP_OK : PP_ERROR_FAILED);
SendReply(*reply_context, PpapiPluginMsg_ExtensionsCommon_CallReply(results));
}

bool PepperExtensionsCommonMessageFilter::HandleRequest(
ppapi::host::HostMessageContext* context,
const std::string& request_name,
base::ListValue* args,
bool has_callback) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

EnsureDispatcherOwnerInitialized();
if (!dispatcher_owner_)
return false;

ExtensionHostMsg_Request_Params params;
PopulateParams(request_name, args, has_callback, &params);

scoped_ptr<ppapi::host::ReplyMessageContext> reply_context;
if (has_callback) {
reply_context.reset(new ppapi::host::ReplyMessageContext(
context->MakeReplyMessageContext()));
}

dispatcher_owner_->dispatcher()->DispatchWithCallback(
params,
dispatcher_owner_->render_frame_host(),
base::Bind(
&PepperExtensionsCommonMessageFilter::OnExtensionFunctionCompleted,
this,
base::Passed(&reply_context)));
return true;
SendReply(reply_context, PpapiPluginMsg_ExtensionsCommon_CallReply(results));
}

} // namespace chrome
26 changes: 7 additions & 19 deletions chrome/browser/renderer_host/pepper/pepper_extensions_common_message_filter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/strings/string16.h"
#include "extensions/browser/extension_function.h"
#include "ppapi/c/pp_instance.h"
#include "ppapi/host/host_message_context.h"
#include "ppapi/host/resource_message_filter.h"
#include "url/gurl.h"

Expand All @@ -27,12 +28,6 @@ namespace content {
class BrowserPpapiHost;
}

namespace ppapi {
namespace host {
struct HostMessageContext;
}
}

namespace chrome {

class PepperExtensionsCommonMessageFilter
Expand Down Expand Up @@ -72,9 +67,8 @@ class PepperExtensionsCommonMessageFilter
const std::string& request_name,
base::ListValue& args);

// It is possible that |dispatcher_owner_| is still NULL after this method is
// called.
void EnsureDispatcherOwnerInitialized();
// Returns true if |dispatcher_owner_| is non-null.
bool EnsureDispatcherOwnerInitialized();
// Resets |dispatcher_owner_| to NULL.
void DetachDispatcherOwner();

Expand All @@ -83,16 +77,10 @@ class PepperExtensionsCommonMessageFilter
bool has_callback,
ExtensionHostMsg_Request_Params* params);

void OnExtensionFunctionCompleted(
scoped_ptr<ppapi::host::ReplyMessageContext> reply_context,
ExtensionFunction::ResponseType type,
const base::ListValue& results,
const std::string& error);

bool HandleRequest(ppapi::host::HostMessageContext* context,
const std::string& request_name,
base::ListValue* args,
bool has_callback);
void OnCallCompleted(ppapi::host::ReplyMessageContext reply_context,
ExtensionFunction::ResponseType type,
const base::ListValue& results,
const std::string& error);

// All the members are initialized on the IO thread when the object is
// constructed, and accessed only on the UI thread afterwards.
Expand Down
10 changes: 4 additions & 6 deletions chrome/renderer/pepper/pepper_extensions_common_host.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,12 @@ int32_t PepperExtensionsCommonHost::OnResourceMessageReceived(
}

void PepperExtensionsCommonHost::OnResponseReceived(
scoped_ptr<ppapi::host::ReplyMessageContext> context,
ppapi::host::ReplyMessageContext reply_context,
bool success,
const base::ListValue& response,
const std::string& /* error */) {
context->params.set_result(success ? PP_OK : PP_ERROR_FAILED);
SendReply(*context, PpapiPluginMsg_ExtensionsCommon_CallReply(response));
reply_context.params.set_result(success ? PP_OK : PP_ERROR_FAILED);
SendReply(reply_context, PpapiPluginMsg_ExtensionsCommon_CallReply(response));
}

int32_t PepperExtensionsCommonHost::OnPost(
Expand All @@ -107,12 +107,10 @@ int32_t PepperExtensionsCommonHost::OnCall(
const std::string& request_name,
const base::ListValue& args) {
std::string error;
scoped_ptr<ppapi::host::ReplyMessageContext> message_context(
new ppapi::host::ReplyMessageContext(context->MakeReplyMessageContext()));
bool success = pepper_request_proxy_->StartRequest(
base::Bind(&PepperExtensionsCommonHost::OnResponseReceived,
weak_factory_.GetWeakPtr(),
base::Passed(&message_context)),
context->MakeReplyMessageContext()),
request_name,
args,
&error);
Expand Down
9 changes: 4 additions & 5 deletions chrome/renderer/pepper/pepper_extensions_common_host.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,10 @@ class PepperExtensionsCommonHost : public ppapi::host::ResourceHost {
const std::string& request_name,
const base::ListValue& args);

void OnResponseReceived(
scoped_ptr<ppapi::host::ReplyMessageContext> response_context,
bool success,
const base::ListValue& response,
const std::string& error);
void OnResponseReceived(ppapi::host::ReplyMessageContext reply_context,
bool success,
const base::ListValue& response,
const std::string& error);

// Non-owning pointer.
content::RendererPpapiHost* renderer_ppapi_host_;
Expand Down
29 changes: 14 additions & 15 deletions content/renderer/pepper/pepper_audio_input_host.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ int32_t PepperAudioInputHost::OnOpen(ppapi::host::HostMessageContext* context,
const std::string& device_id,
PP_AudioSampleRate sample_rate,
uint32_t sample_frame_count) {
if (open_context_)
if (open_context_.is_valid())
return PP_ERROR_INPROGRESS;
if (audio_input_)
return PP_ERROR_FAILED;
Expand All @@ -109,8 +109,7 @@ int32_t PepperAudioInputHost::OnOpen(ppapi::host::HostMessageContext* context,
static_cast<int>(sample_frame_count),
this);
if (audio_input_) {
open_context_.reset(new ppapi::host::ReplyMessageContext(
context->MakeReplyMessageContext()));
open_context_ = context->MakeReplyMessageContext();
return PP_OK_COMPLETIONPENDING;
} else {
return PP_ERROR_FAILED;
Expand Down Expand Up @@ -144,7 +143,7 @@ void PepperAudioInputHost::OnOpenComplete(
base::SyncSocket scoped_socket(socket_handle);
base::SharedMemory scoped_shared_memory(shared_memory_handle, false);

if (!open_context_) {
if (!open_context_.is_valid()) {
NOTREACHED();
return;
}
Expand All @@ -170,12 +169,9 @@ void PepperAudioInputHost::OnOpenComplete(
// inconvenient to clean up. Our IPC code will automatically handle this for
// us, as long as the remote side always closes the handles it receives, even
// in the failure case.
open_context_->params.set_result(result);
open_context_->params.AppendHandle(serialized_socket_handle);
open_context_->params.AppendHandle(serialized_shared_memory_handle);

host()->SendReply(*open_context_, PpapiPluginMsg_AudioInput_OpenReply());
open_context_.reset();
open_context_.params.AppendHandle(serialized_socket_handle);
open_context_.params.AppendHandle(serialized_shared_memory_handle);
SendOpenReply(result);
}

int32_t PepperAudioInputHost::GetRemoteHandles(
Expand Down Expand Up @@ -203,11 +199,14 @@ void PepperAudioInputHost::Close() {
audio_input_->ShutDown();
audio_input_ = NULL;

if (open_context_) {
open_context_->params.set_result(PP_ERROR_ABORTED);
host()->SendReply(*open_context_, PpapiPluginMsg_AudioInput_OpenReply());
open_context_.reset();
}
if (open_context_.is_valid())
SendOpenReply(PP_ERROR_ABORTED);
}

void PepperAudioInputHost::SendOpenReply(int32_t result) {
open_context_.params.set_result(result);
host()->SendReply(open_context_, PpapiPluginMsg_AudioInput_OpenReply());
open_context_ = ppapi::host::ReplyMessageContext();
}

} // namespace content
4 changes: 3 additions & 1 deletion content/renderer/pepper/pepper_audio_input_host.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@ class PepperAudioInputHost : public ppapi::host::ResourceHost {

void Close();

void SendOpenReply(int32_t result);

// Non-owning pointer.
RendererPpapiHostImpl* renderer_ppapi_host_;

scoped_ptr<ppapi::host::ReplyMessageContext> open_context_;
ppapi::host::ReplyMessageContext open_context_;

// Audio input object that we delegate audio IPC through.
// We don't own this pointer but are responsible for calling Shutdown on it.
Expand Down
13 changes: 6 additions & 7 deletions content/renderer/pepper/pepper_device_enumeration_host_helper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ int32_t PepperDeviceEnumerationHostHelper::InternalHandleResourceMessage(

int32_t PepperDeviceEnumerationHostHelper::OnEnumerateDevices(
HostMessageContext* context) {
if (enumerate_devices_context_)
if (enumerate_devices_context_.is_valid())
return PP_ERROR_INPROGRESS;

enumerate_.reset(new ScopedRequest(
Expand All @@ -141,8 +141,7 @@ int32_t PepperDeviceEnumerationHostHelper::OnEnumerateDevices(
if (!enumerate_->requested())
return PP_ERROR_FAILED;

enumerate_devices_context_.reset(
new ppapi::host::ReplyMessageContext(context->MakeReplyMessageContext()));
enumerate_devices_context_ = context->MakeReplyMessageContext();
return PP_OK_COMPLETIONPENDING;
}

Expand All @@ -167,15 +166,15 @@ int32_t PepperDeviceEnumerationHostHelper::OnStopMonitoringDeviceChange(
void PepperDeviceEnumerationHostHelper::OnEnumerateDevicesComplete(
int /* request_id */,
const std::vector<ppapi::DeviceRefData>& devices) {
DCHECK(enumerate_devices_context_.get());
DCHECK(enumerate_devices_context_.is_valid());

enumerate_.reset(NULL);

enumerate_devices_context_->params.set_result(PP_OK);
enumerate_devices_context_.params.set_result(PP_OK);
resource_host_->host()->SendReply(
*enumerate_devices_context_,
enumerate_devices_context_,
PpapiPluginMsg_DeviceEnumeration_EnumerateDevicesReply(devices));
enumerate_devices_context_.reset();
enumerate_devices_context_ = ppapi::host::ReplyMessageContext();
}

void PepperDeviceEnumerationHostHelper::OnNotifyDeviceChange(
Expand Down
8 changes: 4 additions & 4 deletions content/renderer/pepper/pepper_device_enumeration_host_helper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@
#include "base/memory/scoped_ptr.h"
#include "content/common/content_export.h"
#include "ppapi/c/dev/ppb_device_ref_dev.h"
#include "ppapi/host/host_message_context.h"
#include "url/gurl.h"

namespace ppapi {
struct DeviceRefData;

namespace host {
struct HostMessageContext;
struct ReplyMessageContext;
class ResourceHost;
}
}

} // namespace ppapi

namespace IPC {
class Message;
Expand Down Expand Up @@ -101,7 +101,7 @@ class CONTENT_EXPORT PepperDeviceEnumerationHostHelper {
scoped_ptr<ScopedRequest> enumerate_;
scoped_ptr<ScopedRequest> monitor_;

scoped_ptr<ppapi::host::ReplyMessageContext> enumerate_devices_context_;
ppapi::host::ReplyMessageContext enumerate_devices_context_;

DISALLOW_COPY_AND_ASSIGN(PepperDeviceEnumerationHostHelper);
};
Expand Down
4 changes: 2 additions & 2 deletions ppapi/host/host_message_context.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
namespace ppapi {
namespace host {

ReplyMessageContext::ReplyMessageContext() {
}
ReplyMessageContext::ReplyMessageContext()
: sync_reply_msg(NULL), routing_id(MSG_ROUTING_NONE) {}

ReplyMessageContext::ReplyMessageContext(
const ppapi::proxy::ResourceMessageReplyParams& cp,
Expand Down
Loading

0 comments on commit a850702

Please sign in to comment.