Fix strings, remove stringify macros, void return types for Consts.h.

Replace multiple #defines of the same strings with externs.
Fix strings mangled by interaction of # and clang-format.
Remove macros as possible.
Make more JS_ functions void and simplify.

R=thestig@chromium.org

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

3 files changed, 104 insertions, 117 deletions

diff --git a/fpdfsdk/src/javascript/Consts.cpp b/fpdfsdk/src/javascript/Consts.cpp
index b534d2b393..9574c44730 100644
--- a/fpdfsdk/src/javascript/Consts.cpp
+++ b/fpdfsdk/src/javascript/Consts.cpp
@@ -129,60 +129,62 @@ IMPLEMENT_JS_CLASS_CONST(CJS_Zoomtype, zoomtype)
 
 /* ------------------------------ CJS_GlobalConsts ------------------------- */
 
-#define DEFINE_GLOBAL_CONST(pIsolate, const_name, const_value)   \
-  if (JS_DefineGlobalConst(                                      \
-          pIsolate, JS_WIDESTRING(const_name),                   \
-          JS_NewString(pIsolate, JS_WIDESTRING(const_value)))) { \
-    return -1;                                                   \
-  }
+static void DefineGlobalConstString(v8::Isolate* pIsolate,
+                                    const wchar_t* pConstName,
+                                    const wchar_t* pValue) {
+  JS_DefineGlobalConst(pIsolate, pConstName, JS_NewString(pIsolate, pValue));
+}
 
-int CJS_GlobalConsts::DefineJSObjects(v8::Isolate* pIsolate) {
-  DEFINE_GLOBAL_CONST(pIsolate, IDS_GREATER_THAN, Invalid value
-                      : must be greater than or equal to % s.);
-  DEFINE_GLOBAL_CONST(
-      pIsolate, IDS_GT_AND_LT, Invalid value
-      : must be greater than or equal to % s and less than or equal to % s.);
-  DEFINE_GLOBAL_CONST(pIsolate, IDS_LESS_THAN, Invalid value
-                      : must be less than or equal to % s.);
-  DEFINE_GLOBAL_CONST(pIsolate, IDS_INVALID_MONTH, **Invalid**);
-  DEFINE_GLOBAL_CONST(pIsolate, IDS_INVALID_DATE, Invalid date / time
-                      : please ensure that the date / time exists.Field);
-  DEFINE_GLOBAL_CONST(pIsolate, IDS_INVALID_VALUE,
-                      The value entered does not match the format of the field);
-  DEFINE_GLOBAL_CONST(pIsolate, IDS_AM, am);
-  DEFINE_GLOBAL_CONST(pIsolate, IDS_PM, pm);
-  DEFINE_GLOBAL_CONST(
-      pIsolate, IDS_MONTH_INFO,
-      January[1] February[2] March[3] April[4] May[5] June[6] July[7] August
-          [8] September[9] October[10] November[11] December[12] Sept[9] Jan
-              [1] Feb[2] Mar[3] Apr[4] Jun[6] Jul[7] Aug[8] Sep[9] Oct[10] Nov
-                  [11] Dec[12]);
-  DEFINE_GLOBAL_CONST(pIsolate, IDS_STARTUP_CONSOLE_MSG, ** ^ _ ^ **);
-
-  return 0;
+void CJS_GlobalConsts::DefineJSObjects(v8::Isolate* pIsolate) {
+  DefineGlobalConstString(
+      pIsolate, L"IDS_GREATER_THAN",
+      L"Invalid value: must be greater than or equal to % s.");
+  DefineGlobalConstString(
+      pIsolate, L"IDS_GT_AND_LT",
+      L"Invalid value: must be greater than or equal to % s "
+      L"and less than or equal to % s.");
+  DefineGlobalConstString(pIsolate, L"IDS_LESS_THAN",
+                          L"Invalid value: must be less than or equal to % s.");
+  DefineGlobalConstString(pIsolate, L"IDS_INVALID_MONTH", L"**Invalid**");
+  DefineGlobalConstString(
+      pIsolate, L"IDS_INVALID_DATE",
+      L"Invalid date / time: please ensure that the date / time exists.Field");
+  DefineGlobalConstString(
+      pIsolate, L"IDS_INVALID_VALUE",
+      L"The value entered does not match the format of the field");
+  DefineGlobalConstString(pIsolate, L"IDS_AM", L"am");
+  DefineGlobalConstString(pIsolate, L"IDS_PM", L"pm");
+  DefineGlobalConstString(
+      pIsolate, L"IDS_MONTH_INFO",
+      L"January[1] February[2] March[3] April[4] May[5] "
+      L"June[6] July[7] August[8] September[9] October[10] "
+      L"November[11] December[12] Sept[9] Jan[1] Feb[2] Mar[3] "
+      L"Apr[4] Jun[6] Jul[7] Aug[8] Sep[9] Oct[10] Nov[11] "
+      L"Dec[12]");
+  DefineGlobalConstString(pIsolate, L"IDS_STARTUP_CONSOLE_MSG", L"** ^ _ ^ **");
 }
 
 /* ------------------------------ CJS_GlobalArrays  ------------------------ */
 
-#define DEFINE_GLOBAL_ARRAY(pIsolate)                           \
-  int size = FX_ArraySize(ArrayContent);                        \
-                                                                \
-  CJS_Array array(pIsolate);                                    \
-  for (int i = 0; i < size; i++)                                \
-    array.SetElement(i, CJS_Value(pIsolate, ArrayContent[i]));  \
-                                                                \
-  CJS_PropValue prop(pIsolate);                                 \
-  prop << array;                                                \
-  if (JS_DefineGlobalConst(pIsolate, (const wchar_t*)ArrayName, \
-                           prop.ToV8Value()) < 0) {             \
-    return -1;                                                  \
+void DefineGlobalConstStringArray(v8::Isolate* pIsolate,
+                                  const wchar_t* sConstName,
+                                  const wchar_t** pValues,
+                                  size_t nValues) {
+  CJS_Array array(pIsolate);
+  for (size_t i = 0; i < nValues; ++i) {
+    array.SetElement(i, CJS_Value(pIsolate, pValues[i]));
   }
+  CJS_PropValue prop(pIsolate);
+  prop << array;
+  JS_DefineGlobalConst(pIsolate, sConstName, prop.ToV8Value());
+}
 
-int CJS_GlobalArrays::DefineJSObjects(v8::Isolate* pIsolate) {
+void CJS_GlobalArrays::DefineJSObjects(v8::Isolate* pIsolate) {
   {
     const FX_WCHAR* ArrayName = L"RE_NUMBER_ENTRY_DOT_SEP";
     const FX_WCHAR* ArrayContent[] = {L"[+-]?\\d*\\.?\\d*"};
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
@@ -192,14 +194,16 @@ int CJS_GlobalArrays::DefineJSObjects(v8::Isolate* pIsolate) {
         L"[+-]?\\.\\d+",        /* -.1 */
         L"[+-]?\\d+\\."         /* -1. */
     };
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
     const FX_WCHAR* ArrayName = L"RE_NUMBER_ENTRY_COMMA_SEP";
     const FX_WCHAR* ArrayContent[] = {L"[+-]?\\d*,?\\d*"};
 
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
@@ -209,31 +213,36 @@ int CJS_GlobalArrays::DefineJSObjects(v8::Isolate* pIsolate) {
         L"[+-]?[.,]\\d+",        /* -,1 */
         L"[+-]?\\d+[.,]"         /* -1, */
     };
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
     const FX_WCHAR* ArrayName = L"RE_ZIP_ENTRY";
     const FX_WCHAR* ArrayContent[] = {L"\\d{0,5}"};
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
     const FX_WCHAR* ArrayName = L"RE_ZIP_COMMIT";
     const FX_WCHAR* ArrayContent[] = {L"\\d{5}"};
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
     const FX_WCHAR* ArrayName = L"RE_ZIP4_ENTRY";
     const FX_WCHAR* ArrayContent[] = {L"\\d{0,5}(\\.|[- ])?\\d{0,4}"};
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
     const FX_WCHAR* ArrayName = L"RE_ZIP4_COMMIT";
     const FX_WCHAR* ArrayContent[] = {L"\\d{5}(\\.|[- ])?\\d{4}"};
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
@@ -251,7 +260,8 @@ int CJS_GlobalArrays::DefineJSObjects(v8::Isolate* pIsolate) {
                                                                  */
         L"011(\\.|[- \\d])*" /* international */
     };
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
@@ -262,22 +272,23 @@ int CJS_GlobalArrays::DefineJSObjects(v8::Isolate* pIsolate) {
         L"\\(\\d{3}\\)(\\.|[- ])?\\d{3}(\\.|[- ])?\\d{4}", /* (408) 555-1234 */
         L"011(\\.|[- \\d])*"                               /* international */
     };
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
     const FX_WCHAR* ArrayName = L"RE_SSN_ENTRY";
     const FX_WCHAR* ArrayContent[] = {
         L"\\d{0,3}(\\.|[- ])?\\d{0,2}(\\.|[- ])?\\d{0,4}"};
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
 
   {
     const FX_WCHAR* ArrayName = L"RE_SSN_COMMIT";
     const FX_WCHAR* ArrayContent[] = {
         L"\\d{3}(\\.|[- ])?\\d{2}(\\.|[- ])?\\d{4}"};
-    DEFINE_GLOBAL_ARRAY(pIsolate);
+    DefineGlobalConstStringArray(pIsolate, ArrayName, ArrayContent,
+                                 FX_ArraySize(ArrayContent));
   }
-
-  return 0;
 }
diff --git a/fpdfsdk/src/javascript/JS_Runtime.cpp b/fpdfsdk/src/javascript/JS_Runtime.cpp
index 5f4a473c87..4b18d80f6c 100644
--- a/fpdfsdk/src/javascript/JS_Runtime.cpp
+++ b/fpdfsdk/src/javascript/JS_Runtime.cpp
@@ -137,7 +137,7 @@ CJS_Runtime::~CJS_Runtime() {
     m_isolate->Dispose();
 }
 
-FX_BOOL CJS_Runtime::DefineJSObjects() {
+void CJS_Runtime::DefineJSObjects() {
   v8::Isolate::Scope isolate_scope(GetIsolate());
   v8::HandleScope handle_scope(GetIsolate());
   v8::Local<v8::Context> context = v8::Context::New(GetIsolate());
@@ -145,67 +145,43 @@ FX_BOOL CJS_Runtime::DefineJSObjects() {
 
   // The call order determines the "ObjDefID" assigned to each class.
   // ObjDefIDs 0 - 2
-  if (CJS_Border::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-  if (CJS_Display::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-  if (CJS_Font::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
+  CJS_Border::DefineJSObjects(GetIsolate(), JS_STATIC);
+  CJS_Display::DefineJSObjects(GetIsolate(), JS_STATIC);
+  CJS_Font::DefineJSObjects(GetIsolate(), JS_STATIC);
 
   // ObjDefIDs 3 - 5
-  if (CJS_Highlight::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-  if (CJS_Position::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-  if (CJS_ScaleHow::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
+  CJS_Highlight::DefineJSObjects(GetIsolate(), JS_STATIC);
+  CJS_Position::DefineJSObjects(GetIsolate(), JS_STATIC);
+  CJS_ScaleHow::DefineJSObjects(GetIsolate(), JS_STATIC);
 
   // ObjDefIDs 6 - 8
-  if (CJS_ScaleWhen::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-  if (CJS_Style::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-  if (CJS_Zoomtype::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
+  CJS_ScaleWhen::DefineJSObjects(GetIsolate(), JS_STATIC);
+  CJS_Style::DefineJSObjects(GetIsolate(), JS_STATIC);
+  CJS_Zoomtype::DefineJSObjects(GetIsolate(), JS_STATIC);
 
   // ObjDefIDs 9 - 11
-  if (CJS_App::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-  if (CJS_Color::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-  if (CJS_Console::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
+  CJS_App::DefineJSObjects(GetIsolate(), JS_STATIC);
+  CJS_Color::DefineJSObjects(GetIsolate(), JS_STATIC);
+  CJS_Console::DefineJSObjects(GetIsolate(), JS_STATIC);
 
   // ObjDefIDs 12 - 14
-  if (CJS_Document::DefineJSObjects(GetIsolate(), JS_DYNAMIC) < 0)
-    return FALSE;
-  if (CJS_Event::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-  if (CJS_Field::DefineJSObjects(GetIsolate(), JS_DYNAMIC) < 0)
-    return FALSE;
+  CJS_Document::DefineJSObjects(GetIsolate(), JS_DYNAMIC);
+  CJS_Event::DefineJSObjects(GetIsolate(), JS_STATIC);
+  CJS_Field::DefineJSObjects(GetIsolate(), JS_DYNAMIC);
 
   // ObjDefIDs 15 - 17
-  if (CJS_Global::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-  if (CJS_Icon::DefineJSObjects(GetIsolate(), JS_DYNAMIC) < 0)
-    return FALSE;
-  if (CJS_Util::DefineJSObjects(GetIsolate(), JS_STATIC) < 0)
-    return FALSE;
-
-  // ObjDefIDs 18 - 20
-  if (CJS_PublicMethods::DefineJSObjects(GetIsolate()) < 0)
-    return FALSE;
-  if (CJS_GlobalConsts::DefineJSObjects(GetIsolate()) < 0)
-    return FALSE;
-  if (CJS_GlobalArrays::DefineJSObjects(GetIsolate()) < 0)
-    return FALSE;
-
-  if (CJS_TimerObj::DefineJSObjects(GetIsolate(), JS_DYNAMIC) < 0)
-    return FALSE;
-  if (CJS_PrintParamsObj::DefineJSObjects(GetIsolate(), JS_DYNAMIC) < 0)
-    return FALSE;
-
-  return TRUE;
+  CJS_Global::DefineJSObjects(GetIsolate(), JS_STATIC);
+  CJS_Icon::DefineJSObjects(GetIsolate(), JS_DYNAMIC);
+  CJS_Util::DefineJSObjects(GetIsolate(), JS_STATIC);
+
+  // ObjDefIDs 18 - 20 (these can't fail, return void).
+  CJS_PublicMethods::DefineJSObjects(GetIsolate());
+  CJS_GlobalConsts::DefineJSObjects(GetIsolate());
+  CJS_GlobalArrays::DefineJSObjects(GetIsolate());
+
+  // ObjDefIDs 21 - 22.
+  CJS_TimerObj::DefineJSObjects(GetIsolate(), JS_DYNAMIC);
+  CJS_PrintParamsObj::DefineJSObjects(GetIsolate(), JS_DYNAMIC);
 }
 
 IFXJS_Context* CJS_Runtime::NewContext() {
diff --git a/fpdfsdk/src/javascript/global.cpp b/fpdfsdk/src/javascript/global.cpp
index e817eca32c..bde4e7d5ae 100644
--- a/fpdfsdk/src/javascript/global.cpp
+++ b/fpdfsdk/src/javascript/global.cpp
@@ -60,21 +60,21 @@ class HashVerify {
 
 HashVerify::HashVerify() {
   ASSERT(JSCONST_nStringHash ==
-         JS_CalcHash(VALUE_NAME_STRING, wcslen(VALUE_NAME_STRING)));
+         JS_CalcHash(kFXJSValueNameString, wcslen(kFXJSValueNameString)));
   ASSERT(JSCONST_nNumberHash ==
-         JS_CalcHash(VALUE_NAME_NUMBER, wcslen(VALUE_NAME_NUMBER)));
+         JS_CalcHash(kFXJSValueNameNumber, wcslen(kFXJSValueNameNumber)));
   ASSERT(JSCONST_nBoolHash ==
-         JS_CalcHash(VALUE_NAME_BOOLEAN, wcslen(VALUE_NAME_BOOLEAN)));
+         JS_CalcHash(kFXJSValueNameBoolean, wcslen(kFXJSValueNameBoolean)));
   ASSERT(JSCONST_nDateHash ==
-         JS_CalcHash(VALUE_NAME_DATE, wcslen(VALUE_NAME_DATE)));
+         JS_CalcHash(kFXJSValueNameDate, wcslen(kFXJSValueNameDate)));
   ASSERT(JSCONST_nObjectHash ==
-         JS_CalcHash(VALUE_NAME_OBJECT, wcslen(VALUE_NAME_OBJECT)));
+         JS_CalcHash(kFXJSValueNameObject, wcslen(kFXJSValueNameObject)));
   ASSERT(JSCONST_nFXobjHash ==
-         JS_CalcHash(VALUE_NAME_FXOBJ, wcslen(VALUE_NAME_FXOBJ)));
+         JS_CalcHash(kFXJSValueNameFxobj, wcslen(kFXJSValueNameFxobj)));
   ASSERT(JSCONST_nNullHash ==
-         JS_CalcHash(VALUE_NAME_NULL, wcslen(VALUE_NAME_NULL)));
+         JS_CalcHash(kFXJSValueNameNull, wcslen(kFXJSValueNameNull)));
   ASSERT(JSCONST_nUndefHash ==
-         JS_CalcHash(VALUE_NAME_UNDEFINED, wcslen(VALUE_NAME_UNDEFINED)));
+         JS_CalcHash(kFXJSValueNameUndefined, wcslen(kFXJSValueNameUndefined)));
 }
 #endif