Make fx_bidi sane.

Replace array of heterogenous ints with array of struct.
Create a class for traversing a string.
Flip array when R2L and process with forward iterator always.

R=thestig@chromium.org

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

2 files changed, 342 insertions, 98 deletions

diff --git a/core/src/fxcrt/fx_bidi.cpp b/core/src/fxcrt/fx_bidi.cpp
index d5d87a345b..a7a3ecb295 100644
--- a/core/src/fxcrt/fx_bidi.cpp
+++ b/core/src/fxcrt/fx_bidi.cpp
@@ -7,60 +7,75 @@
 #include "core/include/fxcrt/fx_bidi.h"
 #include "core/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) {
-}
+#include <algorithm>
 
-CFX_BidiChar::~CFX_BidiChar() {
-}
+CFX_BidiChar::CFX_BidiChar()
+    : m_CurrentSegment({0, 0, NEUTRAL}), m_LastSegment({0, 0, NEUTRAL}) {}
 
 bool CFX_BidiChar::AppendChar(FX_WCHAR wch) {
   FX_DWORD dwProps = FX_GetUnicodeProperties(wch);
   int32_t iBidiCls = (dwProps & FX_BIDICLASSBITSMASK) >> FX_BIDICLASSBITS;
-  Direction bidi = NEUTRAL;
+  Direction direction = NEUTRAL;
   switch (iBidiCls) {
     case FX_BIDICLASS_L:
     case FX_BIDICLASS_AN:
     case FX_BIDICLASS_EN:
-      bidi = LEFT;
+      direction = LEFT;
       break;
     case FX_BIDICLASS_R:
     case FX_BIDICLASS_AL:
-      bidi = RIGHT;
+      direction = RIGHT;
       break;
   }
 
-  bool bRet = (bidi != m_CurBidi);
-  if (bRet) {
-    SaveCurrentStateToLastState();
-    m_CurBidi = bidi;
-  }
-  m_iCurCount++;
-  return bRet;
+  bool bChangeDirection = (direction != m_CurrentSegment.direction);
+  if (bChangeDirection)
+    StartNewSegment(direction);
+
+  m_CurrentSegment.count++;
+  return bChangeDirection;
 }
 
 bool CFX_BidiChar::EndChar() {
-  SaveCurrentStateToLastState();
-  return m_iLastCount > 0;
+  StartNewSegment(NEUTRAL);
+  return m_LastSegment.count > 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::StartNewSegment(CFX_BidiChar::Direction direction) {
+  m_LastSegment = m_CurrentSegment;
+  m_CurrentSegment.start += m_CurrentSegment.count;
+  m_CurrentSegment.count = 0;
+  m_CurrentSegment.direction = direction;
 }
 
-void CFX_BidiChar::SaveCurrentStateToLastState() {
-  m_LastBidi = m_CurBidi;
-  m_iLastStart = m_iCurStart;
-  m_iCurStart = m_iCurCount;
-  m_iLastCount = m_iCurCount - m_iLastStart;
+CFX_BidiString::CFX_BidiString(const CFX_WideString& str)
+    : m_Str(str),
+      m_pBidiChar(new CFX_BidiChar),
+      m_eOverallDirection(CFX_BidiChar::LEFT) {
+  for (int i = 0; i < m_Str.GetLength(); ++i) {
+    if (m_pBidiChar->AppendChar(m_Str.GetAt(i)))
+      m_Order.push_back(m_pBidiChar->GetSegmentInfo());
+  }
+  if (m_pBidiChar->EndChar())
+    m_Order.push_back(m_pBidiChar->GetSegmentInfo());
+
+  size_t nR2L = std::count_if(m_Order.begin(), m_Order.end(),
+                              [](const CFX_BidiChar::Segment& seg) {
+                                return seg.direction == CFX_BidiChar::RIGHT;
+                              });
+
+  size_t nL2R = std::count_if(m_Order.begin(), m_Order.end(),
+                              [](const CFX_BidiChar::Segment& seg) {
+                                return seg.direction == CFX_BidiChar::LEFT;
+                              });
+
+  if (nR2L > 0 && nR2L >= nL2R)
+    SetOverallDirectionRight();
+}
+
+void CFX_BidiString::SetOverallDirectionRight() {
+  if (m_eOverallDirection != CFX_BidiChar::RIGHT) {
+    std::reverse(m_Order.begin(), m_Order.end());
+    m_eOverallDirection = CFX_BidiChar::RIGHT;
+  }
 }
diff --git a/core/src/fxcrt/fx_bidi_unittest.cpp b/core/src/fxcrt/fx_bidi_unittest.cpp
index 8b5b6df108..a3c148702b 100644
--- a/core/src/fxcrt/fx_bidi_unittest.cpp
+++ b/core/src/fxcrt/fx_bidi_unittest.cpp
@@ -14,123 +14,352 @@ 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);
+  CFX_BidiChar::Segment info;
 
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, info.direction);
+  EXPECT_EQ(0, info.start);
+  EXPECT_EQ(0, info.count);
   EXPECT_FALSE(bidi.EndChar());
 }
 
 TEST(fxcrt, BidiCharLeft) {
-  int32_t start = -1;
-  int32_t count = -1;
   CFX_BidiChar bidi;
+  CFX_BidiChar::Segment info;
 
   EXPECT_TRUE(bidi.AppendChar(kLeftChar));
-  CFX_BidiChar::Direction dir = bidi.GetBidiInfo(&start, &count);
-  EXPECT_EQ(0, start);
-  EXPECT_EQ(0, count);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(0, info.start);
+  EXPECT_EQ(0, info.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);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, info.direction);
+  EXPECT_EQ(0, info.start);
+  EXPECT_EQ(0, info.count);
 
   EXPECT_TRUE(bidi.EndChar());
-  dir = bidi.GetBidiInfo(&start, &count);
-  EXPECT_EQ(CFX_BidiChar::LEFT, dir);
-  EXPECT_EQ(0, start);
-  EXPECT_EQ(3, count);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(CFX_BidiChar::LEFT, info.direction);
+  EXPECT_EQ(0, info.start);
+  EXPECT_EQ(3, info.count);
 
   EXPECT_FALSE(bidi.EndChar());
 }
 
 TEST(fxcrt, BidiCharLeftNeutralRight) {
-  int32_t start = -1;
-  int32_t count = -1;
   CFX_BidiChar bidi;
+  CFX_BidiChar::Segment info;
 
   EXPECT_TRUE(bidi.AppendChar(kLeftChar));
-  CFX_BidiChar::Direction dir = bidi.GetBidiInfo(&start, &count);
-  EXPECT_EQ(0, start);
-  EXPECT_EQ(0, count);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(0, info.start);
+  EXPECT_EQ(0, info.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);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(0, info.start);
+  EXPECT_EQ(3, info.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);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, info.direction);
+  EXPECT_EQ(3, info.start);
+  EXPECT_EQ(4, info.count);
 
   EXPECT_TRUE(bidi.EndChar());
-  dir = bidi.GetBidiInfo(&start, &count);
-  EXPECT_EQ(CFX_BidiChar::RIGHT, dir);
-  EXPECT_EQ(7, start);
-  EXPECT_EQ(1, count);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(CFX_BidiChar::RIGHT, info.direction);
+  EXPECT_EQ(7, info.start);
+  EXPECT_EQ(1, info.count);
 
   EXPECT_FALSE(bidi.EndChar());
 }
 
 TEST(fxcrt, BidiCharLeftRightLeft) {
-  int32_t start = -1;
-  int32_t count = -1;
   CFX_BidiChar bidi;
+  CFX_BidiChar::Segment info;
 
   EXPECT_TRUE(bidi.AppendChar(kLeftChar));
-  CFX_BidiChar::Direction dir = bidi.GetBidiInfo(&start, &count);
-  EXPECT_EQ(0, start);
-  EXPECT_EQ(0, count);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(0, info.start);
+  EXPECT_EQ(0, info.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);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(0, info.start);
+  EXPECT_EQ(3, info.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);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(CFX_BidiChar::RIGHT, info.direction);
+  EXPECT_EQ(3, info.start);
+  EXPECT_EQ(4, info.count);
 
   EXPECT_TRUE(bidi.EndChar());
-  dir = bidi.GetBidiInfo(&start, &count);
-  EXPECT_EQ(CFX_BidiChar::LEFT, dir);
-  EXPECT_EQ(7, start);
-  EXPECT_EQ(1, count);
+  info = bidi.GetSegmentInfo();
+  EXPECT_EQ(CFX_BidiChar::LEFT, info.direction);
+  EXPECT_EQ(7, info.start);
+  EXPECT_EQ(1, info.count);
 
   EXPECT_FALSE(bidi.EndChar());
 }
+
+TEST(fxcrt, BidiStringEmpty) {
+  CFX_BidiString bidi(L"");
+  EXPECT_EQ(CFX_BidiChar::LEFT, bidi.OverallDirection());
+  EXPECT_TRUE(bidi.begin() == bidi.end());
+}
+
+TEST(fxcrt, BidiStringAllNeutral) {
+  {
+    const FX_WCHAR str[] = {kNeutralChar, 0};
+    CFX_BidiString bidi(str);
+    EXPECT_EQ(CFX_BidiChar::LEFT, bidi.OverallDirection());
+
+    auto it = bidi.begin();
+    ASSERT_FALSE(it == bidi.end());
+    EXPECT_EQ(0, it->start);
+    EXPECT_EQ(1, it->count);
+    EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+    ++it;
+    EXPECT_TRUE(it == bidi.end());
+  }
+  {
+    const FX_WCHAR str[] = {kNeutralChar, kNeutralChar, kNeutralChar, 0};
+    CFX_BidiString bidi(str);
+    EXPECT_EQ(CFX_BidiChar::LEFT, bidi.OverallDirection());
+
+    auto it = bidi.begin();
+    ASSERT_FALSE(it == bidi.end());
+    EXPECT_EQ(0, it->start);
+    EXPECT_EQ(3, it->count);
+    EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+    ++it;
+    EXPECT_TRUE(it == bidi.end());
+  }
+}
+
+TEST(fxcrt, BidiStringAllLeft) {
+  {
+    const FX_WCHAR str[] = {kLeftChar, 0};
+    CFX_BidiString bidi(str);
+    EXPECT_EQ(CFX_BidiChar::LEFT, bidi.OverallDirection());
+
+    auto it = bidi.begin();
+    ASSERT_FALSE(it == bidi.end());
+    EXPECT_EQ(0, it->start);
+    EXPECT_EQ(0, it->count);
+    EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+    ASSERT_FALSE(it == bidi.end());
+
+    ++it;
+    EXPECT_EQ(0, it->start);
+    EXPECT_EQ(1, it->count);
+    EXPECT_EQ(CFX_BidiChar::LEFT, it->direction);
+    ASSERT_FALSE(it == bidi.end());
+
+    ++it;
+    EXPECT_TRUE(it == bidi.end());
+  }
+  {
+    const FX_WCHAR str[] = {kLeftChar, kLeftChar, kLeftChar, 0};
+    CFX_BidiString bidi(str);
+    EXPECT_EQ(CFX_BidiChar::LEFT, bidi.OverallDirection());
+
+    auto it = bidi.begin();
+    ASSERT_FALSE(it == bidi.end());
+    EXPECT_EQ(0, it->start);
+    EXPECT_EQ(0, it->count);
+    EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+    ASSERT_FALSE(it == bidi.end());
+
+    ++it;
+    EXPECT_EQ(0, it->start);
+    EXPECT_EQ(3, it->count);
+    EXPECT_EQ(CFX_BidiChar::LEFT, it->direction);
+    ASSERT_FALSE(it == bidi.end());
+
+    ++it;
+    EXPECT_TRUE(it == bidi.end());
+  }
+}
+
+TEST(fxcrt, BidiStringAllRight) {
+  {
+    const FX_WCHAR str[] = {kRightChar, 0};
+    CFX_BidiString bidi(str);
+    EXPECT_EQ(CFX_BidiChar::RIGHT, bidi.OverallDirection());
+
+    auto it = bidi.begin();
+    EXPECT_EQ(0, it->start);
+    EXPECT_EQ(1, it->count);
+    EXPECT_EQ(CFX_BidiChar::RIGHT, it->direction);
+    ASSERT_FALSE(it == bidi.end());
+
+    ++it;
+    ASSERT_FALSE(it == bidi.end());
+    EXPECT_EQ(0, it->start);
+    EXPECT_EQ(0, it->count);
+    EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+    ASSERT_FALSE(it == bidi.end());
+
+    ++it;
+    EXPECT_TRUE(it == bidi.end());
+  }
+  {
+    const FX_WCHAR str[] = {kRightChar, kRightChar, kRightChar, 0};
+    CFX_BidiString bidi(str);
+    EXPECT_EQ(CFX_BidiChar::RIGHT, bidi.OverallDirection());
+
+    auto it = bidi.begin();
+    EXPECT_EQ(0, it->start);
+    EXPECT_EQ(3, it->count);
+    EXPECT_EQ(CFX_BidiChar::RIGHT, it->direction);
+    ASSERT_FALSE(it == bidi.end());
+
+    ++it;
+    ASSERT_FALSE(it == bidi.end());
+    EXPECT_EQ(0, it->start);
+    EXPECT_EQ(0, it->count);
+    EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+    ASSERT_FALSE(it == bidi.end());
+
+    ++it;
+    EXPECT_TRUE(it == bidi.end());
+  }
+}
+
+TEST(fxcrt, BidiStringLeftNeutralLeftRight) {
+  const FX_WCHAR str[] = {kLeftChar, kNeutralChar, kLeftChar, kRightChar, 0};
+  CFX_BidiString bidi(str);
+  EXPECT_EQ(CFX_BidiChar::LEFT, bidi.OverallDirection());
+
+  auto it = bidi.begin();
+  ASSERT_FALSE(it == bidi.end());
+  EXPECT_EQ(0, it->start);
+  EXPECT_EQ(0, it->count);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  EXPECT_EQ(0, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::LEFT, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  EXPECT_EQ(1, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  EXPECT_EQ(2, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::LEFT, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  EXPECT_EQ(3, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::RIGHT, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  EXPECT_TRUE(it == bidi.end());
+}
+
+TEST(fxcrt, BidiStringRightNeutralLeftRight) {
+  const FX_WCHAR str[] = {kRightChar, kNeutralChar, kLeftChar, kRightChar, 0};
+  CFX_BidiString bidi(str);
+  EXPECT_EQ(CFX_BidiChar::RIGHT, bidi.OverallDirection());
+
+  auto it = bidi.begin();
+  EXPECT_EQ(3, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::RIGHT, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  EXPECT_EQ(2, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::LEFT, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  EXPECT_EQ(1, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  EXPECT_EQ(0, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::RIGHT, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  ASSERT_FALSE(it == bidi.end());
+  EXPECT_EQ(0, it->start);
+  EXPECT_EQ(0, it->count);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  EXPECT_TRUE(it == bidi.end());
+}
+
+TEST(fxcrt, BidiStringReverse) {
+  const FX_WCHAR str[] = {kLeftChar, kNeutralChar, kRightChar, kLeftChar, 0};
+  CFX_BidiString bidi(str);
+  EXPECT_EQ(CFX_BidiChar::LEFT, bidi.OverallDirection());
+  bidi.SetOverallDirectionRight();
+
+  auto it = bidi.begin();
+  ASSERT_FALSE(it == bidi.end());
+  EXPECT_EQ(3, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::LEFT, it->direction);
+
+  ++it;
+  ASSERT_FALSE(it == bidi.end());
+  EXPECT_EQ(2, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::RIGHT, it->direction);
+  ASSERT_FALSE(it == bidi.end());
+
+  ++it;
+  ASSERT_FALSE(it == bidi.end());
+  EXPECT_EQ(1, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+
+  ++it;
+  ASSERT_FALSE(it == bidi.end());
+  EXPECT_EQ(0, it->start);
+  EXPECT_EQ(1, it->count);
+  EXPECT_EQ(CFX_BidiChar::LEFT, it->direction);
+
+  ++it;
+  ASSERT_FALSE(it == bidi.end());
+  EXPECT_EQ(0, it->start);
+  EXPECT_EQ(0, it->count);
+  EXPECT_EQ(CFX_BidiChar::NEUTRAL, it->direction);
+
+  ++it;
+  EXPECT_TRUE(it == bidi.end());
+}