diff options
5 files changed, 175 insertions, 73 deletions
diff --git a/chrome/test/ppapi/ppapi_browsertest.cc b/chrome/test/ppapi/ppapi_browsertest.cc index 2f2b19b..337fe21 100644 --- a/chrome/test/ppapi/ppapi_browsertest.cc +++ b/chrome/test/ppapi/ppapi_browsertest.cc @@ -735,8 +735,7 @@ IN_PROC_BROWSER_TEST_F(OutOfProcessPPAPITest, MAYBE_PostMessage) { LIST_TEST(PostMessage_SendingArrayBuffer) LIST_TEST(PostMessage_SendingArray) LIST_TEST(PostMessage_SendingDictionary) - // TODO(mgiuca): Re-enable after http://crbug.com/314007 is fixed. - LIST_TEST(DISABLED_PostMessage_SendingResource) + LIST_TEST(PostMessage_SendingResource) LIST_TEST(PostMessage_SendingComplexVar) LIST_TEST(PostMessage_MessageEvent) LIST_TEST(PostMessage_NoHandler) @@ -751,8 +750,7 @@ IN_PROC_BROWSER_TEST_F(PPAPINaClNewlibTest, PostMessage) { LIST_TEST(PostMessage_SendingArrayBuffer) LIST_TEST(PostMessage_SendingArray) LIST_TEST(PostMessage_SendingDictionary) - // TODO(mgiuca): Re-enable after http://crbug.com/314007 is fixed. - LIST_TEST(DISABLED_PostMessage_SendingResource) + LIST_TEST(PostMessage_SendingResource) LIST_TEST(PostMessage_SendingComplexVar) LIST_TEST(PostMessage_MessageEvent) LIST_TEST(PostMessage_NoHandler) @@ -767,8 +765,7 @@ IN_PROC_BROWSER_TEST_F(PPAPINaClGLibcTest, MAYBE_GLIBC(PostMessage)) { LIST_TEST(PostMessage_SendingArrayBuffer) LIST_TEST(PostMessage_SendingArray) LIST_TEST(PostMessage_SendingDictionary) - // TODO(mgiuca): Re-enable after http://crbug.com/314007 is fixed. - LIST_TEST(DISABLED_PostMessage_SendingResource) + LIST_TEST(PostMessage_SendingResource) LIST_TEST(PostMessage_SendingComplexVar) LIST_TEST(PostMessage_MessageEvent) LIST_TEST(PostMessage_NoHandler) @@ -783,8 +780,7 @@ IN_PROC_BROWSER_TEST_F(PPAPINaClPNaClTest, PostMessage) { LIST_TEST(PostMessage_SendingArrayBuffer) LIST_TEST(PostMessage_SendingArray) LIST_TEST(PostMessage_SendingDictionary) - // TODO(mgiuca): Re-enable after http://crbug.com/314007 is fixed. - LIST_TEST(DISABLED_PostMessage_SendingResource) + LIST_TEST(PostMessage_SendingResource) LIST_TEST(PostMessage_SendingComplexVar) LIST_TEST(PostMessage_MessageEvent) LIST_TEST(PostMessage_NoHandler) diff --git a/content/browser/renderer_host/pepper/pepper_file_system_browser_host.cc b/content/browser/renderer_host/pepper/pepper_file_system_browser_host.cc index 02692b9..d1f1b37 100644 --- a/content/browser/renderer_host/pepper/pepper_file_system_browser_host.cc +++ b/content/browser/renderer_host/pepper/pepper_file_system_browser_host.cc @@ -50,19 +50,24 @@ PepperFileSystemBrowserHost::PepperFileSystemBrowserHost(BrowserPpapiHost* host, weak_factory_(this) { } -PepperFileSystemBrowserHost::PepperFileSystemBrowserHost(BrowserPpapiHost* host, - PP_Instance instance, - PP_Resource resource, - const GURL& root_url, - PP_FileSystemType type) - : ResourceHost(host->GetPpapiHost(), instance, resource), - browser_ppapi_host_(host), - type_(type), - opened_(true), - root_url_(root_url), - fs_context_(NULL), - called_open_(true), - weak_factory_(this) { +void PepperFileSystemBrowserHost::OpenExisting(const GURL& root_url, + const base::Closure& callback) { + root_url_ = root_url; + int render_process_id = 0; + int unused; + if (!browser_ppapi_host_->GetRenderViewIDsForInstance( + pp_instance(), &render_process_id, &unused)) { + NOTREACHED(); + } + called_open_ = true; + // Get the file system context asynchronously, and then complete the Open + // operation by calling |callback|. + BrowserThread::PostTaskAndReplyWithResult( + BrowserThread::UI, + FROM_HERE, + base::Bind(&GetFileSystemContextFromRenderId, render_process_id), + base::Bind(&PepperFileSystemBrowserHost::OpenExistingWithContext, + weak_factory_.GetWeakPtr(), callback)); } PepperFileSystemBrowserHost::~PepperFileSystemBrowserHost() { @@ -133,6 +138,21 @@ int32_t PepperFileSystemBrowserHost::OnHostMsgOpen( return PP_OK_COMPLETIONPENDING; } +void PepperFileSystemBrowserHost::OpenExistingWithContext( + const base::Closure& callback, + scoped_refptr<fileapi::FileSystemContext> fs_context) { + if (fs_context.get()) { + opened_ = true; + } else { + // If there is no file system context, we log a warning and continue with an + // invalid resource (which will produce errors when used), since we have no + // way to communicate the error to the caller. + LOG(WARNING) << "Could not retrieve file system context."; + } + fs_context_ = fs_context; + callback.Run(); +} + void PepperFileSystemBrowserHost::GotFileSystemContext( ppapi::host::ReplyMessageContext reply_context, fileapi::FileSystemType file_system_type, diff --git a/content/browser/renderer_host/pepper/pepper_file_system_browser_host.h b/content/browser/renderer_host/pepper/pepper_file_system_browser_host.h index ebf55f4..5aa6430 100644 --- a/content/browser/renderer_host/pepper/pepper_file_system_browser_host.h +++ b/content/browser/renderer_host/pepper/pepper_file_system_browser_host.h @@ -6,6 +6,7 @@ #define CONTENT_BROWSER_RENDERER_HOST_PEPPER_PEPPER_FILE_SYSTEM_BROWSER_HOST_H_ #include "base/basictypes.h" +#include "base/callback.h" #include "base/memory/weak_ptr.h" #include "ppapi/c/pp_file_info.h" #include "ppapi/host/host_message_context.h" @@ -28,17 +29,14 @@ class PepperFileSystemBrowserHost : PP_Instance instance, PP_Resource resource, PP_FileSystemType type); - // Creates a new PepperFileSystemBrowserHost with an existing file system at - // the given |root_url| and of the given |type|. The file system at |root_url| - // must already be opened. Once created, the PepperFileSystemBrowserHost may - // be used without being opened. - PepperFileSystemBrowserHost(BrowserPpapiHost* host, - PP_Instance instance, - PP_Resource resource, - const GURL& root_url, - PP_FileSystemType type); virtual ~PepperFileSystemBrowserHost(); + // Opens the PepperFileSystemBrowserHost to use an existing file system at the + // given |root_url|. The file system at |root_url| must already be opened and + // have the type given by GetType(). + // Calls |callback| when complete. + void OpenExisting(const GURL& root_url, const base::Closure& callback); + // ppapi::host::ResourceHost override. virtual int32_t OnResourceMessageReceived( const IPC::Message& msg, @@ -54,6 +52,9 @@ class PepperFileSystemBrowserHost : } private: + void OpenExistingWithContext( + const base::Closure& callback, + scoped_refptr<fileapi::FileSystemContext> fs_context); void GotFileSystemContext( ppapi::host::ReplyMessageContext reply_context, fileapi::FileSystemType file_system_type, diff --git a/content/browser/renderer_host/pepper/pepper_renderer_connection.cc b/content/browser/renderer_host/pepper/pepper_renderer_connection.cc index 8c76c69..1c00388 100644 --- a/content/browser/renderer_host/pepper/pepper_renderer_connection.cc +++ b/content/browser/renderer_host/pepper/pepper_renderer_connection.cc @@ -4,6 +4,8 @@ #include "content/browser/renderer_host/pepper/pepper_renderer_connection.h" +#include "base/bind.h" +#include "base/memory/ref_counted.h" #include "content/browser/browser_child_process_host_impl.h" #include "content/browser/ppapi_plugin_process_host.h" #include "content/browser/renderer_host/pepper/browser_ppapi_host_impl.h" @@ -22,6 +24,68 @@ namespace content { +namespace { + +// Responsible for creating the pending resource hosts, holding their IDs until +// all of them have been created for a single message, and sending the reply to +// say that the hosts have been created. +class PendingHostCreator + : public base::RefCounted<PendingHostCreator> { + public: + PendingHostCreator(BrowserPpapiHostImpl* host, + BrowserMessageFilter* connection, + int routing_id, + int sequence_id, + size_t nested_msgs_size); + + // Adds the given resource host as a pending one. The host is remembered as + // host number |index|, and will ultimately be sent to the plugin to be + // attached to a real resource. + void AddPendingResourceHost( + size_t index, + scoped_ptr<ppapi::host::ResourceHost> resource_host); + + private: + friend class base::RefCounted<PendingHostCreator>; + + // When the last reference to this class is released, all of the resource + // hosts would have been added. This destructor sends the message to the + // plugin to tell it to attach real hosts to all of the pending hosts that + // have been added by this object. + ~PendingHostCreator(); + + BrowserPpapiHostImpl* host_; + BrowserMessageFilter* connection_; + int routing_id_; + int sequence_id_; + std::vector<int> pending_resource_host_ids_; +}; + +PendingHostCreator::PendingHostCreator(BrowserPpapiHostImpl* host, + BrowserMessageFilter* connection, + int routing_id, + int sequence_id, + size_t nested_msgs_size) + : host_(host), + connection_(connection), + routing_id_(routing_id), + sequence_id_(sequence_id), + pending_resource_host_ids_(nested_msgs_size, 0) {} + +void PendingHostCreator::AddPendingResourceHost( + size_t index, + scoped_ptr<ppapi::host::ResourceHost> resource_host) { + pending_resource_host_ids_[index] = + host_->GetPpapiHost()->AddPendingResourceHost(resource_host.Pass()); +} + +PendingHostCreator::~PendingHostCreator() { + connection_->Send(new PpapiHostMsg_CreateResourceHostsFromHostReply( + routing_id_, sequence_id_, pending_resource_host_ids_)); +} + +} // namespace + PepperRendererConnection::PepperRendererConnection(int render_process_id) : render_process_id_(render_process_id) { // Only give the renderer permission for stable APIs. @@ -93,57 +157,71 @@ void PepperRendererConnection::OnMsgCreateResourceHostsFromHost( PP_Instance instance, const std::vector<IPC::Message>& nested_msgs) { BrowserPpapiHostImpl* host = GetHostForChildProcess(child_process_id); - - std::vector<int> pending_resource_host_ids(nested_msgs.size(), 0); if (!host) { DLOG(ERROR) << "Invalid plugin process ID."; - } else { - for (size_t i = 0; i < nested_msgs.size(); ++i) { - const IPC::Message& nested_msg = nested_msgs[i]; - scoped_ptr<ppapi::host::ResourceHost> resource_host; - if (host->IsValidInstance(instance)) { - if (nested_msg.type() == PpapiHostMsg_FileRef_CreateExternal::ID) { - // FileRef_CreateExternal is only permitted from the renderer. Because - // of this, we handle this message here and not in - // content_browser_pepper_host_factory.cc. - base::FilePath external_path; - if (ppapi::UnpackMessage<PpapiHostMsg_FileRef_CreateExternal>( - nested_msg, &external_path)) { - resource_host.reset(new PepperFileRefHost( - host, instance, params.pp_resource(), external_path)); - } - } else if (nested_msg.type() == - PpapiHostMsg_FileSystem_CreateFromRenderer::ID) { - // Similarly, FileSystem_CreateFromRenderer is only permitted from the - // renderer. - std::string root_url; - PP_FileSystemType file_system_type; - if (ppapi::UnpackMessage<PpapiHostMsg_FileSystem_CreateFromRenderer>( - nested_msg, &root_url, &file_system_type)) { - resource_host.reset( - new PepperFileSystemBrowserHost(host, - instance, - params.pp_resource(), - GURL(root_url), - file_system_type)); - } - } - } + return; + } - if (!resource_host.get()) { - resource_host = host->GetPpapiHost()->CreateResourceHost( - params, instance, nested_msg); + scoped_refptr<PendingHostCreator> creator = new PendingHostCreator( + host, this, routing_id, params.sequence(), nested_msgs.size()); + for (size_t i = 0; i < nested_msgs.size(); ++i) { + const IPC::Message& nested_msg = nested_msgs[i]; + scoped_ptr<ppapi::host::ResourceHost> resource_host; + if (host->IsValidInstance(instance)) { + if (nested_msg.type() == PpapiHostMsg_FileRef_CreateExternal::ID) { + // FileRef_CreateExternal is only permitted from the renderer. Because + // of this, we handle this message here and not in + // content_browser_pepper_host_factory.cc. + base::FilePath external_path; + if (ppapi::UnpackMessage<PpapiHostMsg_FileRef_CreateExternal>( + nested_msg, &external_path)) { + resource_host.reset(new PepperFileRefHost( + host, instance, params.pp_resource(), external_path)); + } + } else if (nested_msg.type() == + PpapiHostMsg_FileSystem_CreateFromRenderer::ID) { + // Similarly, FileSystem_CreateFromRenderer is only permitted from the + // renderer. + std::string root_url; + PP_FileSystemType file_system_type; + if (ppapi::UnpackMessage<PpapiHostMsg_FileSystem_CreateFromRenderer>( + nested_msg, &root_url, &file_system_type)) { + PepperFileSystemBrowserHost* browser_host = + new PepperFileSystemBrowserHost(host, + instance, + params.pp_resource(), + file_system_type); + resource_host.reset(browser_host); + // Open the file system resource host. This is an asynchronous + // operation, and we must only add the pending resource host and + // send the message once it completes. + browser_host->OpenExisting( + GURL(root_url), + base::Bind( + &PendingHostCreator::AddPendingResourceHost, + creator, + i, + base::Passed(&resource_host))); + // Do not fall through; the fall-through case adds the pending + // resource host to the list. We must do this asynchronously. + continue; + } } + } - if (resource_host.get()) { - pending_resource_host_ids[i] = - host->GetPpapiHost()->AddPendingResourceHost(resource_host.Pass()); - } + if (!resource_host.get()) { + resource_host = host->GetPpapiHost()->CreateResourceHost( + params, instance, nested_msg); } + + if (resource_host.get()) + creator->AddPendingResourceHost(i, resource_host.Pass()); } - Send(new PpapiHostMsg_CreateResourceHostsFromHostReply( - routing_id, params.sequence(), pending_resource_host_ids)); + // Note: All of the pending host IDs that were added as part of this + // operation will automatically be sent to the plugin when |creator| is + // released. This may happen immediately, or (if there are asynchronous + // requests to create resource hosts), once all of them complete. } void PepperRendererConnection::OnMsgDidCreateInProcessInstance( diff --git a/ppapi/tests/test_post_message.cc b/ppapi/tests/test_post_message.cc index eb22c7f..df126c7 100644 --- a/ppapi/tests/test_post_message.cc +++ b/ppapi/tests/test_post_message.cc @@ -610,6 +610,13 @@ std::string TestPostMessage::TestSendingResource() { pp::FileRef file_ref(file_system, file_path.c_str()); ASSERT_NE(0, file_ref.pp_resource()); + // Ensure that the file can be queried. + TestCompletionCallbackWithOutput<PP_FileInfo> cc(instance_->pp_instance(), + callback_type()); + cc.WaitForResult(file_ref.Query(cc.GetCallback())); + CHECK_CALLBACK_BEHAVIOR(cc); + ASSERT_EQ(PP_OK, cc.result()); + // Read the file and test that its contents match. pp::FileIO file_io(instance_); ASSERT_NE(0, file_io.pp_resource()); |