Clean up CXFA_Measurement and callers.

Change-Id: Iaa34127aaf616a0a7bdf7ccc8f8f001d34abcca1
Reviewed-on: https://pdfium-review.googlesource.com/7371
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

5 files changed, 108 insertions, 121 deletions

diff --git a/xfa/fxfa/parser/cscript_layoutpseudomodel.cpp b/xfa/fxfa/parser/cscript_layoutpseudomodel.cpp
index 8eeb0ff30f..46db2ca78c 100644
--- a/xfa/fxfa/parser/cscript_layoutpseudomodel.cpp
+++ b/xfa/fxfa/parser/cscript_layoutpseudomodel.cpp
@@ -69,34 +69,27 @@ void CScript_LayoutPseudoModel::HWXY(CFXJSE_Arguments* pArguments,
     ThrowParamCountMismatchException(methodName);
     return;
   }
-  CXFA_Node* pNode = nullptr;
+  CXFA_Node* pNode = static_cast<CXFA_Node*>(pArguments->GetObject(0));
+  if (!pNode)
+    return;
+
   CFX_WideString wsUnit(L"pt");
-  int32_t iIndex = 0;
-  if (iLength >= 1) {
-    pNode = static_cast<CXFA_Node*>(pArguments->GetObject(0));
-  }
   if (iLength >= 2) {
     CFX_ByteString bsUnit = pArguments->GetUTF8String(1);
-    if (!bsUnit.IsEmpty()) {
+    if (!bsUnit.IsEmpty())
       wsUnit = CFX_WideString::FromUTF8(bsUnit.AsStringC());
-    }
-  }
-  if (iLength >= 3) {
-    iIndex = pArguments->GetInt32(2);
-  }
-  if (!pNode) {
-    return;
   }
+
+  int32_t iIndex = iLength >= 3 ? pArguments->GetInt32(2) : 0;
+
   CXFA_LayoutProcessor* pDocLayout = m_pDocument->GetDocLayout();
-  if (!pDocLayout) {
+  if (!pDocLayout)
     return;
-  }
 
-  CXFA_Measurement measure;
   CXFA_LayoutItem* pLayoutItem = pDocLayout->GetLayoutItem(pNode);
-  if (!pLayoutItem) {
+  if (!pLayoutItem)
     return;
-  }
+
   while (iIndex > 0 && pLayoutItem) {
     pLayoutItem = pLayoutItem->GetNext();
     iIndex--;
@@ -107,6 +100,7 @@ void CScript_LayoutPseudoModel::HWXY(CFXJSE_Arguments* pArguments,
     return;
   }
 
+  CXFA_Measurement measure;
   CFX_RectF rtRect = pLayoutItem->GetRect(true);
   switch (layoutModel) {
     case XFA_LAYOUTMODEL_H:
@@ -122,11 +116,10 @@ void CScript_LayoutPseudoModel::HWXY(CFXJSE_Arguments* pArguments,
       measure.Set(rtRect.top, XFA_UNIT_Pt);
       break;
   }
-  XFA_UNIT unit = measure.GetUnit(wsUnit.AsStringC());
-  float fValue = measure.ToUnit(unit);
+  float fValue =
+      measure.ToUnit(CXFA_Measurement::GetUnitFromString(wsUnit.AsStringC()));
   fValue = FXSYS_round(fValue * 1000) / 1000.0f;
-  if (pValue)
-    pValue->SetFloat(fValue);
+  pValue->SetFloat(fValue);
 }
 
 void CScript_LayoutPseudoModel::H(CFXJSE_Arguments* pArguments) {
@@ -167,9 +160,7 @@ void CScript_LayoutPseudoModel::NumberedPageCount(CFXJSE_Arguments* pArguments,
   } else {
     iPageCount = iPageNum;
   }
-  CFXJSE_Value* pValue = pArguments->GetReturnValue();
-  if (pValue)
-    pValue->SetInteger(iPageCount);
+  pArguments->GetReturnValue()->SetInteger(iPageCount);
 }
 
 void CScript_LayoutPseudoModel::PageCount(CFXJSE_Arguments* pArguments) {
@@ -202,12 +193,11 @@ void CScript_LayoutPseudoModel::PageSpan(CFXJSE_Arguments* pArguments) {
   int32_t iLast = pLayoutItem->GetLast()->GetPage()->GetPageIndex();
   int32_t iFirst = pLayoutItem->GetFirst()->GetPage()->GetPageIndex();
   int32_t iPageSpan = iLast - iFirst + 1;
-  if (pValue)
-    pValue->SetInteger(iPageSpan);
+  pValue->SetInteger(iPageSpan);
 }
 
 void CScript_LayoutPseudoModel::Page(CFXJSE_Arguments* pArguments) {
-  PageImp(pArguments, false);
+  PageInternals(pArguments, false);
 }
 
 std::vector<CXFA_Node*> CScript_LayoutPseudoModel::GetObjArray(
@@ -383,16 +373,12 @@ void CScript_LayoutPseudoModel::AbsPageCount(CFXJSE_Arguments* pArguments) {
 
 void CScript_LayoutPseudoModel::AbsPageCountInBatch(
     CFXJSE_Arguments* pArguments) {
-  CFXJSE_Value* pValue = pArguments->GetReturnValue();
-  if (pValue)
-    pValue->SetInteger(0);
+  pArguments->GetReturnValue()->SetInteger(0);
 }
 
 void CScript_LayoutPseudoModel::SheetCountInBatch(
     CFXJSE_Arguments* pArguments) {
-  CFXJSE_Value* pValue = pArguments->GetReturnValue();
-  if (pValue)
-    pValue->SetInteger(0);
+  pArguments->GetReturnValue()->SetInteger(0);
 }
 
 void CScript_LayoutPseudoModel::Relayout(CFXJSE_Arguments* pArguments) {
@@ -416,9 +402,7 @@ void CScript_LayoutPseudoModel::AbsPageInBatch(CFXJSE_Arguments* pArguments) {
     return;
   }
 
-  CFXJSE_Value* pValue = pArguments->GetReturnValue();
-  if (pValue)
-    pValue->SetInteger(0);
+  pArguments->GetReturnValue()->SetInteger(0);
 }
 
 void CScript_LayoutPseudoModel::SheetInBatch(CFXJSE_Arguments* pArguments) {
@@ -427,13 +411,11 @@ void CScript_LayoutPseudoModel::SheetInBatch(CFXJSE_Arguments* pArguments) {
     return;
   }
 
-  CFXJSE_Value* pValue = pArguments->GetReturnValue();
-  if (pValue)
-    pValue->SetInteger(0);
+  pArguments->GetReturnValue()->SetInteger(0);
 }
 
 void CScript_LayoutPseudoModel::Sheet(CFXJSE_Arguments* pArguments) {
-  PageImp(pArguments, true);
+  PageInternals(pArguments, true);
 }
 
 void CScript_LayoutPseudoModel::RelayoutPageArea(CFXJSE_Arguments* pArguments) {
@@ -444,43 +426,33 @@ void CScript_LayoutPseudoModel::SheetCount(CFXJSE_Arguments* pArguments) {
 }
 
 void CScript_LayoutPseudoModel::AbsPage(CFXJSE_Arguments* pArguments) {
-  PageImp(pArguments, true);
+  PageInternals(pArguments, true);
 }
 
-void CScript_LayoutPseudoModel::PageImp(CFXJSE_Arguments* pArguments,
-                                        bool bAbsPage) {
+void CScript_LayoutPseudoModel::PageInternals(CFXJSE_Arguments* pArguments,
+                                              bool bAbsPage) {
   int32_t iLength = pArguments->GetLength();
   if (iLength != 1) {
-    const wchar_t* methodName;
-    if (bAbsPage) {
-      methodName = L"absPage";
-    } else {
-      methodName = L"page";
-    }
-    ThrowParamCountMismatchException(methodName);
+    ThrowParamCountMismatchException(bAbsPage ? L"absPage" : L"page");
     return;
   }
-  CXFA_Node* pNode = nullptr;
-  if (iLength >= 1) {
-    pNode = static_cast<CXFA_Node*>(pArguments->GetObject(0));
-  }
-  int32_t iPage = 0;
+
+  CXFA_Node* pNode = static_cast<CXFA_Node*>(pArguments->GetObject(0));
   CFXJSE_Value* pValue = pArguments->GetReturnValue();
-  if (!pNode && pValue)
-    pValue->SetInteger(iPage);
+  if (!pNode)
+    pValue->SetInteger(0);
 
   CXFA_LayoutProcessor* pDocLayout = m_pDocument->GetDocLayout();
-  if (!pDocLayout) {
+  if (!pDocLayout)
     return;
-  }
+
   CXFA_LayoutItem* pLayoutItem = pDocLayout->GetLayoutItem(pNode);
   if (!pLayoutItem) {
     pValue->SetInteger(-1);
     return;
   }
-  iPage = pLayoutItem->GetFirst()->GetPage()->GetPageIndex();
-  if (pValue)
-    pValue->SetInteger(bAbsPage ? iPage : iPage + 1);
+  int32_t iPage = pLayoutItem->GetFirst()->GetPage()->GetPageIndex();
+  pValue->SetInteger(bAbsPage ? iPage : iPage + 1);
 }
 
 void CScript_LayoutPseudoModel::ThrowSetReadyException() const {
diff --git a/xfa/fxfa/parser/cscript_layoutpseudomodel.h b/xfa/fxfa/parser/cscript_layoutpseudomodel.h
index ea31a4205e..6f98997b0d 100644
--- a/xfa/fxfa/parser/cscript_layoutpseudomodel.h
+++ b/xfa/fxfa/parser/cscript_layoutpseudomodel.h
@@ -55,7 +55,8 @@ class CScript_LayoutPseudoModel : public CXFA_Object {
                                       int32_t iPageNo,
                                       const CFX_WideString& wsType,
                                       bool bOnPageArea);
-  void PageImp(CFXJSE_Arguments* pArguments, bool bAbsPage);
+
+  void PageInternals(CFXJSE_Arguments* pArguments, bool bAbsPage);
   void ThrowSetReadyException() const;
 };
 
diff --git a/xfa/fxfa/parser/cxfa_measurement.cpp b/xfa/fxfa/parser/cxfa_measurement.cpp
index 70420d7b5e..f161e49e49 100644
--- a/xfa/fxfa/parser/cxfa_measurement.cpp
+++ b/xfa/fxfa/parser/cxfa_measurement.cpp
@@ -8,8 +8,18 @@
 
 #include "core/fxcrt/fx_extension.h"
 
+namespace {
+
+constexpr float kPtToInch = 72;
+constexpr float kPtToCm = kPtToInch / 2.54f;
+constexpr float kPtToMm = kPtToCm / 10;
+constexpr float kPtToMp = 0.001f;
+constexpr float kPtToPc = 12;
+
+}  // namespace
+
 CXFA_Measurement::CXFA_Measurement(const CFX_WideStringC& wsMeasure) {
-  Set(wsMeasure);
+  SetString(wsMeasure);
 }
 
 CXFA_Measurement::CXFA_Measurement() {
@@ -20,7 +30,7 @@ CXFA_Measurement::CXFA_Measurement(float fValue, XFA_UNIT eUnit) {
   Set(fValue, eUnit);
 }
 
-void CXFA_Measurement::Set(const CFX_WideStringC& wsMeasure) {
+void CXFA_Measurement::SetString(const CFX_WideStringC& wsMeasure) {
   if (wsMeasure.IsEmpty()) {
     m_fValue = 0;
     m_eUnit = XFA_UNIT_Unknown;
@@ -30,44 +40,49 @@ void CXFA_Measurement::Set(const CFX_WideStringC& wsMeasure) {
   int32_t iOffset = (wsMeasure.GetAt(0) == L'=') ? 1 : 0;
   float fValue = FXSYS_wcstof(wsMeasure.unterminated_c_str() + iOffset,
                               wsMeasure.GetLength() - iOffset, &iUsedLen);
-  XFA_UNIT eUnit = GetUnit(wsMeasure.Mid(iOffset + iUsedLen));
+  XFA_UNIT eUnit = GetUnitFromString(wsMeasure.Mid(iOffset + iUsedLen));
   Set(fValue, eUnit);
 }
 
-bool CXFA_Measurement::ToString(CFX_WideString& wsMeasure) const {
+bool CXFA_Measurement::ToString(CFX_WideString* wsMeasure) const {
   switch (GetUnit()) {
     case XFA_UNIT_Mm:
-      wsMeasure.Format(L"%.8gmm", GetValue());
+      wsMeasure->Format(L"%.8gmm", GetValue());
       return true;
     case XFA_UNIT_Pt:
-      wsMeasure.Format(L"%.8gpt", GetValue());
+      wsMeasure->Format(L"%.8gpt", GetValue());
       return true;
     case XFA_UNIT_In:
-      wsMeasure.Format(L"%.8gin", GetValue());
+      wsMeasure->Format(L"%.8gin", GetValue());
       return true;
     case XFA_UNIT_Cm:
-      wsMeasure.Format(L"%.8gcm", GetValue());
+      wsMeasure->Format(L"%.8gcm", GetValue());
       return true;
     case XFA_UNIT_Mp:
-      wsMeasure.Format(L"%.8gmp", GetValue());
+      wsMeasure->Format(L"%.8gmp", GetValue());
       return true;
     case XFA_UNIT_Pc:
-      wsMeasure.Format(L"%.8gpc", GetValue());
+      wsMeasure->Format(L"%.8gpc", GetValue());
       return true;
     case XFA_UNIT_Em:
-      wsMeasure.Format(L"%.8gem", GetValue());
+      wsMeasure->Format(L"%.8gem", GetValue());
       return true;
     case XFA_UNIT_Percent:
-      wsMeasure.Format(L"%.8g%%", GetValue());
+      wsMeasure->Format(L"%.8g%%", GetValue());
       return true;
     default:
-      wsMeasure.Format(L"%.8g", GetValue());
+      wsMeasure->Format(L"%.8g", GetValue());
       return false;
   }
 }
 
-bool CXFA_Measurement::ToUnit(XFA_UNIT eUnit, float& fValue) const {
-  fValue = GetValue();
+float CXFA_Measurement::ToUnit(XFA_UNIT eUnit) const {
+  float f;
+  return ToUnitInternal(eUnit, &f) ? f : 0;
+}
+
+bool CXFA_Measurement::ToUnitInternal(XFA_UNIT eUnit, float* fValue) const {
+  *fValue = GetValue();
   XFA_UNIT eFrom = GetUnit();
   if (eFrom == eUnit)
     return true;
@@ -76,49 +91,50 @@ bool CXFA_Measurement::ToUnit(XFA_UNIT eUnit, float& fValue) const {
     case XFA_UNIT_Pt:
       break;
     case XFA_UNIT_Mm:
-      fValue *= 72 / 2.54f / 10;
+      *fValue *= kPtToMm;
       break;
     case XFA_UNIT_In:
-      fValue *= 72;
+      *fValue *= kPtToInch;
       break;
     case XFA_UNIT_Cm:
-      fValue *= 72 / 2.54f;
+      *fValue *= kPtToCm;
       break;
     case XFA_UNIT_Mp:
-      fValue *= 0.001f;
+      *fValue *= kPtToMp;
       break;
     case XFA_UNIT_Pc:
-      fValue *= 12.0f;
+      *fValue *= kPtToPc;
       break;
     default:
-      fValue = 0;
+      *fValue = 0;
       return false;
   }
   switch (eUnit) {
     case XFA_UNIT_Pt:
       return true;
     case XFA_UNIT_Mm:
-      fValue /= 72 / 2.54f / 10;
+      *fValue /= kPtToMm;
       return true;
     case XFA_UNIT_In:
-      fValue /= 72;
+      *fValue /= kPtToInch;
       return true;
     case XFA_UNIT_Cm:
-      fValue /= 72 / 2.54f;
+      *fValue /= kPtToCm;
       return true;
     case XFA_UNIT_Mp:
-      fValue /= 0.001f;
+      *fValue /= kPtToMp;
       return true;
     case XFA_UNIT_Pc:
-      fValue /= 12.0f;
+      *fValue /= kPtToPc;
       return true;
     default:
-      fValue = 0;
+      NOTREACHED();
       return false;
   }
 }
 
-XFA_UNIT CXFA_Measurement::GetUnit(const CFX_WideStringC& wsUnit) {
+// static
+XFA_UNIT CXFA_Measurement::GetUnitFromString(const CFX_WideStringC& wsUnit) {
   if (wsUnit == L"mm")
     return XFA_UNIT_Mm;
   if (wsUnit == L"pt")
diff --git a/xfa/fxfa/parser/cxfa_measurement.h b/xfa/fxfa/parser/cxfa_measurement.h
index 34cf780f81..2c03ec0dc6 100644
--- a/xfa/fxfa/parser/cxfa_measurement.h
+++ b/xfa/fxfa/parser/cxfa_measurement.h
@@ -17,24 +17,23 @@ class CXFA_Measurement {
   CXFA_Measurement();
   CXFA_Measurement(float fValue, XFA_UNIT eUnit);
 
-  void Set(const CFX_WideStringC& wsMeasure);
+  static XFA_UNIT GetUnitFromString(const CFX_WideStringC& wsUnit);
+
   void Set(float fValue, XFA_UNIT eUnit) {
     m_fValue = fValue;
     m_eUnit = eUnit;
   }
 
-  XFA_UNIT GetUnit(const CFX_WideStringC& wsUnit);
   XFA_UNIT GetUnit() const { return m_eUnit; }
   float GetValue() const { return m_fValue; }
 
-  bool ToString(CFX_WideString& wsMeasure) const;
-  bool ToUnit(XFA_UNIT eUnit, float& fValue) const;
-  float ToUnit(XFA_UNIT eUnit) const {
-    float f;
-    return ToUnit(eUnit, f) ? f : 0;
-  }
+  bool ToString(CFX_WideString* wsMeasure) const;
+  float ToUnit(XFA_UNIT eUnit) const;
 
  private:
+  void SetString(const CFX_WideStringC& wsMeasure);
+  bool ToUnitInternal(XFA_UNIT eUnit, float* fValue) const;
+
   float m_fValue;
   XFA_UNIT m_eUnit;
 };
diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index 11c2c3c35e..eac7ac04b0 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -2087,7 +2087,7 @@ void CXFA_Node::Script_Som_BorderWidth(CFXJSE_Value* pValue,
   } else {
     CXFA_Edge edge = border.GetEdge(0);
     CXFA_Measurement thickness = edge.GetMSThickness();
-    thickness.ToString(wsThickness);
+    thickness.ToString(&wsThickness);
     pValue->SetString(wsThickness.UTF8Encode().AsStringC());
   }
 }
@@ -3606,9 +3606,9 @@ bool CXFA_Node::GetAttribute(XFA_ATTRIBUTE eAttr,
                              CFX_WideString& wsValue,
                              bool bUseDefault) {
   const XFA_ATTRIBUTEINFO* pAttr = XFA_GetAttributeByID(eAttr);
-  if (!pAttr) {
+  if (!pAttr)
     return false;
-  }
+
   XFA_ATTRIBUTETYPE eType = pAttr->eType;
   if (eType == XFA_ATTRIBUTETYPE_NOTSURE) {
     const XFA_NOTSUREATTRIBUTE* pNotsure =
@@ -3618,48 +3618,47 @@ bool CXFA_Node::GetAttribute(XFA_ATTRIBUTE eAttr,
   switch (eType) {
     case XFA_ATTRIBUTETYPE_Enum: {
       XFA_ATTRIBUTEENUM eValue;
-      if (!TryEnum(pAttr->eName, eValue, bUseDefault)) {
+      if (!TryEnum(pAttr->eName, eValue, bUseDefault))
         return false;
-      }
+
       wsValue = GetAttributeEnumByID(eValue)->pName;
       return true;
-    } break;
+    }
     case XFA_ATTRIBUTETYPE_Cdata: {
       CFX_WideStringC wsValueC;
-      if (!TryCData(pAttr->eName, wsValueC, bUseDefault)) {
+      if (!TryCData(pAttr->eName, wsValueC, bUseDefault))
         return false;
-      }
+
       wsValue = wsValueC;
       return true;
-    } break;
+    }
     case XFA_ATTRIBUTETYPE_Boolean: {
       bool bValue;
-      if (!TryBoolean(pAttr->eName, bValue, bUseDefault)) {
+      if (!TryBoolean(pAttr->eName, bValue, bUseDefault))
         return false;
-      }
+
       wsValue = bValue ? L"1" : L"0";
       return true;
-    } break;
+    }
     case XFA_ATTRIBUTETYPE_Integer: {
       int32_t iValue;
-      if (!TryInteger(pAttr->eName, iValue, bUseDefault)) {
+      if (!TryInteger(pAttr->eName, iValue, bUseDefault))
         return false;
-      }
+
       wsValue.Format(L"%d", iValue);
       return true;
-    } break;
+    }
     case XFA_ATTRIBUTETYPE_Measure: {
       CXFA_Measurement mValue;
-      if (!TryMeasure(pAttr->eName, mValue, bUseDefault)) {
+      if (!TryMeasure(pAttr->eName, mValue, bUseDefault))
         return false;
-      }
-      mValue.ToString(wsValue);
+
+      mValue.ToString(&wsValue);
       return true;
-    } break;
+    }
     default:
-      break;
+      return false;
   }
-  return false;
 }
 
 bool CXFA_Node::SetAttribute(const CFX_WideStringC& wsAttr,