More define cleanup.

core/fpdftext/fpdf_text_int.cpp

Index: core/fpdftext/fpdf_text_int.cpp
diff --git a/core/fpdftext/fpdf_text_int.cpp b/core/fpdftext/fpdf_text_int.cpp
index b3558ff2a847f886c9f0002df26f0259608995f6..cdaf9085a9b86eb963a1c3b8632d03cff56cf382 100644
--- a/core/fpdftext/fpdf_text_int.cpp
+++ b/core/fpdftext/fpdf_text_int.cpp
@@ -28,10 +28,6 @@
 #include "core/fxcrt/include/fx_ucd.h"
 #include "third_party/base/stl_util.h"
 
-#define FPDFTEXT_RLTB 1
-#define FPDFTEXT_LEFT -1
-#define FPDFTEXT_RIGHT 1

[dsinclair, 2016/05/02 20:59:55]

RLTB and RIGHT were both the same value so I removed RLTB.

[Tom Sepez, 2016/05/02 21:45:39]

Acknowledged.

-
 #define FPDFTEXT_MATCHCASE 0x00000001
 #define FPDFTEXT_MATCHWHOLEWORD 0x00000002
 #define FPDFTEXT_CONSECUTIVE 0x00000004
@@ -42,9 +38,15 @@
 #define FPDFTEXT_CHAR_UNUNICODE 2
 #define FPDFTEXT_CHAR_HYPHEN 3
 #define FPDFTEXT_CHAR_PIECE 4
-#define FPDFTEXT_MC_PASS 0
-#define FPDFTEXT_MC_DONE 1
-#define FPDFTEXT_MC_DELAY 2
+
+#define TEXT_BLANK_CHAR L' '
+#define TEXT_LINEFEED_CHAR L'\n'
+#define TEXT_RETURN_CHAR L'\r'
+#define TEXT_EMPTY L""
+#define TEXT_BLANK L" "
+#define TEXT_RETURN_LINEFEED L"\r\n"
+#define TEXT_LINEFEED L"\n"
+#define TEXT_CHARRATIO_GAPDELTA 0.070
 
 namespace {
 
@@ -143,16 +145,7 @@ FX_STRSIZE Unicode_GetNormalization(FX_WCHAR wch, FX_WCHAR* pDst) {
 
 }  // namespace
 
-#define TEXT_BLANK_CHAR L' '
-#define TEXT_LINEFEED_CHAR L'\n'
-#define TEXT_RETURN_CHAR L'\r'
-#define TEXT_EMPTY L""
-#define TEXT_BLANK L" "
-#define TEXT_RETURN_LINEFEED L"\r\n"
-#define TEXT_LINEFEED L"\n"
-#define TEXT_CHARRATIO_GAPDELTA 0.070
-
-CPDF_TextPage::CPDF_TextPage(const CPDF_Page* pPage, int flags)
+CPDF_TextPage::CPDF_TextPage(const CPDF_Page* pPage, FPDFText_Direction flags)
     : m_pPage(pPage),
       m_parserflag(flags),
       m_pPreTextObj(nullptr),
@@ -727,13 +720,10 @@ int CPDF_TextPage::CountBoundedSegments(FX_FLOAT left,
   return m_Segments.GetSize();
 }
 
-int CPDF_TextPage::GetWordBreak(int index, int direction) const {
+int CPDF_TextPage::GetWordBreak(int index, FPDFText_Direction direction) const {
   if (!m_bIsParsed)
     return -1;
 
-  if (direction != FPDFTEXT_LEFT && direction != FPDFTEXT_RIGHT)

[Tom Sepez, 2016/05/02 21:45:39]

given values of -1 and 1, is it possible that they were also silently using 0 as
a "dont't know"? 

[dsinclair, 2016/05/03 13:58:16]

Turns out, GetWordBreak is never called. Removed.

Now, the only time it's checked is to compare it to ::Right. The setter methods
would have previously set 1 or 0, which means ::Left was actually impossible. It
will now set ::Left or ::Right depending on the value set in the viewer
preferences.

-    return -1;
-
   if (index < 0 || index >= pdfium::CollectionSize<int>(m_CharList))
     return -1;
 
@@ -745,12 +735,12 @@ int CPDF_TextPage::GetWordBreak(int index, int direction) const {
     return index;
   }
   int breakPos = index;
-  if (direction == FPDFTEXT_LEFT) {
+  if (direction == FPDFText_Direction::Left) {
     while (--breakPos > 0) {
       if (!IsLetter(m_CharList[breakPos].m_Unicode))
         break;
     }
-  } else if (direction == FPDFTEXT_RIGHT) {
+  } else if (direction == FPDFText_Direction::Right) {
     while (++breakPos < pdfium::CollectionSize<int>(m_CharList)) {
       if (!IsLetter(m_CharList[breakPos].m_Unicode))
         break;
@@ -996,7 +986,7 @@ void CPDF_TextPage::CloseTempLine() {
     bPrevSpace = TRUE;
   }
   CFX_BidiString bidi(str);
-  if (m_parserflag == FPDFTEXT_RLTB)
+  if (m_parserflag == FPDFText_Direction::Right)
     bidi.SetOverallDirectionRight();
   CFX_BidiChar::Direction eCurrentDirection = bidi.OverallDirection();
   for (const auto& segment : bidi) {
@@ -1098,16 +1088,17 @@ void CPDF_TextPage::ProcessTextObject(
   }
 }
 
-int32_t CPDF_TextPage::PreMarkedContent(PDFTEXT_Obj Obj) {
+FPDFText_MarkedContent CPDF_TextPage::PreMarkedContent(PDFTEXT_Obj Obj) {
   CPDF_TextObject* pTextObj = Obj.m_pTextObj;
   CPDF_ContentMarkData* pMarkData =
       (CPDF_ContentMarkData*)pTextObj->m_ContentMark.GetObject();
   if (!pMarkData)
-    return FPDFTEXT_MC_PASS;
+    return FPDFText_MarkedContent::Pass;
 
   int nContentMark = pMarkData->CountItems();
   if (nContentMark < 1)
-    return FPDFTEXT_MC_PASS;
+    return FPDFText_MarkedContent::Pass;
+
   CFX_WideString actText;
   FX_BOOL bExist = FALSE;
   CPDF_Dictionary* pDict = NULL;
@@ -1125,19 +1116,19 @@ int32_t CPDF_TextPage::PreMarkedContent(PDFTEXT_Obj Obj) {
     }
   }
   if (!bExist)
-    return FPDFTEXT_MC_PASS;
+    return FPDFText_MarkedContent::Pass;
 
   if (m_pPreTextObj) {
     CPDF_ContentMarkData* pPreMarkData =
         (CPDF_ContentMarkData*)m_pPreTextObj->m_ContentMark.GetObject();
     if (pPreMarkData && pPreMarkData->CountItems() == n &&
         pDict == pPreMarkData->GetItem(n - 1).GetParam()) {
-      return FPDFTEXT_MC_DONE;
+      return FPDFText_MarkedContent::Done;
     }
   }
   FX_STRSIZE nItems = actText.GetLength();
   if (nItems < 1)
-    return FPDFTEXT_MC_PASS;
+    return FPDFText_MarkedContent::Pass;
 
   CPDF_Font* pFont = pTextObj->GetFont();
   bExist = FALSE;
@@ -1149,7 +1140,7 @@ int32_t CPDF_TextPage::PreMarkedContent(PDFTEXT_Obj Obj) {
     }
   }
   if (!bExist)
-    return FPDFTEXT_MC_PASS;
+    return FPDFText_MarkedContent::Pass;
 
   bExist = FALSE;
   for (FX_STRSIZE i = 0; i < nItems; i++) {
@@ -1160,9 +1151,9 @@ int32_t CPDF_TextPage::PreMarkedContent(PDFTEXT_Obj Obj) {
     }
   }
   if (!bExist)
-    return FPDFTEXT_MC_DONE;
+    return FPDFText_MarkedContent::Done;
 
-  return FPDFTEXT_MC_DELAY;
+  return FPDFText_MarkedContent::Delay;
 }
 
 void CPDF_TextPage::ProcessMarkedContent(PDFTEXT_Obj Obj) {
@@ -1285,8 +1276,8 @@ void CPDF_TextPage::ProcessTextObject(PDFTEXT_Obj Obj) {
   CFX_Matrix matrix;
   pTextObj->GetTextMatrix(&matrix);
   matrix.Concat(formMatrix);
-  int32_t bPreMKC = PreMarkedContent(Obj);
-  if (FPDFTEXT_MC_DONE == bPreMKC) {
+  FPDFText_MarkedContent bPreMKC = PreMarkedContent(Obj);

[Tom Sepez, 2016/05/02 21:45:39]

prolly shoujld be ePreMKC since its an enum not a bool.

[dsinclair, 2016/05/03 13:58:16]

Sigh, hungarian notation, so verbose.

[Tom Sepez, 2016/05/04 20:31:13]

Actually, it points out a problem that when the type of a variable changes, so
must its name, which makes generic programming difficult and why its a bad idea
in general.

+  if (bPreMKC == FPDFText_MarkedContent::Done) {
     m_pPreTextObj = pTextObj;
     m_perMatrix.Copy(formMatrix);
     return;
@@ -1362,7 +1353,7 @@ void CPDF_TextPage::ProcessTextObject(PDFTEXT_Obj Obj) {
         CFX_FloatRect(Obj.m_pTextObj->m_Left, Obj.m_pTextObj->m_Bottom,
                       Obj.m_pTextObj->m_Right, Obj.m_pTextObj->m_Top);
   }
-  if (FPDFTEXT_MC_DELAY == bPreMKC) {
+  if (bPreMKC == FPDFText_MarkedContent::Delay) {
     ProcessMarkedContent(Obj);
     m_pPreTextObj = pTextObj;
     m_perMatrix.Copy(formMatrix);