Change ArgbToColorRef() to ArgbToAlphaAndColorRef().chromium/3377

Since that is what it really returns. Add a replacement ArgbToColorRef()
that only returns a FX_COLORREF, and remove a duplicate implementation.
Then update callers and only use ArgbToAlphaAndColorRef() where
appropriate.

Also update comments to explain what FX_COLORREF is.

Change-Id: I5ed3d71683898dc2b3a66395dea0ec2562c14a68
Reviewed-on: https://pdfium-review.googlesource.com/28575
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

6 files changed, 39 insertions, 47 deletions

diff --git a/core/fxge/dib/fx_dib_main.cpp b/core/fxge/dib/fx_dib_main.cpp
index 161d2bd32e..5d7b585979 100644
--- a/core/fxge/dib/fx_dib_main.cpp
+++ b/core/fxge/dib/fx_dib_main.cpp
@@ -79,9 +79,12 @@ std::tuple<int, int, int, int> ArgbDecode(FX_ARGB argb) {
                          FXARGB_B(argb));
 }
 
-std::pair<int, FX_COLORREF> ArgbToColorRef(FX_ARGB argb) {
-  return {FXARGB_A(argb),
-          FXSYS_RGB(FXARGB_R(argb), FXARGB_G(argb), FXARGB_B(argb))};
+std::pair<int, FX_COLORREF> ArgbToAlphaAndColorRef(FX_ARGB argb) {
+  return {FXARGB_A(argb), ArgbToColorRef(argb)};
+}
+
+FX_COLORREF ArgbToColorRef(FX_ARGB argb) {
+  return FXSYS_RGB(FXARGB_R(argb), FXARGB_G(argb), FXARGB_B(argb));
 }
 
 uint32_t ArgbEncode(int a, FX_COLORREF bgr) {
diff --git a/core/fxge/fx_dib.h b/core/fxge/fx_dib.h
index 8134c5066d..5dcfac4962 100644
--- a/core/fxge/fx_dib.h
+++ b/core/fxge/fx_dib.h
@@ -37,9 +37,13 @@ struct PixelWeight {
   int m_Weights[1];
 };
 
-typedef uint32_t FX_ARGB;
-typedef uint32_t FX_COLORREF;
-typedef uint32_t FX_CMYK;
+using FX_ARGB = uint32_t;
+
+// FX_COLORREF, like win32 COLORREF, is BGR.
+using FX_COLORREF = uint32_t;
+
+using FX_CMYK = uint32_t;
+
 class CFX_ClipRgn;
 class CFX_DIBSource;
 class CStretchEngine;
@@ -86,11 +90,14 @@ inline FX_CMYK CmykEncode(int c, int m, int y, int k) {
   return (c << 24) | (m << 16) | (y << 8) | k;
 }
 
-// Returns tuple a, r, g, b
+// Returns (a, r, g, b)
 std::tuple<int, int, int, int> ArgbDecode(FX_ARGB argb);
 
-// Returns pair a, rgb
-std::pair<int, FX_COLORREF> ArgbToColorRef(FX_ARGB argb);
+// Returns (a, FX_COLORREF)
+std::pair<int, FX_COLORREF> ArgbToAlphaAndColorRef(FX_ARGB argb);
+
+// Returns FX_COLORREF.
+FX_COLORREF ArgbToColorRef(FX_ARGB argb);
 
 inline FX_ARGB ArgbEncode(int a, int r, int g, int b) {
   return (a << 24) | (r << 16) | (g << 8) | b;
diff --git a/core/fxge/win32/fx_win32_device.cpp b/core/fxge/win32/fx_win32_device.cpp
index 673593cdda..b857dde91b 100644
--- a/core/fxge/win32/fx_win32_device.cpp
+++ b/core/fxge/win32/fx_win32_device.cpp
@@ -132,11 +132,9 @@ HPEN CreateExtPen(const CFX_GraphStateData* pGraphState,
       break;
   }
 
-  int a;
-  FX_COLORREF bgr;
-  std::tie(a, bgr) = ArgbToColorRef(argb);
+  FX_COLORREF colorref = ArgbToColorRef(argb);
   LOGBRUSH lb;
-  lb.lbColor = bgr;
+  lb.lbColor = colorref;
   lb.lbStyle = BS_SOLID;
   lb.lbHatch = 0;
   std::vector<uint32_t> dashes;
@@ -155,10 +153,7 @@ HPEN CreateExtPen(const CFX_GraphStateData* pGraphState,
 }
 
 HBRUSH CreateBrush(uint32_t argb) {
-  int a;
-  FX_COLORREF bgr;
-  std::tie(a, bgr) = ArgbToColorRef(argb);
-  return CreateSolidBrush(bgr);
+  return CreateSolidBrush(ArgbToColorRef(argb));
 }
 
 void SetPathToDC(HDC hDC,
@@ -1073,15 +1068,15 @@ bool CGdiDeviceDriver::FillRectWithBlend(const FX_RECT* pRect,
     return false;
 
   int alpha;
-  FX_COLORREF bgr;
-  std::tie(alpha, bgr) = ArgbToColorRef(fill_color);
+  FX_COLORREF colorref;
+  std::tie(alpha, colorref) = ArgbToAlphaAndColorRef(fill_color);
   if (alpha == 0)
     return true;
 
   if (alpha < 255)
     return false;
 
-  HBRUSH hBrush = CreateSolidBrush(bgr);
+  HBRUSH hBrush = CreateSolidBrush(colorref);
   ::FillRect(m_hDC, (RECT*)pRect, hBrush);
   DeleteObject(hBrush);
   return true;
@@ -1126,13 +1121,13 @@ bool CGdiDeviceDriver::DrawCosmeticLine(const CFX_PointF& ptMoveTo,
   if (blend_type != FXDIB_BLEND_NORMAL)
     return false;
 
-  int a;
-  FX_COLORREF bgr;
-  std::tie(a, bgr) = ArgbToColorRef(color);
-  if (a == 0)
+  int alpha;
+  FX_COLORREF colorref;
+  std::tie(alpha, colorref) = ArgbToAlphaAndColorRef(color);
+  if (alpha == 0)
     return true;
 
-  HPEN hPen = CreatePen(PS_SOLID, 1, bgr);
+  HPEN hPen = CreatePen(PS_SOLID, 1, colorref);
   hPen = (HPEN)SelectObject(m_hDC, hPen);
   MoveToEx(m_hDC, FXSYS_round(ptMoveTo.x), FXSYS_round(ptMoveTo.y), nullptr);
   LineTo(m_hDC, FXSYS_round(ptLineTo.x), FXSYS_round(ptLineTo.y));
diff --git a/core/fxge/win32/fx_win32_print.cpp b/core/fxge/win32/fx_win32_print.cpp
index f2ae8a726e..66fbdd9b02 100644
--- a/core/fxge/win32/fx_win32_print.cpp
+++ b/core/fxge/win32/fx_win32_print.cpp
@@ -280,10 +280,8 @@ bool CGdiPrinterDriver::DrawDeviceText(int nChars,
   ModifyWorldTransform(m_hDC, &xform, MWT_LEFTMULTIPLY);
 
   // Color
-  int iUnusedAlpha;
-  FX_COLORREF bgr;
-  std::tie(iUnusedAlpha, bgr) = ArgbToColorRef(color);
-  SetTextColor(m_hDC, bgr);
+  FX_COLORREF colorref = ArgbToColorRef(color);
+  SetTextColor(m_hDC, colorref);
   SetBkMode(m_hDC, TRANSPARENT);
 
   // Text
diff --git a/fpdfsdk/cpdfsdk_widget.cpp b/fpdfsdk/cpdfsdk_widget.cpp
index 0d765ed947..6d81829bc9 100644
--- a/fpdfsdk/cpdfsdk_widget.cpp
+++ b/fpdfsdk/cpdfsdk_widget.cpp
@@ -41,17 +41,6 @@
 #include "xfa/fxfa/parser/cxfa_node.h"
 #endif  // PDF_ENABLE_XFA
 
-namespace {
-
-// Convert a FX_ARGB to a FX_COLORREF.
-FX_COLORREF ARGBToColorRef(FX_ARGB argb) {
-  return (((static_cast<uint32_t>(argb) & 0x00FF0000) >> 16) |
-          (static_cast<uint32_t>(argb) & 0x0000FF00) |
-          ((static_cast<uint32_t>(argb) & 0x000000FF) << 16));
-}
-
-}  // namespace
-
 CPDFSDK_Widget::CPDFSDK_Widget(CPDF_Annot* pAnnot,
                                CPDFSDK_PageView* pPageView,
                                CPDFSDK_InterForm* pInterForm)
@@ -428,14 +417,14 @@ WideString CPDFSDK_Widget::GetName() const {
 bool CPDFSDK_Widget::GetFillColor(FX_COLORREF& color) const {
   CPDF_FormControl* pFormCtrl = GetFormControl();
   int iColorType = 0;
-  color = ARGBToColorRef(pFormCtrl->GetBackgroundColor(iColorType));
+  color = ArgbToColorRef(pFormCtrl->GetBackgroundColor(iColorType));
   return iColorType != CFX_Color::kTransparent;
 }
 
 bool CPDFSDK_Widget::GetBorderColor(FX_COLORREF& color) const {
   CPDF_FormControl* pFormCtrl = GetFormControl();
   int iColorType = 0;
-  color = ARGBToColorRef(pFormCtrl->GetBorderColor(iColorType));
+  color = ArgbToColorRef(pFormCtrl->GetBorderColor(iColorType));
   return iColorType != CFX_Color::kTransparent;
 }
 
@@ -448,7 +437,7 @@ bool CPDFSDK_Widget::GetTextColor(FX_COLORREF& color) const {
   FX_ARGB argb;
   int iColorType = CFX_Color::kTransparent;
   da.GetColor(argb, iColorType);
-  color = ARGBToColorRef(argb);
+  color = ArgbToColorRef(argb);
   return iColorType != CFX_Color::kTransparent;
 }
 
diff --git a/xfa/fxfa/parser/cxfa_stipple.cpp b/xfa/fxfa/parser/cxfa_stipple.cpp
index 6f15badac9..2c36e11353 100644
--- a/xfa/fxfa/parser/cxfa_stipple.cpp
+++ b/xfa/fxfa/parser/cxfa_stipple.cpp
@@ -61,10 +61,10 @@ void CXFA_Stipple::Draw(CXFA_Graphics* pGS,
   CXFA_Color* pColor = GetColorIfExists();
   FX_ARGB crColor = pColor ? pColor->GetValue() : CXFA_Color::kBlackColor;
 
-  int32_t a;
-  FX_COLORREF bgr;
-  std::tie(a, bgr) = ArgbToColorRef(crColor);
-  FX_ARGB cr = ArgbEncode(iRate * a / 100, bgr);
+  int32_t alpha;
+  FX_COLORREF colorref;
+  std::tie(alpha, colorref) = ArgbToAlphaAndColorRef(crColor);
+  FX_ARGB cr = ArgbEncode(iRate * alpha / 100, colorref);
 
   pGS->SaveGraphState();
   pGS->SetFillColor(CXFA_GEColor(cr));