Clean up IFX_BidiChar

- Replace IFX_BidiChar with just CFX_BidiChar
- Document implementation
- Change out parameters to pointers
- Remove dead code
- Add an enum for bidi directions
- Move several externs to a header
- Add unit tests

R=tsepez@chromium.org

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

6 files changed, 230 insertions, 163 deletions

diff --git a/core/src/fxcrt/fx_arabic.cpp b/core/src/fxcrt/fx_arabic.cpp
deleted file mode 100644
index 3b7d0c1c71..0000000000
--- a/core/src/fxcrt/fx_arabic.cpp
+++ /dev/null
@@ -1,84 +0,0 @@
-// Copyright 2014 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.
-
-// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-
-#include "../../include/fxcrt/fx_ucd.h"
-#include "fx_arabic.h"
-
-extern const FX_DWORD gs_FX_TextLayout_CodeProperties[65536];
-IFX_BidiChar* IFX_BidiChar::Create() {
-  return new CFX_BidiChar;
-}
-CFX_BidiChar::CFX_BidiChar()
-    : m_bSeparateNeutral(TRUE),
-      m_iCurStart(0),
-      m_iCurCount(0),
-      m_iCurBidi(0),
-      m_iLastBidi(0),
-      m_iLastStart(0),
-      m_iLastCount(0) {}
-void CFX_BidiChar::SetPolicy(FX_BOOL bSeparateNeutral) {
-  m_bSeparateNeutral = bSeparateNeutral;
-}
-
-FX_BOOL CFX_BidiChar::AppendChar(FX_WCHAR wch) {
-  FX_DWORD dwProps = gs_FX_TextLayout_CodeProperties[(FX_WORD)wch];
-  int32_t iBidiCls = (dwProps & FX_BIDICLASSBITSMASK) >> FX_BIDICLASSBITS;
-  int32_t iContext = 0;
-  switch (iBidiCls) {
-    case FX_BIDICLASS_L:
-    case FX_BIDICLASS_AN:
-    case FX_BIDICLASS_EN:
-      iContext = 1;
-      break;
-    case FX_BIDICLASS_R:
-    case FX_BIDICLASS_AL:
-      iContext = 2;
-      break;
-  }
-  FX_BOOL bRet = FALSE;
-  if (iContext != m_iCurBidi) {
-    if (m_bSeparateNeutral) {
-      bRet = TRUE;
-    } else {
-      if (m_iCurBidi == 0) {
-        bRet = (m_iCurCount > 0);
-      } else {
-        bRet = (iContext != 0);
-      }
-    }
-    if (bRet) {
-      m_iLastBidi = m_iCurBidi;
-      m_iLastStart = m_iCurStart;
-      m_iCurStart = m_iCurCount;
-      m_iLastCount = m_iCurCount - m_iLastStart;
-    }
-    if (m_bSeparateNeutral || iContext != 0) {
-      m_iCurBidi = iContext;
-    }
-  }
-  m_iCurCount++;
-  return bRet;
-}
-FX_BOOL CFX_BidiChar::EndChar() {
-  m_iLastBidi = m_iCurBidi;
-  m_iLastStart = m_iCurStart;
-  m_iCurStart = m_iCurCount;
-  m_iLastCount = m_iCurCount - m_iLastStart;
-  return m_iLastCount > 0;
-}
-int32_t CFX_BidiChar::GetBidiInfo(int32_t& iStart, int32_t& iCount) {
-  iStart = m_iLastStart;
-  iCount = m_iLastCount;
-  return m_iLastBidi;
-}
-void CFX_BidiChar::Reset() {
-  m_iCurStart = 0;
-  m_iCurCount = 0;
-  m_iCurBidi = 0;
-  m_iLastBidi = 0;
-  m_iLastStart = 0;
-  m_iLastCount = 0;
-}
diff --git a/core/src/fxcrt/fx_arabic.h b/core/src/fxcrt/fx_arabic.h
deleted file mode 100644
index 0230d40579..0000000000
--- a/core/src/fxcrt/fx_arabic.h
+++ /dev/null
@@ -1,33 +0,0 @@
-// Copyright 2014 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.
-
-// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-
-#ifndef CORE_SRC_FXCRT_FX_ARABIC_H_
-#define CORE_SRC_FXCRT_FX_ARABIC_H_
-
-#include "../../include/fxcrt/fx_arb.h"
-
-class CFX_BidiChar final : public IFX_BidiChar {
- public:
-  CFX_BidiChar();
-  ~CFX_BidiChar() override {}
-
-  void SetPolicy(FX_BOOL bSeparateNeutral = TRUE) override;
-  FX_BOOL AppendChar(FX_WCHAR wch) override;
-  FX_BOOL EndChar() override;
-  int32_t GetBidiInfo(int32_t& iStart, int32_t& iCount) override;
-  void Reset() override;
-
- private:
-  FX_BOOL m_bSeparateNeutral;
-  int32_t m_iCurStart;
-  int32_t m_iCurCount;
-  int32_t m_iCurBidi;
-  int32_t m_iLastBidi;
-  int32_t m_iLastStart;
-  int32_t m_iLastCount;
-};
-
-#endif  // CORE_SRC_FXCRT_FX_ARABIC_H_
diff --git a/core/src/fxcrt/fx_bidi.cpp b/core/src/fxcrt/fx_bidi.cpp
new file mode 100644
index 0000000000..0310fa0e94
--- /dev/null
+++ b/core/src/fxcrt/fx_bidi.cpp
@@ -0,0 +1,66 @@
+// Copyright 2014 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.
+
+// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
+
+#include "../../include/fxcrt/fx_bidi.h"
+#include "../../include/fxcrt/fx_ucd.h"
+
+CFX_BidiChar::CFX_BidiChar()
+    : m_iCurStart(0),
+      m_iCurCount(0),
+      m_CurBidi(NEUTRAL),
+      m_iLastStart(0),
+      m_iLastCount(0),
+      m_LastBidi(NEUTRAL) {
+}
+
+CFX_BidiChar::~CFX_BidiChar() {
+}
+
+bool CFX_BidiChar::AppendChar(FX_WCHAR wch) {
+  FX_DWORD dwProps = FX_GetUnicodeProperties(wch);
+  int32_t iBidiCls = (dwProps & FX_BIDICLASSBITSMASK) >> FX_BIDICLASSBITS;
+  Direction bidi = NEUTRAL;
+  switch (iBidiCls) {
+    case FX_BIDICLASS_L:
+    case FX_BIDICLASS_AN:
+    case FX_BIDICLASS_EN:
+      bidi = LEFT;
+      break;
+    case FX_BIDICLASS_R:
+    case FX_BIDICLASS_AL:
+      bidi = RIGHT;
+      break;
+  }
+
+  bool bRet = (bidi != m_CurBidi);
+  if (bRet) {
+    SaveCurrentStateToLastState();
+    m_CurBidi = bidi;
+  }
+  m_iCurCount++;
+  return bRet;
+}
+
+bool CFX_BidiChar::EndChar() {
+  SaveCurrentStateToLastState();
+  return m_iLastCount > 0;
+}
+
+CFX_BidiChar::Direction CFX_BidiChar::GetBidiInfo(int32_t* iStart,
+                                                  int32_t* iCount) const {
+  if (iStart)
+    *iStart = m_iLastStart;
+  if (iCount)
+    *iCount = m_iLastCount;
+  return m_LastBidi;
+}
+
+void CFX_BidiChar::SaveCurrentStateToLastState() {
+  m_LastBidi = m_CurBidi;
+  m_iLastStart = m_iCurStart;
+  m_iCurStart = m_iCurCount;
+  m_iLastCount = m_iCurCount - m_iLastStart;
+}
diff --git a/core/src/fxcrt/fx_bidi_unittest.cpp b/core/src/fxcrt/fx_bidi_unittest.cpp
new file mode 100644
index 0000000000..c629cbbdc6
--- /dev/null
+++ b/core/src/fxcrt/fx_bidi_unittest.cpp
@@ -0,0 +1,136 @@
+// Copyright 2015 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 "testing/gtest/include/gtest/gtest.h"
+#include "../../include/fxcrt/fx_bidi.h"
+
+namespace {
+
+const FX_WCHAR kNeutralChar = 32;
+const FX_WCHAR kLeftChar = 65;
+const FX_WCHAR kRightChar = 1424;
+
+}  // namespace
+
+TEST(fxcrt, BidiCharEmpty) {
+  int32_t start = -1;
+  int32_t count = -1;
+  CFX_BidiChar bidi;
+  CFX_BidiChar::Direction dir = bidi.GetBidiInfo(nullptr, nullptr);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, dir);
+
+  dir = bidi.GetBidiInfo(&start, nullptr);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, dir);
+  EXPECT_EQ(0, start);
+
+  dir = bidi.GetBidiInfo(nullptr, &count);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, dir);
+  EXPECT_EQ(0, count);
+
+  start = -1;
+  count = -1;
+  dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, dir);
+  EXPECT_EQ(0, start);
+  EXPECT_EQ(0, count);
+
+  EXPECT_FALSE(bidi.EndChar());
+}
+
+TEST(fxcrt, BidiCharLeft) {
+  int32_t start = -1;
+  int32_t count = -1;
+  CFX_BidiChar bidi;
+
+  EXPECT_TRUE(bidi.AppendChar(kLeftChar));
+  CFX_BidiChar::Direction dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(0, start);
+  EXPECT_EQ(0, count);
+
+  EXPECT_FALSE(bidi.AppendChar(kLeftChar));
+  EXPECT_FALSE(bidi.AppendChar(kLeftChar));
+
+  dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, dir);
+  EXPECT_EQ(0, start);
+  EXPECT_EQ(0, count);
+
+  EXPECT_TRUE(bidi.EndChar());
+  dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(CFX_BidiChar::LEFT, dir);
+  EXPECT_EQ(0, start);
+  EXPECT_EQ(3, count);
+
+  EXPECT_FALSE(bidi.EndChar());
+}
+
+TEST(fxcrt, BidiCharLeftNeutralRight) {
+  int32_t start = -1;
+  int32_t count = -1;
+  CFX_BidiChar bidi;
+
+  EXPECT_TRUE(bidi.AppendChar(kLeftChar));
+  CFX_BidiChar::Direction dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(0, start);
+  EXPECT_EQ(0, count);
+
+  EXPECT_FALSE(bidi.AppendChar(kLeftChar));
+  EXPECT_FALSE(bidi.AppendChar(kLeftChar));
+  EXPECT_TRUE(bidi.AppendChar(kNeutralChar));
+  dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(0, start);
+  EXPECT_EQ(3, count);
+
+  EXPECT_FALSE(bidi.AppendChar(kNeutralChar));
+  EXPECT_FALSE(bidi.AppendChar(kNeutralChar));
+  EXPECT_FALSE(bidi.AppendChar(kNeutralChar));
+  EXPECT_TRUE(bidi.AppendChar(kRightChar));
+  dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, dir);
+  EXPECT_EQ(3, start);
+  EXPECT_EQ(4, count);
+
+  EXPECT_TRUE(bidi.EndChar());
+  dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(CFX_BidiChar::RIGHT, dir);
+  EXPECT_EQ(7, start);
+  EXPECT_EQ(1, count);
+
+  EXPECT_FALSE(bidi.EndChar());
+}
+
+TEST(fxcrt, BidiCharLeftRightLeft) {
+  int32_t start = -1;
+  int32_t count = -1;
+  CFX_BidiChar bidi;
+
+  EXPECT_TRUE(bidi.AppendChar(kLeftChar));
+  CFX_BidiChar::Direction dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(0, start);
+  EXPECT_EQ(0, count);
+
+  EXPECT_FALSE(bidi.AppendChar(kLeftChar));
+  EXPECT_FALSE(bidi.AppendChar(kLeftChar));
+  EXPECT_TRUE(bidi.AppendChar(kRightChar));
+  dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(0, start);
+  EXPECT_EQ(3, count);
+
+  EXPECT_FALSE(bidi.AppendChar(kRightChar));
+  EXPECT_FALSE(bidi.AppendChar(kRightChar));
+  EXPECT_FALSE(bidi.AppendChar(kRightChar));
+  EXPECT_TRUE(bidi.AppendChar(kLeftChar));
+  dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(CFX_BidiChar::RIGHT, dir);
+  EXPECT_EQ(3, start);
+  EXPECT_EQ(4, count);
+
+  EXPECT_TRUE(bidi.EndChar());
+  dir = bidi.GetBidiInfo(&start, &count);
+  EXPECT_EQ(CFX_BidiChar::LEFT, dir);
+  EXPECT_EQ(7, start);
+  EXPECT_EQ(1, count);
+
+  EXPECT_FALSE(bidi.EndChar());
+}
diff --git a/core/src/fxcrt/fx_ucddata.cpp b/core/src/fxcrt/fx_ucddata.cpp
index 8b35f31e2b..13c5da1419 100644
--- a/core/src/fxcrt/fx_ucddata.cpp
+++ b/core/src/fxcrt/fx_ucddata.cpp
@@ -4,9 +4,10 @@
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
-#include "../../include/fxcrt/fx_system.h"
+#include "../../include/fxcrt/fx_basic.h"
+#include "../../include/fxcrt/fx_ucd.h"
 
-extern const FX_DWORD gs_FX_TextLayout_CodeProperties[65536] = {
+const FX_DWORD kTextLayoutCodeProperties[] = {
     0xfffe9a93, 0xfffe9a93, 0xfffe9a93, 0xfffe9a93, 0xfffe9a93, 0xfffe9a93,
     0xfffe9a93, 0xfffe9a93, 0xfffe9a93, 0xfffe8ae5, 0xfffe9b5c, 0xfffe9ada,
     0xfffe9b1a, 0xfffe9b5b, 0xfffe9a93, 0xfffe9a93, 0xfffe9a93, 0xfffe9a93,
@@ -10931,7 +10932,11 @@ extern const FX_DWORD gs_FX_TextLayout_CodeProperties[65536] = {
     0xfffe02a4, 0xfffe02a4, 0xfffe02a4, 0xfffe3013, 0xfffe3013, 0xfffe3013,
     0xfffe3011, 0xfffe3022, 0xfffe1aa4, 0xfffe1aa4,
 };
-extern const FX_WCHAR gs_FX_TextLayout_VerticalMirror[64] = {
+
+const size_t kTextLayoutCodePropertiesSize =
+    FX_ArraySize(kTextLayoutCodeProperties);
+
+const FX_WCHAR kFXTextLayoutVerticalMirror[] = {
     0xFE33, 0xFE32, 0xFE31, 0xFE41, 0xFE42, 0xFE43, 0xFE44, 0xFE3F,
     0xFE40, 0xFE3D, 0xFE3E, 0xFE41, 0xFE42, 0xFE43, 0xFE44, 0xFE3B,
     0xFE3C, 0xFE39, 0xFE3A, 0xFE34, 0xFE35, 0xFE36, 0xFE37, 0xFE38,
@@ -10941,7 +10946,10 @@ extern const FX_WCHAR gs_FX_TextLayout_VerticalMirror[64] = {
     0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF,
     0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF,
 };
-extern const FX_WCHAR gs_FX_TextLayout_BidiMirror[512] = {
+const size_t kFXTextLayoutVerticalMirrorSize =
+    FX_ArraySize(kFXTextLayoutVerticalMirror);
+
+const FX_WCHAR kFXTextLayoutBidiMirror[] = {
     0x0029, 0x0028, 0x003E, 0x003C, 0x005D, 0x005B, 0x007D, 0x007B, 0x00BB,
     0x00AB, 0x0F3B, 0x0F3A, 0x0F3D, 0x0F3C, 0x169C, 0x169B, 0x2019, 0x2018,
     0x201D, 0x201C, 0x203A, 0x2039, 0x2046, 0x2045, 0x207E, 0x207D, 0x208E,
@@ -11000,3 +11008,6 @@ extern const FX_WCHAR gs_FX_TextLayout_BidiMirror[512] = {
     0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF,
     0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF, 0xFEFF,
 };
+
+const size_t kFXTextLayoutBidiMirrorSize =
+    FX_ArraySize(kFXTextLayoutBidiMirror);
diff --git a/core/src/fxcrt/fx_unicode.cpp b/core/src/fxcrt/fx_unicode.cpp
index f05aeb5efb..c7ab618200 100644
--- a/core/src/fxcrt/fx_unicode.cpp
+++ b/core/src/fxcrt/fx_unicode.cpp
@@ -6,58 +6,29 @@
 
 #include "../../include/fxcrt/fx_ucd.h"
 
-extern const FX_DWORD gs_FX_TextLayout_CodeProperties[65536];
-extern const FX_WCHAR gs_FX_TextLayout_VerticalMirror[64];
-extern const FX_WCHAR gs_FX_TextLayout_BidiMirror[512];
 FX_DWORD FX_GetUnicodeProperties(FX_WCHAR wch) {
-  return gs_FX_TextLayout_CodeProperties[(FX_WORD)wch];
-}
-FX_BOOL FX_IsCtrlCode(FX_WCHAR ch) {
-  FX_DWORD dwRet =
-      (gs_FX_TextLayout_CodeProperties[(FX_WORD)ch] & FX_CHARTYPEBITSMASK);
-  return dwRet == FX_CHARTYPE_Tab || dwRet == FX_CHARTYPE_Control;
-}
-FX_BOOL FX_IsRotationCode(FX_WCHAR ch) {
-  return (gs_FX_TextLayout_CodeProperties[(FX_WORD)ch] & 0x8000) != 0;
-}
-FX_BOOL FX_IsCombinationChar(FX_WCHAR wch) {
-  FX_DWORD dwProps =
-      (gs_FX_TextLayout_CodeProperties[(FX_WORD)wch] & FX_CHARTYPEBITSMASK);
-  return dwProps == FX_CHARTYPE_Combination;
-}
-FX_BOOL FX_IsBidiChar(FX_WCHAR wch) {
-  FX_DWORD dwProps = gs_FX_TextLayout_CodeProperties[(FX_WORD)wch];
-  int32_t iBidiCls = (dwProps & FX_BIDICLASSBITSMASK) >> FX_BIDICLASSBITS;
-  return (FX_BIDICLASS_R == iBidiCls || FX_BIDICLASS_AL == iBidiCls);
+  size_t idx = static_cast<size_t>(wch);
+  if (idx < kTextLayoutCodePropertiesSize)
+    return kTextLayoutCodeProperties[(FX_WORD)wch];
+  return 0;
 }
+
 FX_WCHAR FX_GetMirrorChar(FX_WCHAR wch, FX_BOOL bRTL, FX_BOOL bVertical) {
-  FX_DWORD dwProps = gs_FX_TextLayout_CodeProperties[(FX_WORD)wch];
+  FX_DWORD dwProps = FX_GetUnicodeProperties(wch);
   FX_DWORD dwTemp = (dwProps & 0xFF800000);
   if (bRTL && dwTemp < 0xFF800000) {
-    wch = gs_FX_TextLayout_BidiMirror[dwTemp >> 23];
-    dwProps = gs_FX_TextLayout_CodeProperties[(FX_WORD)wch];
-  }
-  if (bVertical) {
-    dwTemp = (dwProps & 0x007E0000);
-    if (dwTemp < 0x007E0000) {
-      wch = gs_FX_TextLayout_VerticalMirror[dwTemp >> 17];
+    size_t idx = dwTemp >> 23;
+    if (idx < kFXTextLayoutBidiMirrorSize) {
+      wch = kFXTextLayoutBidiMirror[idx];
+      dwProps = FX_GetUnicodeProperties(wch);
     }
   }
-  return wch;
-}
-FX_WCHAR FX_GetMirrorChar(FX_WCHAR wch,
-                          FX_DWORD dwProps,
-                          FX_BOOL bRTL,
-                          FX_BOOL bVertical) {
-  FX_DWORD dwTemp = (dwProps & 0xFF800000);
-  if (bRTL && dwTemp < 0xFF800000) {
-    wch = gs_FX_TextLayout_BidiMirror[dwTemp >> 23];
-    dwProps = gs_FX_TextLayout_CodeProperties[(FX_WORD)wch];
-  }
   if (bVertical) {
     dwTemp = (dwProps & 0x007E0000);
     if (dwTemp < 0x007E0000) {
-      wch = gs_FX_TextLayout_VerticalMirror[dwTemp >> 17];
+      size_t idx = dwTemp >> 17;
+      if (idx < kFXTextLayoutVerticalMirrorSize)
+        wch = kFXTextLayoutVerticalMirror[idx];
     }
   }
   return wch;