Remove DefaultAppearance HasColor and return an optional instead

This CL converts CPDF_DefaultAppearance to return
Optional<CFX_Color:Type> items instead of having a HasColor. This saves
the double parse of the appearance stream.

Change-Id: Ib3c136da6e2adfb559e495de1d299cce0b4ad25f
Reviewed-on: https://pdfium-review.googlesource.com/29630
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

6 files changed, 51 insertions, 62 deletions

diff --git a/core/fpdfdoc/cpdf_defaultappearance.cpp b/core/fpdfdoc/cpdf_defaultappearance.cpp
index 161f90e014..4efb0655b4 100644
--- a/core/fpdfdoc/cpdf_defaultappearance.cpp
+++ b/core/fpdfdoc/cpdf_defaultappearance.cpp
@@ -68,64 +68,53 @@ Optional<ByteString> CPDF_DefaultAppearance::GetFont(float* fFontSize) {
   return {PDF_NameDecode(csFontNameTag.AsStringView())};
 }
 
-bool CPDF_DefaultAppearance::HasColor() {
-  if (m_csDA.IsEmpty())
-    return false;
-
-  CPDF_SimpleParser syntax(m_csDA.AsStringView());
-  if (FindTagParamFromStart(&syntax, "g", 1))
-    return true;
-  if (FindTagParamFromStart(&syntax, "rg", 3))
-    return true;
-  return FindTagParamFromStart(&syntax, "k", 4);
-}
-
-CFX_Color::Type CPDF_DefaultAppearance::GetColor(float fc[4]) {
+Optional<CFX_Color::Type> CPDF_DefaultAppearance::GetColor(float fc[4]) {
   for (int c = 0; c < 4; c++)
     fc[c] = 0;
 
   if (m_csDA.IsEmpty())
-    return CFX_Color::kTransparent;
+    return {};
 
   CPDF_SimpleParser syntax(m_csDA.AsStringView());
   if (FindTagParamFromStart(&syntax, "g", 1)) {
     fc[0] = FX_atof(syntax.GetWord());
-    return CFX_Color::kGray;
+    return {CFX_Color::kGray};
   }
   if (FindTagParamFromStart(&syntax, "rg", 3)) {
     fc[0] = FX_atof(syntax.GetWord());
     fc[1] = FX_atof(syntax.GetWord());
     fc[2] = FX_atof(syntax.GetWord());
-    return CFX_Color::kRGB;
+    return {CFX_Color::kRGB};
   }
   if (FindTagParamFromStart(&syntax, "k", 4)) {
     fc[0] = FX_atof(syntax.GetWord());
     fc[1] = FX_atof(syntax.GetWord());
     fc[2] = FX_atof(syntax.GetWord());
     fc[3] = FX_atof(syntax.GetWord());
-    return CFX_Color::kCMYK;
+    return {CFX_Color::kCMYK};
   }
 
-  return CFX_Color::kTransparent;
+  return {};
 }
 
-std::pair<CFX_Color::Type, FX_ARGB> CPDF_DefaultAppearance::GetColor() {
+std::pair<Optional<CFX_Color::Type>, FX_ARGB>
+CPDF_DefaultAppearance::GetColor() {
   float values[4];
-  CFX_Color::Type type = GetColor(values);
-  if (type == CFX_Color::kTransparent)
+  Optional<CFX_Color::Type> type = GetColor(values);
+  if (!type)
     return {type, 0};
 
-  if (type == CFX_Color::kGray) {
+  if (*type == CFX_Color::kGray) {
     int g = static_cast<int>(values[0] * 255 + 0.5f);
     return {type, ArgbEncode(255, g, g, g)};
   }
-  if (type == CFX_Color::kRGB) {
+  if (*type == CFX_Color::kRGB) {
     int r = static_cast<int>(values[0] * 255 + 0.5f);
     int g = static_cast<int>(values[1] * 255 + 0.5f);
     int b = static_cast<int>(values[2] * 255 + 0.5f);
     return {type, ArgbEncode(255, r, g, b)};
   }
-  if (type == CFX_Color::kCMYK) {
+  if (*type == CFX_Color::kCMYK) {
     float r = 1.0f - std::min(1.0f, values[0] + values[3]);
     float g = 1.0f - std::min(1.0f, values[1] + values[3]);
     float b = 1.0f - std::min(1.0f, values[2] + values[3]);
@@ -134,7 +123,7 @@ std::pair<CFX_Color::Type, FX_ARGB> CPDF_DefaultAppearance::GetColor() {
                              static_cast<int>(b * 255 + 0.5f))};
   }
   NOTREACHED();
-  return {CFX_Color::kTransparent, 0};
+  return {{}, 0};
 }
 
 bool CPDF_DefaultAppearance::FindTagParamFromStartForTesting(
diff --git a/core/fpdfdoc/cpdf_defaultappearance.h b/core/fpdfdoc/cpdf_defaultappearance.h
index ae3c4f5517..73e35c7cd4 100644
--- a/core/fpdfdoc/cpdf_defaultappearance.h
+++ b/core/fpdfdoc/cpdf_defaultappearance.h
@@ -27,9 +27,8 @@ class CPDF_DefaultAppearance {
 
   Optional<ByteString> GetFont(float* fFontSize);
 
-  bool HasColor();
-  CFX_Color::Type GetColor(float fc[4]);
-  std::pair<CFX_Color::Type, FX_ARGB> GetColor();
+  Optional<CFX_Color::Type> GetColor(float fc[4]);
+  std::pair<Optional<CFX_Color::Type>, FX_ARGB> GetColor();
 
   bool FindTagParamFromStartForTesting(CPDF_SimpleParser* parser,
                                        const ByteStringView& token,
diff --git a/core/fxge/cfx_color.cpp b/core/fxge/cfx_color.cpp
index 8a2f42d920..19d1d4fd24 100644
--- a/core/fxge/cfx_color.cpp
+++ b/core/fxge/cfx_color.cpp
@@ -85,17 +85,15 @@ CFX_Color CFX_Color::ParseColor(const CPDF_Array& array) {
 // Static.
 CFX_Color CFX_Color::ParseColor(const ByteString& str) {
   CPDF_DefaultAppearance appearance(str);
-  ASSERT(appearance.HasColor());
-
   float values[4];
-  int color_type = appearance.GetColor(values);
-  if (color_type == CFX_Color::kTransparent)
+  Optional<CFX_Color::Type> color_type = appearance.GetColor(values);
+  if (!color_type || *color_type == CFX_Color::kTransparent)
     return CFX_Color(CFX_Color::kTransparent);
-  if (color_type == CFX_Color::kGray)
+  if (*color_type == CFX_Color::kGray)
     return CFX_Color(CFX_Color::kGray, values[0]);
-  if (color_type == CFX_Color::kRGB)
+  if (*color_type == CFX_Color::kRGB)
     return CFX_Color(CFX_Color::kRGB, values[0], values[1], values[2]);
-  if (color_type == CFX_Color::kCMYK) {
+  if (*color_type == CFX_Color::kCMYK) {
     return CFX_Color(CFX_Color::kCMYK, values[0], values[1], values[2],
                      values[3]);
   }
diff --git a/fpdfsdk/cpdfsdk_widget.cpp b/fpdfsdk/cpdfsdk_widget.cpp
index f7aae09933..d2d8ecb148 100644
--- a/fpdfsdk/cpdfsdk_widget.cpp
+++ b/fpdfsdk/cpdfsdk_widget.cpp
@@ -431,14 +431,14 @@ bool CPDFSDK_Widget::GetBorderColor(FX_COLORREF& color) const {
 bool CPDFSDK_Widget::GetTextColor(FX_COLORREF& color) const {
   CPDF_FormControl* pFormCtrl = GetFormControl();
   CPDF_DefaultAppearance da = pFormCtrl->GetDefaultAppearance();
-  if (!da.HasColor())
-    return false;
-
   FX_ARGB argb;
-  int iColorType;
+  Optional<CFX_Color::Type> iColorType;
   std::tie(iColorType, argb) = da.GetColor();
+  if (!iColorType)
+    return false;
+
   color = ArgbToColorRef(argb);
-  return iColorType != CFX_Color::kTransparent;
+  return *iColorType != CFX_Color::kTransparent;
 }
 
 float CPDFSDK_Widget::GetFontSize() const {
@@ -779,11 +779,11 @@ CFX_Color CPDFSDK_Widget::GetTextPWLColor() const {
 
   CPDF_FormControl* pFormCtrl = GetFormControl();
   CPDF_DefaultAppearance da = pFormCtrl->GetDefaultAppearance();
-  if (da.HasColor()) {
-    float fc[4];
-    int iColorType = da.GetColor(fc);
-    crText = CFX_Color(iColorType, fc[0], fc[1], fc[2], fc[3]);
-  }
+
+  float fc[4];
+  Optional<CFX_Color::Type> iColorType = da.GetColor(fc);
+  if (iColorType)
+    crText = CFX_Color(*iColorType, fc[0], fc[1], fc[2], fc[3]);
 
   return crText;
 }
diff --git a/fpdfsdk/pwl/cpwl_appstream.cpp b/fpdfsdk/pwl/cpwl_appstream.cpp
index 6e65831627..cbe6501a32 100644
--- a/fpdfsdk/pwl/cpwl_appstream.cpp
+++ b/fpdfsdk/pwl/cpwl_appstream.cpp
@@ -1180,8 +1180,9 @@ void CPWL_AppStream::SetAsPushButton() {
   CFX_Color crText(CFX_Color::kGray, 0);
   ByteString csNameTag;
   CPDF_DefaultAppearance da = pControl->GetDefaultAppearance();
-  if (da.HasColor()) {
-    iColorType = da.GetColor(fc);
+  Optional<CFX_Color::Type> color = da.GetColor(fc);
+  if (color) {
+    iColorType = *color;
     crText = CFX_Color(iColorType, fc[0], fc[1], fc[2], fc[3]);
   }
 
@@ -1358,8 +1359,9 @@ void CPWL_AppStream::SetAsCheckBox() {
   CFX_FloatRect rcWindow = widget_->GetRotatedRect();
   CFX_FloatRect rcClient = rcWindow.GetDeflated(fBorderWidth, fBorderWidth);
   CPDF_DefaultAppearance da = pControl->GetDefaultAppearance();
-  if (da.HasColor()) {
-    iColorType = da.GetColor(fc);
+  Optional<CFX_Color::Type> color = da.GetColor(fc);
+  if (color) {
+    iColorType = *color;
     crText = CFX_Color(iColorType, fc[0], fc[1], fc[2], fc[3]);
   }
 
@@ -1474,8 +1476,9 @@ void CPWL_AppStream::SetAsRadioButton() {
   CFX_FloatRect rcWindow = widget_->GetRotatedRect();
   CFX_FloatRect rcClient = rcWindow.GetDeflated(fBorderWidth, fBorderWidth);
   CPDF_DefaultAppearance da = pControl->GetDefaultAppearance();
-  if (da.HasColor()) {
-    iColorType = da.GetColor(fc);
+  Optional<CFX_Color::Type> color = da.GetColor(fc);
+  if (color) {
+    iColorType = *color;
     crText = CFX_Color(iColorType, fc[0], fc[1], fc[2], fc[3]);
   }
 
diff --git a/fxjs/cjs_field.cpp b/fxjs/cjs_field.cpp
index acc0fe3188..3ae40a97a8 100644
--- a/fxjs/cjs_field.cpp
+++ b/fxjs/cjs_field.cpp
@@ -1885,22 +1885,22 @@ CJS_Return CJS_Field::get_text_color(CJS_Runtime* pRuntime) {
   if (!pFormControl)
     return CJS_Return(false);
 
-  int iColorType;
+  Optional<CFX_Color::Type> iColorType;
   FX_ARGB color;
   CPDF_DefaultAppearance FieldAppearance = pFormControl->GetDefaultAppearance();
   std::tie(iColorType, color) = FieldAppearance.GetColor();
 
-  int32_t a;
-  int32_t r;
-  int32_t g;
-  int32_t b;
-  std::tie(a, r, g, b) = ArgbDecode(color);
-
-  CFX_Color crRet =
-      CFX_Color(CFX_Color::kRGB, r / 255.0f, g / 255.0f, b / 255.0f);
-
-  if (iColorType == CFX_Color::kTransparent)
+  CFX_Color crRet;
+  if (!iColorType || *iColorType == CFX_Color::kTransparent) {
     crRet = CFX_Color(CFX_Color::kTransparent);
+  } else {
+    int32_t a;
+    int32_t r;
+    int32_t g;
+    int32_t b;
+    std::tie(a, r, g, b) = ArgbDecode(color);
+    crRet = CFX_Color(CFX_Color::kRGB, r / 255.0f, g / 255.0f, b / 255.0f);
+  }
 
   v8::Local<v8::Value> array =
       CJS_Color::ConvertPWLColorToArray(pRuntime, crRet);