Use the correct v8 context for conversions when calling into the plugin from JS

Previously we were using the plugin context when converting objects during calls
into the plugin from JS. As decided previously in https://codereview.chromium.org/555583003/
we should always use the caller's context in these circumstances.

This CL also separates the checks in PepperPluginInstanceImpl::GetContext to
help diagnose crbug.com/416311.

BUG=416311

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

Cr-Commit-Position: refs/heads/master@{#297136}

content/renderer/pepper/message_channel.cc

@@ -90,7 +90,7 @@ MessageChannel* MessageChannel::Create(PepperPluginInstanceImpl* instance,
                                        v8::Persistent<v8::Object>* result) {
   MessageChannel* message_channel = new MessageChannel(instance);
   v8::HandleScope handle_scope(instance->GetIsolate());
-  v8::Context::Scope context_scope(instance->GetContext());
+  v8::Context::Scope context_scope(instance->GetMainWorldContext());
   gin::Handle<MessageChannel> handle =
       gin::CreateHandle(instance->GetIsolate(), message_channel);
   result->Reset(instance->GetIsolate(), handle.ToV8()->ToObject());
@@ -112,7 +112,7 @@ void MessageChannel::PostMessageToJavaScript(PP_Var message_data) {
 
   // Because V8 is probably not on the stack for Native->JS calls, we need to
   // enter the appropriate context for the plugin.
-  v8::Local<v8::Context> context = instance_->GetContext();
+  v8::Local<v8::Context> context = instance_->GetMainWorldContext();
   if (context.IsEmpty())
     return;
 

content/renderer/pepper/pepper_plugin_instance_impl.cc

@@ -672,13 +672,18 @@ void PepperPluginInstanceImpl::MessageChannelDestroyed() {
   message_channel_object_.Reset();
 }
 
-v8::Local<v8::Context> PepperPluginInstanceImpl::GetContext() {
-  if (!container_ ||
-      container_->element().isNull() ||
-      container_->element().document().isNull() ||
-      !container_->element().document().frame()) {
+v8::Local<v8::Context> PepperPluginInstanceImpl::GetMainWorldContext() {
+  if (!container_)
+    return v8::Handle<v8::Context>();
+
+  if (container_->element().isNull())
+    return v8::Handle<v8::Context>();
+
+  if (container_->element().document().isNull())
+    return v8::Handle<v8::Context>();
+
+  if (!container_->element().document().frame())
     return v8::Handle<v8::Context>();
-  }
 
   v8::Local<v8::Context> context =
       container_->element().document().frame()->mainWorldScriptContext();

content/renderer/pepper/pepper_plugin_instance_impl.h

@@ -149,8 +149,12 @@ class CONTENT_EXPORT PepperPluginInstanceImpl
   // the plugin being destroyed.
   void MessageChannelDestroyed();
 
-  // Return the v8 context that the plugin is in.
-  v8::Local<v8::Context> GetContext();
+  // Return the v8 context for the frame that the plugin is contained in. Care
+  // should be taken to use the correct context for plugin<->JS interactions.
+  // In cases where JS calls into the plugin, the caller's context should
+  // typically be used. When calling from the plugin into JS, this context
+  // should typically used.
+  v8::Local<v8::Context> GetMainWorldContext();
 
   // Does some pre-destructor cleanup on the instance. This is necessary
   // because some cleanup depends on the plugin instance still existing (like

content/renderer/pepper/pepper_try_catch.cc

@@ -26,10 +26,6 @@ PepperTryCatch::PepperTryCatch(PepperPluginInstanceImpl* instance,
 
 PepperTryCatch::~PepperTryCatch() {}
 
-v8::Handle<v8::Context> PepperTryCatch::GetContext() {
-  return instance_->GetContext();
-}
-
 v8::Handle<v8::Value> PepperTryCatch::ToV8(PP_Var var) {
   if (HasException()) {
     SetException(kConversionException);
@@ -85,12 +81,17 @@ bool PepperTryCatchV8::HasException() {
   return GetContext().IsEmpty() || exception_.type != PP_VARTYPE_UNDEFINED;
 }
 
+v8::Handle<v8::Context> PepperTryCatchV8::GetContext() {
+  // When calling from JS into the plugin always use the current context.
+  return instance_->GetIsolate()->GetCurrentContext();
+}
+
 bool PepperTryCatchV8::ThrowException() {
   if (!HasException())
     return false;
 
-  // If the plugin context is gone, then we have an exception but we don't try
-  // to throw it into v8.
+  // If there is no context then we have an exception but we don't try to throw
+  // it into v8.
   if (GetContext().IsEmpty())
     return true;
 
@@ -156,6 +157,11 @@ bool PepperTryCatchVar::HasException() {
   return exception_is_set_;
 }
 
+v8::Handle<v8::Context> PepperTryCatchVar::GetContext() {
+  // When calling into JS from the plugin, always use the plugin context.
+  return instance_->GetMainWorldContext();
+}
+
 void PepperTryCatchVar::SetException(const char* message) {
   if (exception_is_set_)
     return;

content/renderer/pepper/pepper_try_catch.h

@@ -27,8 +27,8 @@ class CONTENT_EXPORT PepperTryCatch {
 
   virtual void SetException(const char* message) = 0;
   virtual bool HasException() = 0;
-  // Gets the plugin context. Virtual so it can be overriden for testing.
-  virtual v8::Handle<v8::Context> GetContext();
+  // Gets the context to execute scripts in.
+  virtual v8::Handle<v8::Context> GetContext() = 0;
 
   // Convenience functions for doing conversions to/from V8 values and sets an
   // exception if there is an error in the conversion.
@@ -66,6 +66,7 @@ class PepperTryCatchV8 : public PepperTryCatch {
   // PepperTryCatch
   virtual void SetException(const char* message) OVERRIDE;
   virtual bool HasException() OVERRIDE;
+  virtual v8::Handle<v8::Context> GetContext() OVERRIDE;
 
  private:
   PP_Var exception_;
@@ -86,6 +87,7 @@ class PepperTryCatchVar : public PepperTryCatch {
   // PepperTryCatch
   virtual void SetException(const char* message) OVERRIDE;
   virtual bool HasException() OVERRIDE;
+  virtual v8::Handle<v8::Context> GetContext() OVERRIDE;
 
  private:
   // Code which uses PepperTryCatchVar doesn't typically have a HandleScope,

content/renderer/pepper/ppb_var_deprecated_impl.cc

@@ -220,7 +220,7 @@ PP_Var CallDeprecatedInternal(PP_Var var,
 
   v8::Handle<v8::Object> function = accessor.GetObject();
   v8::Handle<v8::Object> recv =
-      accessor.instance()->GetContext()->Global();
+      accessor.instance()->GetMainWorldContext()->Global();
   if (v8_method_name.As<v8::String>()->Length() != 0) {
     function = function->Get(v8_method_name)->ToObject();
     recv = accessor.GetObject();
@@ -290,7 +290,7 @@ bool IsInstanceOfDeprecated(PP_Var var,
     return false;  // Not an object at all.
 
   v8::HandleScope handle_scope(object->instance()->GetIsolate());
-  v8::Context::Scope context_scope(object->instance()->GetContext());
+  v8::Context::Scope context_scope(object->instance()->GetMainWorldContext());
   PluginObject* plugin_object = PluginObject::FromV8Object(
       object->instance()->GetIsolate(), object->GetHandle());
   if (plugin_object && plugin_object->ppp_class() == ppp_class) {