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 .

5 files changed, 425 insertions, 281 deletions

diff --git a/core/include/fxcrt/fx_bidi.h b/core/include/fxcrt/fx_bidi.h
index a55ce6cfd2..ecf888b306 100644
--- a/core/include/fxcrt/fx_bidi.h
+++ b/core/include/fxcrt/fx_bidi.h
@@ -7,15 +7,23 @@
 #ifndef CORE_INCLUDE_FXCRT_FX_BIDI_H_
 #define CORE_INCLUDE_FXCRT_FX_BIDI_H_
 
+#include <memory>
+#include <vector>
+
+#include "fx_string.h"
 #include "fx_system.h"
 
 // Processes characters and group them into segments based on text direction.
 class CFX_BidiChar {
  public:
   enum Direction { NEUTRAL, LEFT, RIGHT };
+  struct Segment {
+    int32_t start;        // Start position.
+    int32_t count;        // Character count.
+    Direction direction;  // Segment direction.
+  };
 
   CFX_BidiChar();
-  ~CFX_BidiChar();
 
   // Append a character and classify it as left, right, or neutral.
   // Returns true if the character has a different direction than the
@@ -27,33 +35,39 @@ class CFX_BidiChar {
   // Returns true if there is still a segment to process.
   bool EndChar();
 
-  // Get information about the segment to process.
-  // The segment's start position and character count is returned in |iStart|
-  // and |iCount|, respectively. Pass in null pointers if the information is
-  // not needed.
-  // Returns the segment direction.
-  Direction GetBidiInfo(int32_t* iStart, int32_t* iCount) const;
+  // Call after a change in direction is indicated by the above to get
+  // information about the segment to process.
+  Segment GetSegmentInfo() const { return m_LastSegment; }
 
  private:
-  void SaveCurrentStateToLastState();
+  void StartNewSegment(CFX_BidiChar::Direction direction);
 
-  // Position of the current segment.
-  int32_t m_iCurStart;
+  Segment m_CurrentSegment;
+  Segment m_LastSegment;
+};
 
-  // Number of characters in the current segment.
-  int32_t m_iCurCount;
+class CFX_BidiString {
+ public:
+  using const_iterator = std::vector<CFX_BidiChar::Segment>::const_iterator;
+  explicit CFX_BidiString(const CFX_WideString& str);
 
-  // Direction of the current segment.
-  Direction m_CurBidi;
+  // Overall direction is always LEFT or RIGHT, never NEUTRAL.
+  CFX_BidiChar::Direction OverallDirection() const {
+    return m_eOverallDirection;
+  }
 
-  // Number of characters in the last segment.
-  int32_t m_iLastStart;
+  // Force the overall direction to be R2L regardless of what was detected.
+  void SetOverallDirectionRight();
 
-  // Number of characters in the last segment.
-  int32_t m_iLastCount;
+  FX_WCHAR CharAt(size_t x) const { return m_Str[x]; }
+  const_iterator begin() const { return m_Order.begin(); }
+  const_iterator end() const { return m_Order.end(); }
 
-  // Direction of the last segment.
-  Direction m_LastBidi;
+ private:
+  const CFX_WideString m_Str;
+  std::unique_ptr<CFX_BidiChar> m_pBidiChar;
+  std::vector<CFX_BidiChar::Segment> m_Order;
+  CFX_BidiChar::Direction m_eOverallDirection;
 };
 
 #endif  // CORE_INCLUDE_FXCRT_FX_BIDI_H_
diff --git a/core/src/fpdftext/fpdf_text_int.cpp b/core/src/fpdftext/fpdf_text_int.cpp
index ede5f83616..22591651ab 100644
--- a/core/src/fpdftext/fpdf_text_int.cpp
+++ b/core/src/fpdftext/fpdf_text_int.cpp
@@ -914,24 +914,21 @@ int CPDF_TextPage::GetCharWidth(FX_DWORD charCode, CPDF_Font* pFont) const {
   return w;
 }
 void CPDF_TextPage::OnPiece(CFX_BidiChar* pBidi, CFX_WideString& str) {
-  int32_t start, count;
-  CFX_BidiChar::Direction ret = pBidi->GetBidiInfo(&start, &count);
-  if (ret == CFX_BidiChar::RIGHT) {
-    for (int i = start + count - 1; i >= start; i--) {
-      m_TextBuf.AppendChar(str.GetAt(i));
-      m_CharList.push_back(m_TempCharList[i]);
+  CFX_BidiChar::Segment seg = pBidi->GetSegmentInfo();
+  if (seg.direction == CFX_BidiChar::RIGHT) {
+    for (int i = seg.start + seg.count; i > seg.start; i--) {
+      m_TextBuf.AppendChar(str.GetAt(i - i));
+      m_CharList.push_back(m_TempCharList[i - 1]);
     }
   } else {
-    int end = start + count;
-    for (int i = start; i < end; i++) {
+    for (int i = seg.start; i < seg.start + seg.count; i++) {
       m_TextBuf.AppendChar(str.GetAt(i));
       m_CharList.push_back(m_TempCharList[i]);
     }
   }
 }
-void CPDF_TextPage::AddCharInfoByLRDirection(CFX_WideString& str, int i) {
-  PAGECHAR_INFO info = m_TempCharList[i];
-  FX_WCHAR wChar = str.GetAt(i);
+void CPDF_TextPage::AddCharInfoByLRDirection(FX_WCHAR wChar,
+                                             PAGECHAR_INFO info) {
   if (!IsControlChar(info)) {
     info.m_Index = m_TextBuf.GetLength();
     if (wChar >= 0xFB00 && wChar <= 0xFB06) {
@@ -957,11 +954,11 @@ void CPDF_TextPage::AddCharInfoByLRDirection(CFX_WideString& str, int i) {
   }
   m_CharList.push_back(info);
 }
-void CPDF_TextPage::AddCharInfoByRLDirection(CFX_WideString& str, int i) {
-  PAGECHAR_INFO info = m_TempCharList[i];
+void CPDF_TextPage::AddCharInfoByRLDirection(FX_WCHAR wChar,
+                                             PAGECHAR_INFO info) {
   if (!IsControlChar(info)) {
     info.m_Index = m_TextBuf.GetLength();
-    FX_WCHAR wChar = FX_GetMirrorChar(str.GetAt(i), TRUE, FALSE);
+    wChar = FX_GetMirrorChar(wChar, TRUE, FALSE);
     FX_WCHAR* pDst = NULL;
     FX_STRSIZE nCount = FX_Unicode_GetNormalization(wChar, pDst);
     if (nCount >= 1) {
@@ -984,140 +981,47 @@ void CPDF_TextPage::AddCharInfoByRLDirection(CFX_WideString& str, int i) {
   }
   m_CharList.push_back(info);
 }
+
 void CPDF_TextPage::CloseTempLine() {
-  if (m_TempCharList.empty()) {
+  if (m_TempCharList.empty())
     return;
-  }
-  std::unique_ptr<CFX_BidiChar> pBidiChar(new CFX_BidiChar);
+
   CFX_WideString str = m_TempTextBuf.GetWideString();
-  std::vector<FX_WORD> order;
-  FX_BOOL bR2L = FALSE;
-  int32_t start = 0, count = 0;
-  int nR2L = 0, nL2R = 0;
   FX_BOOL bPrevSpace = FALSE;
   for (int i = 0; i < str.GetLength(); i++) {
-    if (str.GetAt(i) == 32) {
-      if (bPrevSpace) {
-        m_TempTextBuf.Delete(i, 1);
-        m_TempCharList.erase(m_TempCharList.begin() + i);
-        str.Delete(i);
-        i--;
-        continue;
-      }
-      bPrevSpace = TRUE;
-    } else {
+    if (str.GetAt(i) != ' ') {
       bPrevSpace = FALSE;
+      continue;
     }
-    if (pBidiChar->AppendChar(str.GetAt(i))) {
-      CFX_BidiChar::Direction ret = pBidiChar->GetBidiInfo(&start, &count);
-      order.push_back(start);
-      order.push_back(count);
-      order.push_back(ret);
-      if (!bR2L) {
-        if (ret == CFX_BidiChar::RIGHT) {
-          nR2L++;
-        } else if (ret == CFX_BidiChar::LEFT) {
-          nL2R++;
-        }
-      }
-    }
-  }
-  if (pBidiChar->EndChar()) {
-    CFX_BidiChar::Direction ret = pBidiChar->GetBidiInfo(&start, &count);
-    order.push_back(start);
-    order.push_back(count);
-    order.push_back(ret);
-    if (!bR2L) {
-      if (ret == CFX_BidiChar::RIGHT) {
-        nR2L++;
-      } else if (ret == CFX_BidiChar::LEFT) {
-        nL2R++;
-      }
-    }
-  }
-  if (nR2L > 0 && nR2L >= nL2R) {
-    bR2L = TRUE;
-  }
-  if (m_parserflag == FPDFTEXT_RLTB || bR2L) {
-    int count = pdfium::CollectionSize<int>(order);
-    for (int i = count - 1; i > 0; i -= 3) {
-      int ret = order[i];
-      int count1 = order[i - 1];
-      int start = order[i - 2];
-      if (ret == 2 || ret == 0) {
-        for (int j = start + count1 - 1; j >= start; j--) {
-          AddCharInfoByRLDirection(str, j);
-        }
-      } else {
-        int j = i;
-        FX_BOOL bSymbol = FALSE;
-        while (j > 0 && order[j] != 2) {
-          bSymbol = !order[j];
-          j -= 3;
-        }
-        int end = start + count1;
-        int n = 0;
-        if (bSymbol) {
-          n = j + 6;
-        } else {
-          n = j + 3;
-        }
-        if (n >= i) {
-          for (int m = start; m < end; m++) {
-            AddCharInfoByLRDirection(str, m);
-          }
-        } else {
-          j = i;
-          i = n;
-          for (; n <= j; n += 3) {
-            int start = order[n - 2];
-            int count1 = order[n - 1];
-            int end = start + count1;
-            for (int m = start; m < end; m++) {
-              AddCharInfoByLRDirection(str, m);
-            }
-          }
-        }
-      }
-    }
-  } else {
-    int count = pdfium::CollectionSize<int>(order);
-    FX_BOOL bL2R = FALSE;
-    for (int i = 0; i < count; i += 3) {
-      int start = order[i];
-      int count1 = order[i + 1];
-      int ret = order[i + 2];
-      if (ret == 2 || (i == 0 && ret == 0 && !bL2R)) {
-        int j = i + 3;
-        while (bR2L && j < count) {
-          if (order[j + 2] == 1)
-            break;
-          j += 3;
-        }
-        if (j == 3) {
-          i = -3;
-          bL2R = TRUE;
-          continue;
-        }
-        int end = pdfium::CollectionSize<int>(m_TempCharList) - 1;
-        if (j < count) {
-          end = order[j] - 1;
-        }
-        i = j - 3;
-        for (int n = end; n >= start; n--) {
-          AddCharInfoByRLDirection(str, n);
-        }
-      } else {
-        int end = start + count1;
-        for (int n = start; n < end; n++) {
-          AddCharInfoByLRDirection(str, n);
-        }
-      }
+    if (bPrevSpace) {
+      m_TempTextBuf.Delete(i, 1);
+      m_TempCharList.erase(m_TempCharList.begin() + i);
+      str.Delete(i);
+      i--;
+    }
+    bPrevSpace = TRUE;
+  }
+  CFX_BidiString bidi(str);
+  if (m_parserflag == FPDFTEXT_RLTB)
+    bidi.SetOverallDirectionRight();
+  CFX_BidiChar::Direction eCurrentDirection = bidi.OverallDirection();
+  for (const auto& segment : bidi) {
+    if (segment.direction == CFX_BidiChar::RIGHT ||
+        (segment.direction == CFX_BidiChar::NEUTRAL &&
+         eCurrentDirection == CFX_BidiChar::RIGHT)) {
+      eCurrentDirection = CFX_BidiChar::RIGHT;
+      for (int m = segment.start + segment.count; m > segment.start; --m)
+        AddCharInfoByRLDirection(bidi.CharAt(m - 1), m_TempCharList[m - 1]);
+    } else {
+      eCurrentDirection = CFX_BidiChar::LEFT;
+      for (int m = segment.start; m < segment.start + segment.count; m++)
+        AddCharInfoByLRDirection(bidi.CharAt(m), m_TempCharList[m]);
     }
   }
   m_TempCharList.clear();
   m_TempTextBuf.Delete(0, m_TempTextBuf.GetLength());
 }
+
 void CPDF_TextPage::ProcessTextObject(CPDF_TextObject* pTextObj,
                                       const CFX_Matrix& formMatrix,
                                       FX_POSITION ObjPos) {
@@ -1360,15 +1264,13 @@ void CPDF_TextPage::SwapTempTextBuf(int32_t iCharListStartAppend,
     std::swap(pTempBuffer[i], pTempBuffer[j]);
   }
 }
+
 FX_BOOL CPDF_TextPage::IsRightToLeft(const CPDF_TextObject* pTextObj,
                                      const CPDF_Font* pFont,
                                      int nItems) const {
-  std::unique_ptr<CFX_BidiChar> pBidiChar(new CFX_BidiChar);
-  int32_t nR2L = 0;
-  int32_t nL2R = 0;
-  int32_t start = 0, count = 0;
-  CPDF_TextObjectItem item;
+  CFX_WideString str;
   for (int32_t i = 0; i < nItems; i++) {
+    CPDF_TextObjectItem item;
     pTextObj->GetItemInfo(i, &item);
     if (item.m_CharCode == (FX_DWORD)-1) {
       continue;
@@ -1378,28 +1280,12 @@ FX_BOOL CPDF_TextPage::IsRightToLeft(const CPDF_TextObject* pTextObj,
     if ((wstrItem.IsEmpty() || wChar == 0) && item.m_CharCode) {
       wChar = (FX_WCHAR)item.m_CharCode;
     }
-    if (!wChar) {
-      continue;
-    }
-    if (pBidiChar->AppendChar(wChar)) {
-      CFX_BidiChar::Direction ret = pBidiChar->GetBidiInfo(&start, &count);
-      if (ret == CFX_BidiChar::RIGHT) {
-        nR2L++;
-      } else if (ret == CFX_BidiChar::LEFT) {
-        nL2R++;
-      }
-    }
+    if (wChar)
+      str += wChar;
   }
-  if (pBidiChar->EndChar()) {
-    CFX_BidiChar::Direction ret = pBidiChar->GetBidiInfo(&start, &count);
-    if (ret == CFX_BidiChar::RIGHT) {
-      nR2L++;
-    } else if (ret == CFX_BidiChar::LEFT) {
-      nL2R++;
-    }
-  }
-  return (nR2L > 0 && nR2L >= nL2R);
+  return CFX_BidiString(str).OverallDirection() == CFX_BidiChar::RIGHT;
 }
+
 void CPDF_TextPage::ProcessTextObject(PDFTEXT_Obj Obj) {
   CPDF_TextObject* pTextObj = Obj.m_pTextObj;
   if (FXSYS_fabs(pTextObj->m_Right - pTextObj->m_Left) < 0.01f) {
diff --git a/core/src/fpdftext/text_int.h b/core/src/fpdftext/text_int.h
index 93c22736ef..9131cdb321 100644
--- a/core/src/fpdftext/text_int.h
+++ b/core/src/fpdftext/text_int.h
@@ -121,8 +121,8 @@ class CPDF_TextPage : public IPDF_TextPage {
   void ProcessMarkedContent(PDFTEXT_Obj pObj);
   void CheckMarkedContentObject(int32_t& start, int32_t& nCount) const;
   void FindPreviousTextObject(void);
-  void AddCharInfoByLRDirection(CFX_WideString& str, int i);
-  void AddCharInfoByRLDirection(CFX_WideString& str, int i);
+  void AddCharInfoByLRDirection(FX_WCHAR wChar, PAGECHAR_INFO info);
+  void AddCharInfoByRLDirection(FX_WCHAR wChar, PAGECHAR_INFO info);
   int32_t GetTextObjectWritingMode(const CPDF_TextObject* pTextObj);
   int32_t FindTextlineFlowDirection();
 
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());
+}