Replace some repeated code with existing CFXJS_Engine::NewString() helper.

In turn, fix type information for New*() return values.
In turn, resolve some ambiguity in a ? operator expression.

Change-Id: I45c31d81f815b11590b46dacc8060d8e489dde5c
Reviewed-on: https://pdfium-review.googlesource.com/6131
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

3 files changed, 35 insertions, 61 deletions

diff --git a/fpdfsdk/javascript/JS_Define.h b/fpdfsdk/javascript/JS_Define.h
index fb901dc714..f360e8f6b9 100644
--- a/fpdfsdk/javascript/JS_Define.h
+++ b/fpdfsdk/javascript/JS_Define.h
@@ -173,14 +173,15 @@ void JSMethod(const char* method_name_string,
   static JSConstSpec ConstSpecs[];    \
   static void DefineConsts(CFXJS_Engine* pEngine);
 
-#define IMPLEMENT_JS_CLASS_CONST_PART(js_class_name, class_name)             \
-  void js_class_name::DefineConsts(CFXJS_Engine* pEngine) {                  \
-    for (size_t i = 0; i < FX_ArraySize(ConstSpecs) - 1; ++i) {              \
-      pEngine->DefineObjConst(g_nObjDefnID, ConstSpecs[i].pName,             \
-                              ConstSpecs[i].eType == JSConstSpec::Number     \
-                                  ? pEngine->NewNumber(ConstSpecs[i].number) \
-                                  : pEngine->NewString(ConstSpecs[i].pStr)); \
-    }                                                                        \
+#define IMPLEMENT_JS_CLASS_CONST_PART(js_class_name, class_name)         \
+  void js_class_name::DefineConsts(CFXJS_Engine* pEngine) {              \
+    for (size_t i = 0; i < FX_ArraySize(ConstSpecs) - 1; ++i) {          \
+      pEngine->DefineObjConst(                                           \
+          g_nObjDefnID, ConstSpecs[i].pName,                             \
+          ConstSpecs[i].eType == JSConstSpec::Number                     \
+              ? pEngine->NewNumber(ConstSpecs[i].number).As<v8::Value>() \
+              : pEngine->NewString(ConstSpecs[i].pStr).As<v8::Value>()); \
+    }                                                                    \
   }
 
 // Convenience macros for declaring classes without an alternate.
diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp
index ce7dc50e63..970e4f144d 100644
--- a/fxjs/fxjs_v8.cpp
+++ b/fxjs/fxjs_v8.cpp
@@ -311,11 +311,8 @@ void CFXJS_Engine::DefineObjMethod(int nObjDefnID,
   v8::Local<v8::FunctionTemplate> fun = v8::FunctionTemplate::New(
       m_isolate, pMethodCall, v8::Local<v8::Value>(), pObjDef->GetSignature());
   fun->RemovePrototype();
-  pObjDef->GetInstanceTemplate()->Set(
-      v8::String::NewFromUtf8(m_isolate, sMethodName,
-                              v8::NewStringType::kNormal)
-          .ToLocalChecked(),
-      fun, v8::ReadOnly);
+  pObjDef->GetInstanceTemplate()->Set(NewString(sMethodName), fun,
+                                      v8::ReadOnly);
 }
 
 void CFXJS_Engine::DefineObjProperty(int nObjDefnID,
@@ -326,10 +323,8 @@ void CFXJS_Engine::DefineObjProperty(int nObjDefnID,
   v8::HandleScope handle_scope(m_isolate);
   CFXJS_ObjDefinition* pObjDef =
       CFXJS_ObjDefinition::ForID(m_isolate, nObjDefnID);
-  pObjDef->GetInstanceTemplate()->SetAccessor(
-      v8::String::NewFromUtf8(m_isolate, sPropName, v8::NewStringType::kNormal)
-          .ToLocalChecked(),
-      pPropGet, pPropPut);
+  pObjDef->GetInstanceTemplate()->SetAccessor(NewString(sPropName), pPropGet,
+                                              pPropPut);
 }
 
 void CFXJS_Engine::DefineObjAllProperties(
@@ -363,26 +358,19 @@ void CFXJS_Engine::DefineGlobalMethod(const char* sMethodName,
   v8::Local<v8::FunctionTemplate> fun =
       v8::FunctionTemplate::New(m_isolate, pMethodCall);
   fun->RemovePrototype();
-  GetGlobalObjectTemplate(m_isolate)->Set(
-      v8::String::NewFromUtf8(m_isolate, sMethodName,
-                              v8::NewStringType::kNormal)
-          .ToLocalChecked(),
-      fun, v8::ReadOnly);
+  GetGlobalObjectTemplate(m_isolate)->Set(NewString(sMethodName), fun,
+                                          v8::ReadOnly);
 }
 
 void CFXJS_Engine::DefineGlobalConst(const wchar_t* sConstName,
                                      v8::FunctionCallback pConstGetter) {
   v8::Isolate::Scope isolate_scope(m_isolate);
   v8::HandleScope handle_scope(m_isolate);
-  CFX_ByteString bsConst = FX_UTF8Encode(CFX_WideStringC(sConstName));
   v8::Local<v8::FunctionTemplate> fun =
       v8::FunctionTemplate::New(m_isolate, pConstGetter);
   fun->RemovePrototype();
-  GetGlobalObjectTemplate(m_isolate)->SetAccessorProperty(
-      v8::String::NewFromUtf8(m_isolate, bsConst.c_str(),
-                              v8::NewStringType::kNormal)
-          .ToLocalChecked(),
-      fun);
+  GetGlobalObjectTemplate(m_isolate)->SetAccessorProperty(NewString(sConstName),
+                                                          fun);
 }
 
 void CFXJS_Engine::InitializeEngine() {
@@ -419,12 +407,7 @@ void CFXJS_Engine::InitializeEngine() {
                                           .ToLocalChecked());
       }
     } else if (pObjDef->m_ObjType == FXJSOBJTYPE_STATIC) {
-      v8::Local<v8::String> pObjName =
-          v8::String::NewFromUtf8(m_isolate, pObjDef->m_ObjName,
-                                  v8::NewStringType::kNormal,
-                                  strlen(pObjDef->m_ObjName))
-              .ToLocalChecked();
-
+      v8::Local<v8::String> pObjName = NewString(pObjDef->m_ObjName);
       v8::Local<v8::Object> obj = NewFxDynamicObj(i, true);
       v8Context->Global()->Set(v8Context, pObjName, obj).FromJust();
       m_StaticObjects[i] = new v8::Global<v8::Object>(m_isolate, obj);
@@ -439,7 +422,6 @@ void CFXJS_Engine::ReleaseEngine() {
   v8::Local<v8::Context> context =
       v8::Local<v8::Context>::New(m_isolate, m_V8PersistentContext);
   v8::Context::Scope context_scope(context);
-
   FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(m_isolate);
   if (!pData)
     return;
@@ -478,14 +460,9 @@ void CFXJS_Engine::ReleaseEngine() {
 int CFXJS_Engine::Execute(const CFX_WideString& script, FXJSErr* pError) {
   v8::Isolate::Scope isolate_scope(m_isolate);
   v8::TryCatch try_catch(m_isolate);
-  CFX_ByteString bsScript = script.UTF8Encode();
   v8::Local<v8::Context> context = m_isolate->GetCurrentContext();
   v8::Local<v8::Script> compiled_script;
-  if (!v8::Script::Compile(context,
-                           v8::String::NewFromUtf8(m_isolate, bsScript.c_str(),
-                                                   v8::NewStringType::kNormal,
-                                                   bsScript.GetLength())
-                               .ToLocalChecked())
+  if (!v8::Script::Compile(context, NewString(script.AsStringC()))
            .ToLocal(&compiled_script)) {
     v8::String::Utf8Value error(try_catch.Exception());
     // TODO(tsepez): return error via pError->message.
@@ -548,14 +525,7 @@ v8::Local<v8::Object> CFXJS_Engine::GetThisObj() {
 }
 
 void CFXJS_Engine::Error(const CFX_WideString& message) {
-  // Conversion from pdfium's wchar_t wide-strings to v8's uint16_t
-  // wide-strings isn't handled by v8, so use UTF8 as a common
-  // intermediate format.
-  CFX_ByteString utf8_message = message.UTF8Encode();
-  m_isolate->ThrowException(v8::String::NewFromUtf8(m_isolate,
-                                                    utf8_message.c_str(),
-                                                    v8::NewStringType::kNormal)
-                                .ToLocalChecked());
+  m_isolate->ThrowException(NewString(message.AsStringC()));
 }
 
 void CFXJS_Engine::SetObjectPrivate(v8::Local<v8::Object> pObj, void* p) {
@@ -660,30 +630,33 @@ v8::Local<v8::Context> CFXJS_Engine::GetPersistentContext() {
   return m_V8PersistentContext.Get(m_isolate);
 }
 
-v8::Local<v8::Value> CFXJS_Engine::NewNumber(int number) {
+v8::Local<v8::Number> CFXJS_Engine::NewNumber(int number) {
   return v8::Int32::New(m_isolate, number);
 }
 
-v8::Local<v8::Value> CFXJS_Engine::NewNumber(double number) {
+v8::Local<v8::Number> CFXJS_Engine::NewNumber(double number) {
   return v8::Number::New(m_isolate, number);
 }
 
-v8::Local<v8::Value> CFXJS_Engine::NewNumber(float number) {
+v8::Local<v8::Number> CFXJS_Engine::NewNumber(float number) {
   return v8::Number::New(m_isolate, (float)number);
 }
 
-v8::Local<v8::Value> CFXJS_Engine::NewBoolean(bool b) {
+v8::Local<v8::Boolean> CFXJS_Engine::NewBoolean(bool b) {
   return v8::Boolean::New(m_isolate, b);
 }
 
-v8::Local<v8::Value> CFXJS_Engine::NewString(const CFX_ByteStringC& str) {
+v8::Local<v8::String> CFXJS_Engine::NewString(const CFX_ByteStringC& str) {
   v8::Isolate* pIsolate = m_isolate ? m_isolate : v8::Isolate::GetCurrent();
   return v8::String::NewFromUtf8(pIsolate, str.c_str(),
                                  v8::NewStringType::kNormal, str.GetLength())
       .ToLocalChecked();
 }
 
-v8::Local<v8::Value> CFXJS_Engine::NewString(const CFX_WideStringC& str) {
+v8::Local<v8::String> CFXJS_Engine::NewString(const CFX_WideStringC& str) {
+  // Conversion from pdfium's wchar_t wide-strings to v8's uint16_t
+  // wide-strings isn't handled by v8, so use UTF8 as a common
+  // intermediate format.
   return NewString(FX_UTF8Encode(str).AsStringC());
 }
 
diff --git a/fxjs/fxjs_v8.h b/fxjs/fxjs_v8.h
index d44af2f8fa..cdec942d4b 100644
--- a/fxjs/fxjs_v8.h
+++ b/fxjs/fxjs_v8.h
@@ -182,12 +182,12 @@ class CFXJS_Engine {
 
   v8::Local<v8::Value> NewNull();
   v8::Local<v8::Array> NewArray();
-  v8::Local<v8::Value> NewNumber(int number);
-  v8::Local<v8::Value> NewNumber(double number);
-  v8::Local<v8::Value> NewNumber(float number);
-  v8::Local<v8::Value> NewBoolean(bool b);
-  v8::Local<v8::Value> NewString(const CFX_ByteStringC& str);
-  v8::Local<v8::Value> NewString(const CFX_WideStringC& str);
+  v8::Local<v8::Number> NewNumber(int number);
+  v8::Local<v8::Number> NewNumber(double number);
+  v8::Local<v8::Number> NewNumber(float number);
+  v8::Local<v8::Boolean> NewBoolean(bool b);
+  v8::Local<v8::String> NewString(const CFX_ByteStringC& str);
+  v8::Local<v8::String> NewString(const CFX_WideStringC& str);
   v8::Local<v8::Date> NewDate(double d);
   v8::Local<v8::Object> NewFxDynamicObj(int nObjDefnID, bool bStatic = false);