Add PepperTryCatch and V8ObjectVar classes

content/renderer/pepper/pepper_try_catch.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 "content/renderer/pepper/pepper_try_catch.h"
+
+#include "content/renderer/pepper/pepper_plugin_instance_impl.h"
+#include "content/renderer/pepper/v8_var_converter.h"
+#include "gin/converter.h"
+#include "ppapi/shared_impl/ppapi_globals.h"
+#include "ppapi/shared_impl/var_tracker.h"
+
+namespace content {
+
+namespace {
+
+const char kConversionException[] =
+    "Error: Failed conversion between PP_Var and V8 value";
+const char kInvalidException[] = "Error: An invalid exception was thrown.";
+
+}  // namespace
+
+PepperTryCatch::PepperTryCatch(PepperPluginInstanceImpl* instance,
+                               bool convert_objects)
+    : instance_(instance),
+      convert_objects_(convert_objects) {}
+
+PepperTryCatch::~PepperTryCatch() {}
+
+v8::Handle<v8::Context> PepperTryCatch::GetContext() {
+  return instance_->GetContext();
+}
+
+v8::Handle<v8::Value> PepperTryCatch::ToV8(PP_Var var) {
+  V8VarConverter converter(instance_->pp_instance(), convert_objects_);
+  v8::Handle<v8::Value> result;
+  bool success = converter.ToV8Value(var, GetContext(), &result);
+  if (!success) {
+    SetException(kConversionException);
+    return v8::Handle<v8::Value>();
+  }
+  return result;
+}
+
+ppapi::ScopedPPVar PepperTryCatch::FromV8(v8::Handle<v8::Value> v8_value) {
+  if (v8_value.IsEmpty()) {
+    SetException(kConversionException);
+    return ppapi::ScopedPPVar();
+  }
+  ppapi::ScopedPPVar result;
+  V8VarConverter converter(instance_->pp_instance(), convert_objects_);
+  bool success = converter.FromV8ValueSync(v8_value, GetContext(), &result);
+  if (!success) {
+    SetException(kConversionException);
+    return ppapi::ScopedPPVar();
+  }
+  return result;
+}
+
+PepperTryCatchV8::PepperTryCatchV8(PepperPluginInstanceImpl* instance,
+                                   bool convert_objects,
+                                   v8::Isolate* isolate)
+    : PepperTryCatch(instance, convert_objects),
+      exception_(PP_MakeUndefined()) {
+  // Typically when using PepperTryCatchV8 we are passed an isolate. We verify
+  // that this isolate is the same as the plugin isolate.
+  DCHECK(isolate == instance_->GetIsolate());
+  // We assume we are already in the plugin context for PepperTryCatchV8.
+  DCHECK(GetContext() == isolate->GetCurrentContext());
+}
+
+PepperTryCatchV8::~PepperTryCatchV8() {
+  ppapi::PpapiGlobals::Get()->GetVarTracker()->ReleaseVar(exception_);
+}
+
+bool PepperTryCatchV8::HasException() {
+  return exception_.type != PP_VARTYPE_UNDEFINED;
+}
+
+bool PepperTryCatchV8::ThrowException() {
+  if (!HasException())
+    return false;
+
+  std::string message(kInvalidException);
+  ppapi::StringVar* message_var = ppapi::StringVar::FromPPVar(exception_);
+  if (message_var)
+    message = message_var->value();
+  instance_->GetIsolate()->ThrowException(v8::Exception::Error(
+      gin::StringToV8(instance_->GetIsolate(), message)));
+
+  ppapi::PpapiGlobals::Get()->GetVarTracker()->ReleaseVar(exception_);
+  exception_ = PP_MakeUndefined();
+  return true;
+}
+
+void PepperTryCatchV8::ThrowException(const char* message) {
+  SetException(message);
+  ThrowException();
+}
+
+void PepperTryCatchV8::SetException(const char* message) {
+  if (HasException()) {
+    NOTREACHED();
+    return;
+  }
+  exception_ = ppapi::StringVar::StringToPPVar(message);
+}
+
+PepperTryCatchVar::PepperTryCatchVar(PepperPluginInstanceImpl* instance,
+                                     bool convert_objects,
+                                     PP_Var* exception)
+    : PepperTryCatch(instance, convert_objects),
+      handle_scope_(instance_->GetIsolate()),
+      exception_(exception),
+      exception_is_set_(false) {
+  // We switch to the plugin context.
+  if (!GetContext().IsEmpty())
+    GetContext()->Enter();
+}
+
+PepperTryCatchVar::~PepperTryCatchVar() {
+  if (!GetContext().IsEmpty())

[dmichael (off chromium), 2014/07/30 19:34:14]

This should never be empty, right? Maybe you can just:
  GetContext()->Exit();
unconditionally?

[raymes, 2014/08/06 03:22:52]

Done.

[dmichael (off chromium), 2014/08/06 15:41:45]

Hmm, I might have been wrong about this, sorry. Though I think in the case of an
empty Context the old code would have just broken someplace else.

It can wait until we see how the class gets used, but I think you might end up
wanting to make this class handle that case gracefully enough that we can
prevent crashing if a caller needs to use a PepperTryCatchVar but the plugin has
no context. We see this case for surf-away or tab closures sometimes, and it's
been a common source of crashers for us.

I think being blindly optimistic in the latest patchset is good for this CL,
though :)

[raymes, 2014/08/11 00:26:33]

Good to know, let's leave the check out for now and see where we crash :)

Wondering how many times I'm gonna need to revert and reland the CL that flips
the switch :(

+    GetContext()->Exit();
+}
+
+bool PepperTryCatchVar::HasException() {
+  // Check if a v8 exception was caught.
+  if (!exception_is_set_ && try_catch_.HasCaught()) {
+    v8::String::Utf8Value utf8(try_catch_.Message()->Get());
+    if (exception_) {

[dmichael (off chromium), 2014/07/30 19:34:13]

Is this so you can allow a user of this class to just eat the exception?

If so, I think the documentation in the header should say they can pass a NULL
var for that.

OTOH, I actually think it would be better to just do it unconditionally, and
force the user to pass a valid var. That way, they at least have to think about
it and be explicit if they're ignoring it. (Just seeing NULL at the call site is
easier to overlook than an unused out-param)

[raymes, 2014/08/06 03:22:52]

There are two cases when we use this class:
-One is where the plugin code calls in and provides an out-param exception. In
this case, it handles the lifetime.
-We use this class internally in which case we don't care about the out-param
exception and we always pass NULL. 

We could have two constructors but it probably doesn't make much difference.
I've updated the comment for now, but let me know

[dmichael (off chromium), 2014/08/06 15:41:45]

I was trying to say it might be clearer if you just require a PP_Var, and the
caller can choose whether to do anything with it. E.g., a caller who didn't care
about the exception would say:
PP_Var unused;
PepperTryCatchVar catch(instance, true, &unused);
...which just seems slightly more readable to me than...
PepperTryCatchVar catch(instance, true, NULL);

WDYT?

[raymes, 2014/08/11 00:26:33]

Ah I see. I think that this would make the intended behavior clearer. On the
other hand it seems verbose to force a caller to pass an unused param when we
know there are going to be callers like that.

My preference (for the sake of readability of the code which uses this) is to
allow NULL for now, or alternatively I can make a separate constructor which
takes no param. Let me know, I can change it in a followup CL.

+      *exception_ = ppapi::StringVar::StringToPPVar(
+          std::string(*utf8, utf8.length()));
+    }
+    exception_is_set_ = true;
+  }
+
+  return exception_is_set_;
+}
+
+void PepperTryCatchVar::SetException(const char* message) {
+  if (exception_is_set_) {
+    NOTREACHED();
+    return;
+  }
+  if (exception_)
+    *exception_ = ppapi::StringVar::StringToPPVar(message, strlen(message));
+  exception_is_set_ = true;
+}
+
+}  // namespace content