Fix CJS_PublicMethods::IsNumber() with unit test and some cleanup

R=tsepez@chromium.org

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

6 files changed, 96 insertions, 32 deletions

diff --git a/BUILD.gn b/BUILD.gn
index a08970868e..a4ea471fcf 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1488,6 +1488,15 @@ test("pdfium_unittests") {
       "xfa/fxfa/parser/xfa_utils_imp_unittest.cpp",
     ]
   }
+  if (pdf_enable_v8) {
+    sources += [
+      "fpdfsdk/javascript/public_methods_unittest.cpp",
+    ]
+    include_dirs += [
+      "//v8",
+      "//v8/include",
+    ]
+  }
   configs += [ ":pdfium_config" ]
 }
 
diff --git a/core/include/fxcrt/fx_ext.h b/core/include/fxcrt/fx_ext.h
index dda6a5c72b..0deb4266c8 100644
--- a/core/include/fxcrt/fx_ext.h
+++ b/core/include/fxcrt/fx_ext.h
@@ -30,10 +30,10 @@ FX_WCHAR* FXSYS_wcsncpy(FX_WCHAR* dstStr, const FX_WCHAR* srcStr, size_t count);
 int32_t FXSYS_wcsnicmp(const FX_WCHAR* s1, const FX_WCHAR* s2, size_t count);
 int32_t FXSYS_strnicmp(const FX_CHAR* s1, const FX_CHAR* s2, size_t count);
 
-inline FX_BOOL FXSYS_islower(int32_t ch) {
+inline bool FXSYS_islower(int32_t ch) {
   return ch >= 'a' && ch <= 'z';
 }
-inline FX_BOOL FXSYS_isupper(int32_t ch) {
+inline bool FXSYS_isupper(int32_t ch) {
   return ch >= 'A' && ch <= 'Z';
 }
 inline int32_t FXSYS_tolower(int32_t ch) {
@@ -42,13 +42,13 @@ inline int32_t FXSYS_tolower(int32_t ch) {
 inline int32_t FXSYS_toupper(int32_t ch) {
   return ch < 'a' || ch > 'z' ? ch : (ch - 0x20);
 }
-inline FX_BOOL FXSYS_iswalpha(wchar_t wch) {
+inline bool FXSYS_iswalpha(wchar_t wch) {
   return (wch >= L'A' && wch <= L'Z') || (wch >= L'a' && wch <= L'z');
 }
-inline FX_BOOL FXSYS_iswdigit(wchar_t wch) {
+inline bool FXSYS_iswdigit(wchar_t wch) {
   return wch >= L'0' && wch <= L'9';
 }
-inline FX_BOOL FXSYS_iswalnum(wchar_t wch) {
+inline bool FXSYS_iswalnum(wchar_t wch) {
   return FXSYS_iswalpha(wch) || FXSYS_iswdigit(wch);
 }
 
diff --git a/fpdfsdk/javascript/PublicMethods.cpp b/fpdfsdk/javascript/PublicMethods.cpp
index d154fc1766..6097369430 100644
--- a/fpdfsdk/javascript/PublicMethods.cpp
+++ b/fpdfsdk/javascript/PublicMethods.cpp
@@ -62,44 +62,44 @@ static const FX_WCHAR* const fullmonths[] = {
     L"May",       L"June",     L"July",     L"August",
     L"September", L"October",  L"November", L"December"};
 
-FX_BOOL CJS_PublicMethods::IsNumber(const FX_WCHAR* str) {
+bool CJS_PublicMethods::IsNumber(const FX_WCHAR* str) {
   CFX_WideString sTrim = StrTrim(str);
   const FX_WCHAR* pTrim = sTrim.c_str();
   const FX_WCHAR* p = pTrim;
 
-  FX_BOOL bDot = FALSE;
-  FX_BOOL bKXJS = FALSE;
+  bool bDot = false;
+  bool bKXJS = false;
 
   wchar_t c;
-  while ((c = *p)) {
-    if (c == '.' || c == ',') {
+  while ((c = *p) != L'\0') {
+    if (c == L'.' || c == L',') {
       if (bDot)
-        return FALSE;
-      bDot = TRUE;
-    } else if (c == '-' || c == '+') {
+        return false;
+      bDot = true;
+    } else if (c == L'-' || c == L'+') {
       if (p != pTrim)
-        return FALSE;
-    } else if (c == 'e' || c == 'E') {
+        return false;
+    } else if (c == L'e' || c == L'E') {
       if (bKXJS)
-        return FALSE;
+        return false;
 
       p++;
       c = *p;
-      if (c == '+' || c == '-') {
-        bKXJS = TRUE;
+      if (c == L'+' || c == L'-') {
+        bKXJS = true;
       } else {
-        return FALSE;
+        return false;
       }
     } else if (!FXSYS_iswdigit(c)) {
-      return FALSE;
+      return false;
     }
     p++;
   }
 
-  return TRUE;
+  return true;
 }
 
-FX_BOOL CJS_PublicMethods::maskSatisfied(wchar_t c_Change, wchar_t c_Mask) {
+bool CJS_PublicMethods::maskSatisfied(wchar_t c_Change, wchar_t c_Mask) {
   switch (c_Mask) {
     case L'9':
       return FXSYS_iswdigit(c_Change);
@@ -108,13 +108,13 @@ FX_BOOL CJS_PublicMethods::maskSatisfied(wchar_t c_Change, wchar_t c_Mask) {
     case L'O':
       return FXSYS_iswalnum(c_Change);
     case L'X':
-      return TRUE;
+      return true;
     default:
       return (c_Change == c_Mask);
   }
 }
 
-FX_BOOL CJS_PublicMethods::isReservedMaskChar(wchar_t ch) {
+bool CJS_PublicMethods::isReservedMaskChar(wchar_t ch) {
   return ch == L'9' || ch == L'A' || ch == L'O' || ch == L'X';
 }
 
diff --git a/fpdfsdk/javascript/PublicMethods.h b/fpdfsdk/javascript/PublicMethods.h
index 8961c5abc8..16cc5bc21a 100644
--- a/fpdfsdk/javascript/PublicMethods.h
+++ b/fpdfsdk/javascript/PublicMethods.h
@@ -18,7 +18,6 @@ class CJS_PublicMethods : public CJS_Object {
       : CJS_Object(pObject) {}
   ~CJS_PublicMethods() override {}
 
- public:
   static FX_BOOL AFNumber_Format(IJS_Context* cc,
                                  const std::vector<CJS_Value>& params,
                                  CJS_Value& vRet,
@@ -108,7 +107,6 @@ class CJS_PublicMethods : public CJS_Object {
                                CJS_Value& vRet,
                                CFX_WideString& sError);
 
- public:
   JS_STATIC_GLOBAL_FUN(AFNumber_Format);
   JS_STATIC_GLOBAL_FUN(AFNumber_Keystroke);
   JS_STATIC_GLOBAL_FUN(AFPercent_Format);
@@ -134,7 +132,6 @@ class CJS_PublicMethods : public CJS_Object {
 
   JS_STATIC_DECLARE_GLOBAL_FUN();
 
- public:
   static int ParseStringInteger(const CFX_WideString& string,
                                 int nStart,
                                 int& nSkip,
@@ -151,7 +148,6 @@ class CJS_PublicMethods : public CJS_Object {
                                 bool* bWrongFormat);
   static double MakeInterDate(CFX_WideString strValue);
 
- public:
   static CFX_WideString StrLTrim(const FX_WCHAR* pStr);
   static CFX_WideString StrRTrim(const FX_WCHAR* pStr);
   static CFX_WideString StrTrim(const FX_WCHAR* pStr);
@@ -160,11 +156,10 @@ class CJS_PublicMethods : public CJS_Object {
   static CFX_ByteString StrRTrim(const FX_CHAR* pStr);
   static CFX_ByteString StrTrim(const FX_CHAR* pStr);
 
-  static FX_BOOL IsNumber(const FX_CHAR* string);
-  static FX_BOOL IsNumber(const FX_WCHAR* string);
+  static bool IsNumber(const FX_WCHAR* string);
 
-  static FX_BOOL maskSatisfied(wchar_t c_Change, wchar_t c_Mask);
-  static FX_BOOL isReservedMaskChar(wchar_t ch);
+  static bool maskSatisfied(wchar_t c_Change, wchar_t c_Mask);
+  static bool isReservedMaskChar(wchar_t ch);
 
   static double AF_Simple(const FX_WCHAR* sFuction,
                           double dValue1,
diff --git a/fpdfsdk/javascript/public_methods_unittest.cpp b/fpdfsdk/javascript/public_methods_unittest.cpp
new file mode 100644
index 0000000000..ace0920e8f
--- /dev/null
+++ b/fpdfsdk/javascript/public_methods_unittest.cpp
@@ -0,0 +1,51 @@
+// Copyright 2016 PDFium 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 "fpdfsdk/javascript/PublicMethods.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/test_support.h"
+
+TEST(CJS_PublicMethods, IsNumber) {
+  // TODO(weili): Check whether results from case 0, 1, 10, 15 are intended.
+  struct {
+    const wchar_t* input;
+    bool expected;
+  } test_data[] = {
+      // Empty string.
+      {L"", true},
+      // Only whitespaces.
+      {L"  ", true},
+      // Content with invalid characters.
+      {L"xyz00", false},
+      {L"1%", false},
+      // Hex string.
+      {L"0x234", false},
+      // Signed numbers.
+      {L"+123", true},
+      {L"-98765", true},
+      // Numbers with whitespaces.
+      {L"  345 ", true},
+      // Float numbers.
+      {L"-1e5", false},
+      {L"-2e", false},
+      {L"e-5", true},
+      {L"0.023", true},
+      {L".356089", true},
+      {L"1e-9", true},
+      {L"-1.23e+23", true},
+      // Numbers with commas.
+      {L"1,000,000", false},
+      {L"560,024", true},
+      // Regular numbers.
+      {L"0", true},
+      {L"0123", true},
+      {L"9876123", true},
+  };
+  for (size_t i = 0; i < FX_ArraySize(test_data); ++i) {
+    EXPECT_EQ(test_data[i].expected,
+              CJS_PublicMethods::IsNumber(test_data[i].input))
+        << "for case " << i;
+  }
+}
diff --git a/pdfium.gyp b/pdfium.gyp
index f440b9e018..cdbf184970 100644
--- a/pdfium.gyp
+++ b/pdfium.gyp
@@ -853,6 +853,15 @@
             'xfa/fxfa/parser/xfa_utils_imp_unittest.cpp',
           ],
         }],
+        ['pdf_enable_v8==1', {
+          'include_dirs': [
+            '<(DEPTH)/v8',
+            '<(DEPTH)/v8/include',
+          ],
+          'sources': [
+            'fpdfsdk/javascript/public_methods_unittest.cpp',
+          ],
+        }],
       ],
     },
     {