extensions: Use V8 Maybe APIs in ModuleSystem

extensions/renderer/v8_maybe_helpers.h

Index: extensions/renderer/v8_maybe_helpers.h
diff --git a/extensions/renderer/v8_maybe_helpers.h b/extensions/renderer/v8_maybe_helpers.h
new file mode 100644
index 0000000000000000000000000000000000000000..d0b90d868a7637fa2f693819a826f4071e14448d
--- /dev/null
+++ b/extensions/renderer/v8_maybe_helpers.h
@@ -0,0 +1,48 @@
+// Copyright 2015 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 EXTENSIONS_RENDERER_V8_MAYBE_HELPERS_H_
+#define EXTENSIONS_RENDERER_V8_MAYBE_HELPERS_H_

[not at google - send to devlin, 2015/06/12 16:51:37]

I think just "v8_helpers" is fine, maybe extensions will want other v8 helpers
that aren't necessarily to do with maybe. Also, it's fewer characters.

[bashi, 2015/06/16 03:12:47]

Done.

+
+#include "v8/include/v8.h"
+
+namespace extensions {
+

[not at google - send to devlin, 2015/06/12 16:51:37]

I'm a little bit put off by a function as generic named as "SetProperty" being
directly exposed in the global extensions namespace. Can we put this in a
"v8_helpers" namespace, and then do "using v8_helpers" in places they're heavily
used?

[bashi, 2015/06/16 03:12:47]

Done.

+// This crashes when strlen(str) >= v8::String::kMaxLength.
+inline v8::Local<v8::String> ToV8String(
+    v8::Isolate* isolate,
+    const char* str,
+    v8::NewStringType string_type = v8::NewStringType::kNormal) {

[not at google - send to devlin, 2015/06/12 16:51:37]

You may also want to CHECK the string length here?

[bashi, 2015/06/16 03:12:47]

Added as DCHECK(). ToLocalChecked() has CHECK() so we don't need it, but I agree
that having an assertion here is helpful.

+  return v8::String::NewFromUtf8(isolate, str, string_type)
+      .ToLocalChecked();
+}
+
+inline bool CheckV8Call(v8::Maybe<bool> maybe) {

[not at google - send to devlin, 2015/06/12 16:51:37]

Put in an "internal" namespace? I don't see a utility of this function beyond
the scope of this file, it's very specific.

[bashi, 2015/06/16 03:12:47]

I'd prefer putting this in the current namespace. It is used in module_system.cc
and I would use it in follow-up CLs when I replace V8 APIs in other files.

+  return maybe.IsJust() && maybe.FromJust();
+}
+
+inline bool SetProperty(v8::Local<v8::Context> context,
+                        v8::Local<v8::Object> object,
+                        v8::Local<v8::Value> key,
+                        v8::Local<v8::Value> value) {
+  return CheckV8Call(object->Set(context, key, value));
+}
+
+inline bool SetProperty(v8::Local<v8::Context> context,
+                        v8::Local<v8::Object> object,
+                        uint32_t index,
+                        v8::Local<v8::Value> value) {
+  return CheckV8Call(object->Set(context, index, value));
+}
+
+// This crashes when an exception is thrown.
+inline v8::Local<v8::Value> UnsafeGet(v8::Local<v8::Context> context,
+                                      v8::Local<v8::Object> object,
+                                      v8::Local<v8::Value> key) {
+  return object->Get(context, key).ToLocalChecked();
+}
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_RENDERER_V8_MAYBE_HELPERS_H_