PPAPI: Refactor MessageChannel to prep for sync postMessage

1) Remove unused CopyPPVar function from MessageChannel
2) Remove mostly-duplicate NPVariantToPPVar from MessageChannel.
3) Separate V8VarConverter::FromV8Value implementation from calling the callback (so I can have a sync path later, and this is shorter anyway).
4) Simplify NaCl in-process to out-of-process transition. Now, we just queue stuff for in-process (nobody but NaCl uses Messaging in-process), and if/when we switch to out-of-process (handing off to a NaCl app) we drain the queues then.
5) Some other renames/tweaks to how MessageChannel queues pending conversions.

BUG=367896
R=raymes@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272583 0039d316-1c4b-4281-b951-d872f2087c98

chrome/renderer/chrome_content_renderer_client.cc

@@ -110,6 +110,10 @@
 #include "ui/base/webui/jstemplate_builder.h"
 #include "widevine_cdm_version.h"  // In SHARED_INTERMEDIATE_DIR.
 
+#if !defined(DISABLE_NACL)
+#include "components/nacl/renderer/nacl_helper.h"
+#endif
+
 #if defined(ENABLE_WEBRTC)
 #include "chrome/renderer/media/webrtc_logging_message_filter.h"
 #endif
@@ -397,6 +401,10 @@ void ChromeContentRendererClient::RenderFrameCreated(
   new PepperHelper(render_frame);
 #endif
 
+#if !defined(DISABLE_NACL)
+  new nacl::NaClHelper(render_frame);
+#endif
+
   // TODO(jam): when the frame tree moves into content and parent() works at
   // RenderFrame construction, simplify this by just checking parent().
   if (render_frame->GetRenderView()->GetMainRenderFrame() != render_frame) {

components/nacl.gyp

@@ -161,6 +161,8 @@
             'nacl/renderer/manifest_downloader.h',
             'nacl/renderer/manifest_service_channel.cc',
             'nacl/renderer/manifest_service_channel.h',
+            'nacl/renderer/nacl_helper.cc',
+            'nacl/renderer/nacl_helper.h',
             'nacl/renderer/json_manifest.cc',
             'nacl/renderer/json_manifest.h',
             'nacl/renderer/nexe_load_manager.cc',

components/nacl/renderer/nacl_helper.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/nacl/renderer/nacl_helper.h"
+
+#include "content/public/renderer/renderer_ppapi_host.h"
+
+namespace nacl {
+
+NaClHelper::NaClHelper(content::RenderFrame* render_frame)
+    : RenderFrameObserver(render_frame) {}
+
+NaClHelper::~NaClHelper() {}
+
+void NaClHelper::DidCreatePepperPlugin(content::RendererPpapiHost* host) {
+  // The Native Client plugin is a host for external plugins.
+  if (host->GetPluginName() == "Native Client")
+    host->SetToExternalPluginHost();
+}
+
+}  // namespace nacl

components/nacl/renderer/nacl_helper.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_NACL_RENDERER_NACL_HELPER_H_
+#define COMPONENTS_NACL_RENDERER_NACL_HELPER_H_
+
+#include "base/compiler_specific.h"
+#include "content/public/renderer/render_frame_observer.h"
+
+namespace nacl {
+
+// This class listens for Pepper creation events from the RenderFrame. For the
+// NaCl trusted plugin, it configures it as an external plugin host.
+// TODO(dmichael): When the trusted plugin goes away, we need to figure out the
+//                 right event to watch for.
+class NaClHelper : public content::RenderFrameObserver {
+ public:
+  explicit NaClHelper(content::RenderFrame* render_frame);
+  virtual ~NaClHelper();
+
+  // RenderFrameObserver.
+  virtual void DidCreatePepperPlugin(content::RendererPpapiHost* host) OVERRIDE;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NaClHelper);
+};
+
+}  // namespace nacl
+
+#endif  // COMPONENTS_NACL_RENDERER_NACL_HELPER_H_

content/public/renderer/renderer_ppapi_host.h

@@ -123,6 +123,23 @@ class RendererPpapiHost {
   // Returns true if the plugin is running in process.
   virtual bool IsRunningInProcess() const = 0;
 
+  virtual std::string GetPluginName() const = 0;
+
+  // Used by the embedder to inform this RendererPpapiHost that the associated
+  // plugin module is a host for "external plugins."
+  //
+  // An embedder may, at the time a plugin module is created, configure it to
+  // be a host for external plugins. Instances of such plugins go through two
+  // two stages of initialization; the first stage initializes a host plugin
+  // instance, which then loads and initializes a child plugin which takes
+  // over control. These are treated as one Pepper Instance, because despite the
+  // two-stage initialization process, the host and child appear to blink as
+  // one plugin instance.
+  //
+  // The host plugin appears as an in-process plugin, while we interact with the
+  // child plugin via the Pepper proxy.
+  virtual void SetToExternalPluginHost() = 0;
+
   // There are times when the renderer needs to create a ResourceHost in the
   // browser. This function does so asynchronously. |nested_msgs| is a list of
   // resource host creation messages and |instance| is the PP_Instance which

content/renderer/pepper/message_channel.h

@@ -53,16 +53,13 @@ class MessageChannel {
   explicit MessageChannel(PepperPluginInstanceImpl* instance);
   ~MessageChannel();
 
-  // Converts an NPVariant to a PP_Var. This occurs asynchronously and
-  // NPVariantToPPVarComplete will be called upon completion.
-  void NPVariantToPPVar(const NPVariant* variant);
-
   // Post a message to the onmessage handler for this channel's instance
   // asynchronously.
   void PostMessageToJavaScript(PP_Var message_data);
-  // Post a message to the PPP_Instance HandleMessage function for this
-  // channel's instance.
-  void PostMessageToNative(PP_Var message_data);
+
+  // Post a message to the plugin's HandleMessage function for this channel's
+  // instance.
+  void PostMessageToNative(const NPVariant* message_data);
 
   // Return the NPObject* to which we should forward any calls which aren't
   // related to postMessage.  Note that this can be NULL;  it only gets set if
@@ -75,11 +72,10 @@ class MessageChannel {
 
   PepperPluginInstanceImpl* instance() { return instance_; }
 
-  // Messages sent to JavaScript are queued by default. After the DOM is
-  // set up for the plugin, users of MessageChannel should call
-  // StopQueueingJavaScriptMessages to start dispatching messages to JavaScript.
-  void QueueJavaScriptMessages();
-  void StopQueueingJavaScriptMessages();
+  // Messages are queued initially. After the PepperPluginInstanceImpl is ready
+  // to send and handle messages, users of MessageChannel should call
+  // Start().
+  void Start();
 
   bool GetReadOnlyProperty(NPIdentifier key, NPVariant* value) const;
   void SetReadOnlyProperty(PP_Var key, PP_Var value);
@@ -88,13 +84,12 @@ class MessageChannel {
   // Struct for storing the result of a NPVariant being converted to a PP_Var.
   struct VarConversionResult;
 
-  // This is called when an NPVariant is finished being converted.
-  // |result_iteartor| is an iterator into |converted_var_queue_| where the
-  // result should be stored.
-  void NPVariantToPPVarComplete(
-      const std::list<VarConversionResult>::iterator& result_iterator,
-      const ppapi::ScopedPPVar& result,
-      bool success);
+  void EnqueuePluginMessage(const NPVariant* variant);
+
+  void FromV8ValueComplete(VarConversionResult* result_holder,
+                           const ppapi::ScopedPPVar& result_var,
+                           bool success);
+  void DrainCompletedPluginMessages();
 
   PepperPluginInstanceImpl* instance_;
 
@@ -118,22 +113,22 @@ class MessageChannel {
 
   void DrainEarlyMessageQueue();
 
-  // TODO(teravest): Remove all the tricky DRAIN_CANCELLED logic once
-  // PluginInstance::ResetAsProxied() is gone.
   std::deque<blink::WebSerializedScriptValue> early_message_queue_;
   enum EarlyMessageQueueState {
     QUEUE_MESSAGES,  // Queue JS messages.
     SEND_DIRECTLY,   // Post JS messages directly.
-    DRAIN_PENDING,   // Drain queue, then transition to DIRECT.
-    DRAIN_CANCELLED  // Preempt drain, go back to QUEUE.
   };
   EarlyMessageQueueState early_message_queue_state_;
 
-  // This queue stores vars that have been converted from NPVariants. Because
-  // conversion can happen asynchronously, the queue stores the var until all
-  // previous vars have been converted before calling PostMessage to ensure that
-  // the order in which messages are processed is preserved.
-  std::list<VarConversionResult> converted_var_queue_;
+  // This queue stores vars that are being sent to the plugin. Because
+  // conversion can happen asynchronously for object types, the queue stores
+  // the var until all previous vars have been converted and sent. This
+  // preserves the order in which JS->plugin messages are processed.
+  //
+  // Note we rely on raw VarConversionResult* pointers remaining valid after
+  // calls to push_back or pop_front; hence why we're using list. (deque would
+  // probably also work, but is less clearly specified).
+  std::list<VarConversionResult> plugin_message_queue_;
 
   std::map<NPIdentifier, ppapi::ScopedPPVar> internal_properties_;
 

content/renderer/pepper/mock_renderer_ppapi_host.cc

@@ -79,6 +79,14 @@ IPC::PlatformFileForTransit MockRendererPpapiHost::ShareHandleWithRemote(
 
 bool MockRendererPpapiHost::IsRunningInProcess() const { return false; }
 
+std::string MockRendererPpapiHost::GetPluginName() const {
+  return std::string();
+}
+
+void MockRendererPpapiHost::SetToExternalPluginHost() {
+  NOTIMPLEMENTED();
+}
+
 void MockRendererPpapiHost::CreateBrowserResourceHosts(
     PP_Instance instance,
     const std::vector<IPC::Message>& nested_msgs,

content/renderer/pepper/mock_renderer_ppapi_host.h

@@ -52,6 +52,8 @@ class MockRendererPpapiHost : public RendererPpapiHost {
       base::PlatformFile handle,
       bool should_close_source) OVERRIDE;
   virtual bool IsRunningInProcess() const OVERRIDE;
+  virtual std::string GetPluginName() const OVERRIDE;
+  virtual void SetToExternalPluginHost() OVERRIDE;
   virtual void CreateBrowserResourceHosts(
       PP_Instance instance,
       const std::vector<IPC::Message>& nested_msgs,

content/renderer/pepper/pepper_plugin_instance_impl.cc

@@ -80,6 +80,7 @@
 #include "ppapi/shared_impl/ppp_instance_combined.h"
 #include "ppapi/shared_impl/resource.h"
 #include "ppapi/shared_impl/scoped_pp_resource.h"
+#include "ppapi/shared_impl/scoped_pp_var.h"
 #include "ppapi/shared_impl/time_conversion.h"
 #include "ppapi/shared_impl/url_request_info_data.h"
 #include "ppapi/shared_impl/var.h"
@@ -146,6 +147,7 @@ using ppapi::PPB_View_Shared;
 using ppapi::PPP_Instance_Combined;
 using ppapi::Resource;
 using ppapi::ScopedPPResource;
+using ppapi::ScopedPPVar;
 using ppapi::StringVar;
 using ppapi::TrackedCallback;
 using ppapi::thunk::EnterResourceNoLock;
@@ -822,8 +824,14 @@ bool PepperPluginInstanceImpl::Initialize(
   scoped_ptr<const char * []> argv_array(StringVectorToArgArray(argv_));
   bool success = PP_ToBool(instance_interface_->DidCreate(
       pp_instance(), argn_.size(), argn_array.get(), argv_array.get()));
-  if (success)
-    message_channel_->StopQueueingJavaScriptMessages();
+  // If this is a plugin that hosts external plugins, we should delay messages
+  // so that the child plugin that's created later will receive all the
+  // messages. (E.g., NaCl trusted plugin starting a child NaCl app.)
+  //
+  // A host for external plugins will call ResetAsProxied later, at which point
+  // we can Start() the message_channel_.
+  if (success && (!module_->renderer_ppapi_host()->IsExternalPluginHost()))
+    message_channel_->Start();
   return success;
 }
 
@@ -1112,11 +1120,11 @@ bool PepperPluginInstanceImpl::HandleInputEvent(
   return rv;
 }
 
-void PepperPluginInstanceImpl::HandleMessage(PP_Var message) {
+void PepperPluginInstanceImpl::HandleMessage(ScopedPPVar message) {
   TRACE_EVENT0("ppapi", "PepperPluginInstanceImpl::HandleMessage");
   ppapi::proxy::HostDispatcher* dispatcher =
       ppapi::proxy::HostDispatcher::GetForInstance(pp_instance());
-  if (!dispatcher || (message.type == PP_VARTYPE_OBJECT)) {
+  if (!dispatcher || (message.get().type == PP_VARTYPE_OBJECT)) {
     // The dispatcher should always be valid, and the browser should never send
     // an 'object' var over PPP_Messaging.
     NOTREACHED();
@@ -1125,7 +1133,7 @@ void PepperPluginInstanceImpl::HandleMessage(PP_Var message) {
   dispatcher->Send(new PpapiMsg_PPPMessaging_HandleMessage(
       ppapi::API_ID_PPP_MESSAGING,
       pp_instance(),
-      ppapi::proxy::SerializedVarSendInputShmem(dispatcher, message,
+      ppapi::proxy::SerializedVarSendInputShmem(dispatcher, message.get(),
                                                 pp_instance())));
 }
 
@@ -2737,9 +2745,6 @@ PP_ExternalPluginResult PepperPluginInstanceImpl::ResetAsProxied(
   original_module_ = module_;
   module_ = module;
 
-  // Don't send any messages to the plugin until DidCreate() has finished.
-  message_channel_->QueueJavaScriptMessages();
-
   // For NaCl instances, remember the NaCl plugin instance interface, so we
   // can shut it down by calling its DidDestroy in our Delete() method.
   original_instance_interface_.reset(instance_interface_.release());
@@ -2775,7 +2780,7 @@ PP_ExternalPluginResult PepperPluginInstanceImpl::ResetAsProxied(
   if (!instance_interface_->DidCreate(
           pp_instance(), argn_.size(), argn_array.get(), argv_array.get()))
     return PP_EXTERNAL_PLUGIN_ERROR_INSTANCE;
-  message_channel_->StopQueueingJavaScriptMessages();
+  message_channel_->Start();
 
   // Clear sent_initial_did_change_view_ and cancel any pending DidChangeView
   // event. This way, SendDidChangeView will send the "current" view

content/renderer/pepper/pepper_plugin_instance_impl.h

@@ -89,6 +89,7 @@ namespace ppapi {
 class Resource;
 struct InputEventData;
 struct PPP_Instance_Combined;
+class ScopedPPVar;
 }
 
 namespace v8 {
@@ -296,8 +297,8 @@ class CONTENT_EXPORT PepperPluginInstanceImpl
   // already in fullscreen mode).
   bool SetFullscreen(bool fullscreen);
 
-  // Implementation of PPP_Messaging.
-  void HandleMessage(PP_Var message);
+  // Send the message on to the plugin.
+  void HandleMessage(ppapi::ScopedPPVar message);
 
   // Returns true if the plugin is processing a user gesture.
   bool IsProcessingUserGesture();

content/renderer/pepper/renderer_ppapi_host_impl.cc

@@ -41,7 +41,9 @@ RendererPpapiHostImpl::RendererPpapiHostImpl(
     PluginModule* module,
     ppapi::proxy::HostDispatcher* dispatcher,
     const ppapi::PpapiPermissions& permissions)
-    : module_(module), dispatcher_(dispatcher) {
+    : module_(module),
+      dispatcher_(dispatcher),
+      is_external_plugin_host_(false) {
   // Hook the PpapiHost up to the dispatcher for out-of-process communication.
   ppapi_host_.reset(new ppapi::host::PpapiHost(dispatcher, permissions));
   ppapi_host_->AddHostFactoryFilter(scoped_ptr<ppapi::host::HostFactory>(
@@ -54,7 +56,7 @@ RendererPpapiHostImpl::RendererPpapiHostImpl(
 RendererPpapiHostImpl::RendererPpapiHostImpl(
     PluginModule* module,
     const ppapi::PpapiPermissions& permissions)
-    : module_(module), dispatcher_(NULL) {
+    : module_(module), dispatcher_(NULL), is_external_plugin_host_(false) {
   // Hook the host up to the in-process router.
   in_process_router_.reset(new PepperInProcessRouter(this));
   ppapi_host_.reset(new ppapi::host::PpapiHost(
@@ -125,6 +127,10 @@ PepperPluginInstanceImpl* RendererPpapiHostImpl::GetPluginInstanceImpl(
   return GetAndValidateInstance(instance);
 }
 
+bool RendererPpapiHostImpl::IsExternalPluginHost() const {
+  return is_external_plugin_host_;
+}
+
 ppapi::host::PpapiHost* RendererPpapiHostImpl::GetPpapiHost() {
   return ppapi_host_.get();
 }
@@ -227,6 +233,14 @@ bool RendererPpapiHostImpl::IsRunningInProcess() const {
   return is_running_in_process_;
 }
 
+std::string RendererPpapiHostImpl::GetPluginName() const {
+  return module_->name();
+}
+
+void RendererPpapiHostImpl::SetToExternalPluginHost() {
+  is_external_plugin_host_ = true;
+}
+
 void RendererPpapiHostImpl::CreateBrowserResourceHosts(
     PP_Instance instance,
     const std::vector<IPC::Message>& nested_msgs,

content/renderer/pepper/renderer_ppapi_host_impl.h

@@ -73,6 +73,8 @@ class RendererPpapiHostImpl : public RendererPpapiHost {
 
   PepperPluginInstanceImpl* GetPluginInstanceImpl(PP_Instance instance) const;
 
+  bool IsExternalPluginHost() const;
+
   // RendererPpapiHost implementation.
   virtual ppapi::host::PpapiHost* GetPpapiHost() OVERRIDE;
   virtual bool IsValidInstance(PP_Instance instance) const OVERRIDE;
@@ -94,6 +96,8 @@ class RendererPpapiHostImpl : public RendererPpapiHost {
       base::PlatformFile handle,
       bool should_close_source) OVERRIDE;
   virtual bool IsRunningInProcess() const OVERRIDE;
+  virtual std::string GetPluginName() const OVERRIDE;
+  virtual void SetToExternalPluginHost() OVERRIDE;
   virtual void CreateBrowserResourceHosts(
       PP_Instance instance,
       const std::vector<IPC::Message>& nested_msgs,
@@ -130,6 +134,9 @@ class RendererPpapiHostImpl : public RendererPpapiHost {
   // Whether the plugin is running in process.
   bool is_running_in_process_;
 
+  // Whether this is a host for external plugins.
+  bool is_external_plugin_host_;
+
   DISALLOW_COPY_AND_ASSIGN(RendererPpapiHostImpl);
 };
 

content/renderer/pepper/resource_converter.cc

@@ -203,7 +203,7 @@ ResourceConverterImpl::ResourceConverterImpl(PP_Instance instance,
 ResourceConverterImpl::~ResourceConverterImpl() {
   // Verify Flush() was called.
   DCHECK(browser_host_create_messages_.empty());
-  DCHECK(browser_vars.empty());
+  DCHECK(browser_vars_.empty());
 }
 
 bool ResourceConverterImpl::FromV8Value(v8::Handle<v8::Object> val,
@@ -269,9 +269,9 @@ void ResourceConverterImpl::Flush(const base::Callback<void(bool)>& callback) {
   host_->CreateBrowserResourceHosts(
       instance_,
       browser_host_create_messages_,
-      base::Bind(&FlushComplete, callback, browser_vars));
+      base::Bind(&FlushComplete, callback, browser_vars_));
   browser_host_create_messages_.clear();
-  browser_vars.clear();
+  browser_vars_.clear();
 }
 
 bool ResourceConverterImpl::ToV8Value(const PP_Var& var,
@@ -336,7 +336,7 @@ ResourceConverterImpl::CreateResourceVarWithBrowserHost(
   scoped_refptr<HostResourceVar> result =
       CreateResourceVar(pending_renderer_id, create_message);
   browser_host_create_messages_.push_back(browser_host_create_message);
-  browser_vars.push_back(result);
+  browser_vars_.push_back(result);
   return result;
 }
 

content/renderer/pepper/resource_converter.h

@@ -95,7 +95,7 @@ class ResourceConverterImpl : public ResourceConverter {
   // conveniently passed to |CreateBrowserResourceHosts|.
   std::vector<IPC::Message> browser_host_create_messages_;
   // A list of the resource vars associated with browser hosts.
-  std::vector<scoped_refptr<HostResourceVar> > browser_vars;
+  std::vector<scoped_refptr<HostResourceVar> > browser_vars_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourceConverterImpl);
 };

content/renderer/pepper/v8_var_converter.cc

@@ -399,6 +399,20 @@ void V8VarConverter::FromV8Value(
     v8::Handle<v8::Value> val,
     v8::Handle<v8::Context> context,
     const base::Callback<void(const ScopedPPVar&, bool)>& callback) {
+  ScopedPPVar result_var;
+  if (FromV8ValueInternal(val, context, &result_var)) {
+    resource_converter_->Flush(base::Bind(callback, result_var));
+  } else {
+    message_loop_proxy_->PostTask(
+        FROM_HERE,
+        base::Bind(callback, result_var, false));
+  }
+}
+
+bool V8VarConverter::FromV8ValueInternal(
+    v8::Handle<v8::Value> val,
+    v8::Handle<v8::Context> context,
+    ppapi::ScopedPPVar* result_var) {
   v8::Context::Scope context_scope(context);
   v8::HandleScope handle_scope(context->GetIsolate());
 
@@ -408,6 +422,7 @@ void V8VarConverter::FromV8Value(
   std::stack<StackEntry<v8::Handle<v8::Value> > > stack;
   stack.push(StackEntry<v8::Handle<v8::Value> >(val));
   ScopedPPVar root;
+  *result_var = PP_MakeUndefined();
   bool is_root = true;
 
   while (!stack.empty()) {
@@ -431,10 +446,7 @@ void V8VarConverter::FromV8Value(
                         &visited_handles,
                         &parent_handles,
                         resource_converter_.get())) {
-      message_loop_proxy_->PostTask(
-          FROM_HERE,
-          base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-      return;
+      return false;
     }
 
     if (is_root) {
@@ -451,21 +463,14 @@ void V8VarConverter::FromV8Value(
       ArrayVar* array_var = ArrayVar::FromPPVar(current_var);
       if (!array_var) {
         NOTREACHED();
-        message_loop_proxy_->PostTask(
-            FROM_HERE,
-            base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-        return;
+        return false;
       }
 
       for (uint32 i = 0; i < v8_array->Length(); ++i) {
         v8::TryCatch try_catch;
         v8::Handle<v8::Value> child_v8 = v8_array->Get(i);
-        if (try_catch.HasCaught()) {
-          message_loop_proxy_->PostTask(
-              FROM_HERE,
-              base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-          return;
-        }
+        if (try_catch.HasCaught())
+          return false;
 
         if (!v8_array->HasRealIndexedProperty(i))
           continue;
@@ -478,10 +483,7 @@ void V8VarConverter::FromV8Value(
                             &visited_handles,
                             &parent_handles,
                             resource_converter_.get())) {
-          message_loop_proxy_->PostTask(
-              FROM_HERE,
-              base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-          return;
+          return false;
         }
         if (did_create && child_v8->IsObject())
           stack.push(child_v8);
@@ -496,10 +498,7 @@ void V8VarConverter::FromV8Value(
       DictionaryVar* dict_var = DictionaryVar::FromPPVar(current_var);
       if (!dict_var) {
         NOTREACHED();
-        message_loop_proxy_->PostTask(
-            FROM_HERE,
-            base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-        return;
+        return false;
       }
 
       v8::Handle<v8::Array> property_names(v8_object->GetOwnPropertyNames());
@@ -511,10 +510,7 @@ void V8VarConverter::FromV8Value(
           NOTREACHED() << "Key \"" << *v8::String::Utf8Value(key)
                        << "\" "
                           "is neither a string nor a number";
-          message_loop_proxy_->PostTask(
-              FROM_HERE,
-              base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-          return;
+          return false;
         }
 
         // Skip all callbacks: crbug.com/139933
@@ -525,12 +521,8 @@ void V8VarConverter::FromV8Value(
 
         v8::TryCatch try_catch;
         v8::Handle<v8::Value> child_v8 = v8_object->Get(key);
-        if (try_catch.HasCaught()) {
-          message_loop_proxy_->PostTask(
-              FROM_HERE,
-              base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-          return;
-        }
+        if (try_catch.HasCaught())
+          return false;
 
         PP_Var child_var;
         if (!GetOrCreateVar(child_v8,
@@ -540,10 +532,7 @@ void V8VarConverter::FromV8Value(
                             &visited_handles,
                             &parent_handles,
                             resource_converter_.get())) {
-          message_loop_proxy_->PostTask(
-              FROM_HERE,
-              base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-          return;
+          return false;
         }
         if (did_create && child_v8->IsObject())
           stack.push(child_v8);
@@ -554,7 +543,8 @@ void V8VarConverter::FromV8Value(
       }
     }
   }
-  resource_converter_->Flush(base::Bind(callback, root));
+  *result_var = root;
+  return true;
 }
 
 }  // namespace content

content/renderer/pepper/v8_var_converter.h

@@ -46,8 +46,12 @@ class CONTENT_EXPORT V8VarConverter {
       v8::Handle<v8::Value> val,
       v8::Handle<v8::Context> context,
       const base::Callback<void(const ppapi::ScopedPPVar&, bool)>& callback);
-
  private:
+  // Returns true on success, false on failure.
+  bool FromV8ValueInternal(v8::Handle<v8::Value> val,
+                           v8::Handle<v8::Context> context,
+                           ppapi::ScopedPPVar* result_var);
+
   // The message loop to run the callback to |FromV8Value| from.
   scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;