extensions: Use V8 Maybe APIs in ModuleSystem

extensions/renderer/v8_helpers.h

+// 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_HELPERS_H_
+#define EXTENSIONS_RENDERER_V8_HELPERS_H_
+
+#include <string.h>
+
+#include "v8/include/v8.h"
+
+namespace extensions {
+namespace v8_helpers {

[not at google - send to devlin, 2015/06/16 20:52:25]

Comments in this class, and its methods, would be nice.

[bashi, 2015/06/17 01:40:45]

Done.

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

[not at google - send to devlin, 2015/06/16 20:52:25]

Seems like it should be <= really, "max length" should be inclusive?

[bashi, 2015/06/17 01:40:45]

Yes. Thanks for the catch.

+  return v8::String::NewFromUtf8(isolate, str, string_type)
+      .ToLocalChecked();
+}
+
+inline bool ToV8String(v8::Isolate* isolate,
+                       const char* str,
+                       v8::Local<v8::String>* out) {
+  return v8::String::NewFromUtf8(isolate, str, v8::NewStringType::kNormal)
+      .ToLocal(out);
+}
+
+inline bool CheckV8Call(v8::Maybe<bool> maybe) {

[not at google - send to devlin, 2015/06/16 20:52:25]

CheckV8Call is not a good name, it sounds like this is some sort of function
call. It also sounds like it does a CHECK.

The comment would be along the lines of "Returns true if |maybe| is both a
value, and that value is true." You could call it "IsTrue"?

[bashi, 2015/06/17 01:40:45]

Done.

+  return maybe.IsJust() && maybe.FromJust();
+}
+
+inline bool SetProperty(v8::Local<v8::Context> context,
+                        v8::Local<v8::Object> object,
+                        v8::Local<v8::Value> key,

[not at google - send to devlin, 2015/06/16 20:52:25]

Another suggestion: provide a version of SetProperty which takes a std::string
or const char* key, since many callers seem to be calling SetProperty(...,
ToV8StringUnsafe(key), ...).

[bashi, 2015/06/17 01:40:45]

Done.

+                        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 v8_helpers
+}  // namespace extensions
+
+#endif  // EXTENSIONS_RENDERER_V8_HELPERS_H_