Tidy CXFA_Color class and argument passing.

Remove friendship.
De-virtualize.
Nest enum inside class.
Make copy-assignable and pass by const ref.
Make pack better on 64-bits.

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

4 files changed, 97 insertions, 109 deletions

diff --git a/xfa/fxgraphics/cxfa_color.cpp b/xfa/fxgraphics/cxfa_color.cpp
index 79c0e5f794..731d144c41 100644
--- a/xfa/fxgraphics/cxfa_color.cpp
+++ b/xfa/fxgraphics/cxfa_color.cpp
@@ -6,41 +6,41 @@
 
 #include "xfa/fxgraphics/cxfa_color.h"
 
-CXFA_Color::CXFA_Color() : m_type(FX_COLOR_None) {}
+CXFA_Color::CXFA_Color() : m_type(Invalid) {}
 
-CXFA_Color::CXFA_Color(const FX_ARGB argb) {
-  Set(argb);
+CXFA_Color::CXFA_Color(const FX_ARGB argb) : m_type(Solid), m_argb(argb) {
+  m_pointer.pattern = nullptr;
 }
 
-CXFA_Color::CXFA_Color(CXFA_Pattern* pattern, const FX_ARGB argb) {
-  Set(pattern, argb);
+CXFA_Color::CXFA_Color(CXFA_Pattern* pattern, const FX_ARGB argb)
+    : m_type(Pattern), m_argb(argb) {
+  m_pointer.pattern = pattern;
 }
 
-CXFA_Color::CXFA_Color(CXFA_Shading* shading) {
-  Set(shading);
+CXFA_Color::CXFA_Color(CXFA_Shading* shading) : m_type(Shading), m_argb(0) {
+  m_pointer.shading = shading;
 }
 
-CXFA_Color::~CXFA_Color() {
-  m_type = FX_COLOR_None;
-}
-
-void CXFA_Color::Set(const FX_ARGB argb) {
-  m_type = FX_COLOR_Solid;
-  m_info.argb = argb;
-  m_info.pattern = nullptr;
-}
-
-void CXFA_Color::Set(CXFA_Pattern* pattern, const FX_ARGB argb) {
-  if (!pattern)
-    return;
-  m_type = FX_COLOR_Pattern;
-  m_info.argb = argb;
-  m_info.pattern = pattern;
-}
-
-void CXFA_Color::Set(CXFA_Shading* shading) {
-  if (!shading)
-    return;
-  m_type = FX_COLOR_Shading;
-  m_shading = shading;
+CXFA_Color::~CXFA_Color() {}
+
+CXFA_Color& CXFA_Color::operator=(const CXFA_Color& that) {
+  if (this != &that) {
+    m_type = that.m_type;
+    switch (m_type) {
+      case Solid:
+        m_argb = that.m_argb;
+        m_pointer.pattern = nullptr;
+        break;
+      case Pattern:
+        m_argb = that.m_argb;
+        m_pointer.pattern = that.m_pointer.pattern;
+        break;
+      case Shading:
+        m_argb = 0;
+        m_pointer.shading = that.m_pointer.shading;
+      default:
+        break;
+    }
+  }
+  return *this;
 }
diff --git a/xfa/fxgraphics/cxfa_color.h b/xfa/fxgraphics/cxfa_color.h
index f5bba27586..4ae790507b 100644
--- a/xfa/fxgraphics/cxfa_color.h
+++ b/xfa/fxgraphics/cxfa_color.h
@@ -8,36 +8,43 @@
 #define XFA_FXGRAPHICS_CXFA_COLOR_H_
 
 #include "core/fxge/fx_dib.h"
-#include "xfa/fxgraphics/cxfa_graphics.h"
 
 class CXFA_Pattern;
 class CXFA_Shading;
 
-enum { FX_COLOR_None = 0, FX_COLOR_Solid, FX_COLOR_Pattern, FX_COLOR_Shading };
-
 class CXFA_Color {
  public:
+  enum Type { Invalid, Solid, Pattern, Shading };
+
   CXFA_Color();
   explicit CXFA_Color(const FX_ARGB argb);
   explicit CXFA_Color(CXFA_Shading* shading);
   CXFA_Color(CXFA_Pattern* pattern, const FX_ARGB argb);
-  virtual ~CXFA_Color();
-
-  void Set(const FX_ARGB argb);
-  void Set(CXFA_Pattern* pattern, const FX_ARGB argb);
-  void Set(CXFA_Shading* shading);
+  ~CXFA_Color();
+
+  Type GetType() const { return m_type; }
+  FX_ARGB GetArgb() const {
+    ASSERT(m_type == Solid || m_type == Pattern);
+    return m_argb;
+  }
+  CXFA_Pattern* GetPattern() const {
+    ASSERT(m_type == Pattern);
+    return m_pointer.pattern;
+  }
+  CXFA_Shading* GetShading() const {
+    ASSERT(m_type == Shading);
+    return m_pointer.shading;
+  }
+
+  CXFA_Color& operator=(const CXFA_Color& that);
 
  private:
-  friend class CXFA_Graphics;
-
-  int32_t m_type;
+  Type m_type;
+  FX_ARGB m_argb;
   union {
-    struct {
-      FX_ARGB argb;
-      CXFA_Pattern* pattern;
-    } m_info;
-    CXFA_Shading* m_shading;
-  };
+    CXFA_Pattern* pattern;
+    CXFA_Shading* shading;
+  } m_pointer;
 };
 
 #endif  // XFA_FXGRAPHICS_CXFA_COLOR_H_
diff --git a/xfa/fxgraphics/cxfa_graphics.cpp b/xfa/fxgraphics/cxfa_graphics.cpp
index 8648abb81d..981c288b46 100644
--- a/xfa/fxgraphics/cxfa_graphics.cpp
+++ b/xfa/fxgraphics/cxfa_graphics.cpp
@@ -165,20 +165,14 @@ void CXFA_Graphics::SetLineWidth(float lineWidth, bool isActOnDash) {
   }
 }
 
-void CXFA_Graphics::SetStrokeColor(CXFA_Color* color) {
-  if (!color)
-    return;
-  if (m_type == FX_CONTEXT_Device && m_renderDevice) {
+void CXFA_Graphics::SetStrokeColor(const CXFA_Color& color) {
+  if (m_type == FX_CONTEXT_Device && m_renderDevice)
     m_info.strokeColor = color;
-  }
 }
 
-void CXFA_Graphics::SetFillColor(CXFA_Color* color) {
-  if (!color)
-    return;
-  if (m_type == FX_CONTEXT_Device && m_renderDevice) {
+void CXFA_Graphics::SetFillColor(const CXFA_Color& color) {
+  if (m_type == FX_CONTEXT_Device && m_renderDevice)
     m_info.fillColor = color;
-  }
 }
 
 void CXFA_Graphics::StrokePath(CXFA_Path* path, CFX_Matrix* matrix) {
@@ -273,44 +267,33 @@ void CXFA_Graphics::RenderDeviceSetLineDash(FX_DashStyle dashStyle) {
 
 void CXFA_Graphics::RenderDeviceStrokePath(CXFA_Path* path,
                                            CFX_Matrix* matrix) {
-  if (!m_info.strokeColor)
+  if (m_info.strokeColor.GetType() != CXFA_Color::Solid)
     return;
-  CFX_Matrix m(m_info.CTM.a, m_info.CTM.b, m_info.CTM.c, m_info.CTM.d,
-               m_info.CTM.e, m_info.CTM.f);
-  if (matrix) {
+
+  CFX_Matrix m = m_info.CTM;
+  if (matrix)
     m.Concat(*matrix);
-  }
-  switch (m_info.strokeColor->m_type) {
-    case FX_COLOR_Solid: {
-      m_renderDevice->DrawPath(path->GetPathData(), &m, &m_info.graphState, 0x0,
-                               m_info.strokeColor->m_info.argb, 0);
-      return;
-    }
-    default:
-      return;
-  }
+
+  m_renderDevice->DrawPath(path->GetPathData(), &m, &m_info.graphState, 0x0,
+                           m_info.strokeColor.GetArgb(), 0);
 }
 
 void CXFA_Graphics::RenderDeviceFillPath(CXFA_Path* path,
                                          FX_FillMode fillMode,
                                          CFX_Matrix* matrix) {
-  if (!m_info.fillColor)
-    return;
-  CFX_Matrix m(m_info.CTM.a, m_info.CTM.b, m_info.CTM.c, m_info.CTM.d,
-               m_info.CTM.e, m_info.CTM.f);
-  if (matrix) {
+  CFX_Matrix m = m_info.CTM;
+  if (matrix)
     m.Concat(*matrix);
-  }
-  switch (m_info.fillColor->m_type) {
-    case FX_COLOR_Solid: {
+
+  switch (m_info.fillColor.GetType()) {
+    case CXFA_Color::Solid:
       m_renderDevice->DrawPath(path->GetPathData(), &m, &m_info.graphState,
-                               m_info.fillColor->m_info.argb, 0x0, fillMode);
+                               m_info.fillColor.GetArgb(), 0x0, fillMode);
       return;
-    }
-    case FX_COLOR_Pattern:
+    case CXFA_Color::Pattern:
       FillPathWithPattern(path, fillMode, &m);
       return;
-    case FX_COLOR_Shading:
+    case CXFA_Color::Shading:
       FillPathWithShading(path, fillMode, &m);
       return;
     default:
@@ -347,7 +330,7 @@ void CXFA_Graphics::RenderDeviceStretchImage(
 void CXFA_Graphics::FillPathWithPattern(CXFA_Path* path,
                                         FX_FillMode fillMode,
                                         CFX_Matrix* matrix) {
-  CXFA_Pattern* pattern = m_info.fillColor->m_info.pattern;
+  CXFA_Pattern* pattern = m_info.fillColor.GetPattern();
   CFX_RetainPtr<CFX_DIBitmap> bitmap = m_renderDevice->GetBitmap();
   int32_t width = bitmap->GetWidth();
   int32_t height = bitmap->GetHeight();
@@ -355,7 +338,7 @@ void CXFA_Graphics::FillPathWithPattern(CXFA_Path* path,
   bmp->Create(width, height, FXDIB_Argb);
   m_renderDevice->GetDIBits(bmp, 0, 0);
 
-  FX_HatchStyle hatchStyle = m_info.fillColor->m_info.pattern->m_hatchStyle;
+  FX_HatchStyle hatchStyle = m_info.fillColor.GetPattern()->m_hatchStyle;
   const FX_HATCHDATA& data = hatchBitmapData[static_cast<int>(hatchStyle)];
 
   auto mask = pdfium::MakeRetain<CFX_DIBitmap>();
@@ -369,12 +352,10 @@ void CXFA_Graphics::FillPathWithPattern(CXFA_Path* path,
                FXSYS_round(rectf.right), FXSYS_round(rectf.bottom));
   CFX_DefaultRenderDevice device;
   device.Attach(bmp, false, nullptr, false);
-  device.FillRect(&rect, m_info.fillColor->m_info.pattern->m_backArgb);
+  device.FillRect(&rect, m_info.fillColor.GetPattern()->m_backArgb);
   for (int32_t j = rect.bottom; j < rect.top; j += mask->GetHeight()) {
-    for (int32_t i = rect.left; i < rect.right; i += mask->GetWidth()) {
-      device.SetBitMask(mask, i, j,
-                        m_info.fillColor->m_info.pattern->m_foreArgb);
-    }
+    for (int32_t i = rect.left; i < rect.right; i += mask->GetWidth())
+      device.SetBitMask(mask, i, j, m_info.fillColor.GetPattern()->m_foreArgb);
   }
   CFX_RenderDevice::StateRestorer restorer(m_renderDevice);
   m_renderDevice->SetClip_PathFill(path->GetPathData(), matrix, fillMode);
@@ -387,16 +368,16 @@ void CXFA_Graphics::FillPathWithShading(CXFA_Path* path,
   CFX_RetainPtr<CFX_DIBitmap> bitmap = m_renderDevice->GetBitmap();
   int32_t width = bitmap->GetWidth();
   int32_t height = bitmap->GetHeight();
-  float start_x = m_info.fillColor->m_shading->m_beginPoint.x;
-  float start_y = m_info.fillColor->m_shading->m_beginPoint.y;
-  float end_x = m_info.fillColor->m_shading->m_endPoint.x;
-  float end_y = m_info.fillColor->m_shading->m_endPoint.y;
+  float start_x = m_info.fillColor.GetShading()->m_beginPoint.x;
+  float start_y = m_info.fillColor.GetShading()->m_beginPoint.y;
+  float end_x = m_info.fillColor.GetShading()->m_endPoint.x;
+  float end_y = m_info.fillColor.GetShading()->m_endPoint.y;
   auto bmp = pdfium::MakeRetain<CFX_DIBitmap>();
   bmp->Create(width, height, FXDIB_Argb);
   m_renderDevice->GetDIBits(bmp, 0, 0);
   int32_t pitch = bmp->GetPitch();
   bool result = false;
-  switch (m_info.fillColor->m_shading->m_type) {
+  switch (m_info.fillColor.GetShading()->m_type) {
     case FX_SHADING_Axial: {
       float x_span = end_x - start_x;
       float y_span = end_y - start_y;
@@ -409,26 +390,26 @@ void CXFA_Graphics::FillPathWithShading(CXFA_Path* path,
           float scale = (((x - start_x) * x_span) + ((y - start_y) * y_span)) /
                         axis_len_square;
           if (scale < 0) {
-            if (!m_info.fillColor->m_shading->m_isExtendedBegin) {
+            if (!m_info.fillColor.GetShading()->m_isExtendedBegin) {
               continue;
             }
             scale = 0;
           } else if (scale > 1.0f) {
-            if (!m_info.fillColor->m_shading->m_isExtendedEnd) {
+            if (!m_info.fillColor.GetShading()->m_isExtendedEnd) {
               continue;
             }
             scale = 1.0f;
           }
           int32_t index = (int32_t)(scale * (FX_SHADING_Steps - 1));
-          dib_buf[column] = m_info.fillColor->m_shading->m_argbArray[index];
+          dib_buf[column] = m_info.fillColor.GetShading()->m_argbArray[index];
         }
       }
       result = true;
       break;
     }
     case FX_SHADING_Radial: {
-      float start_r = m_info.fillColor->m_shading->m_beginRadius;
-      float end_r = m_info.fillColor->m_shading->m_endRadius;
+      float start_r = m_info.fillColor.GetShading()->m_beginRadius;
+      float end_r = m_info.fillColor.GetShading()->m_endRadius;
       float a = ((start_x - end_x) * (start_x - end_x)) +
                 ((start_y - end_y) * (start_y - end_y)) -
                 ((start_r - end_r) * (start_r - end_r));
@@ -459,7 +440,7 @@ void CXFA_Graphics::FillPathWithShading(CXFA_Path* path,
               s2 = (-b - root) / (2 * a);
               s1 = (-b + root) / (2 * a);
             }
-            if (s2 <= 1.0f || m_info.fillColor->m_shading->m_isExtendedEnd) {
+            if (s2 <= 1.0f || m_info.fillColor.GetShading()->m_isExtendedEnd) {
               s = (s2);
             } else {
               s = (s1);
@@ -469,19 +450,19 @@ void CXFA_Graphics::FillPathWithShading(CXFA_Path* path,
             }
           }
           if (s < 0) {
-            if (!m_info.fillColor->m_shading->m_isExtendedBegin) {
+            if (!m_info.fillColor.GetShading()->m_isExtendedBegin) {
               continue;
             }
             s = 0;
           }
           if (s > 1.0f) {
-            if (!m_info.fillColor->m_shading->m_isExtendedEnd) {
+            if (!m_info.fillColor.GetShading()->m_isExtendedEnd) {
               continue;
             }
             s = 1.0f;
           }
           int index = (int32_t)(s * (FX_SHADING_Steps - 1));
-          dib_buf[column] = m_info.fillColor->m_shading->m_argbArray[index];
+          dib_buf[column] = m_info.fillColor.GetShading()->m_argbArray[index];
         }
       }
       result = true;
diff --git a/xfa/fxgraphics/cxfa_graphics.h b/xfa/fxgraphics/cxfa_graphics.h
index 5492d4d3be..8069c8ecbd 100644
--- a/xfa/fxgraphics/cxfa_graphics.h
+++ b/xfa/fxgraphics/cxfa_graphics.h
@@ -16,8 +16,8 @@
 #include "core/fxge/cfx_renderdevice.h"
 #include "core/fxge/fx_dib.h"
 #include "core/fxge/fx_font.h"
+#include "xfa/fxgraphics/cxfa_color.h"
 
-class CXFA_Color;
 class CXFA_Path;
 
 using FX_FillMode = int32_t;
@@ -57,8 +57,8 @@ class CXFA_Graphics {
   void SetLineDash(float dashPhase, float* dashArray, int32_t dashCount);
   void SetLineDash(FX_DashStyle dashStyle);
   void SetLineWidth(float lineWidth, bool isActOnDash = false);
-  void SetStrokeColor(CXFA_Color* color);
-  void SetFillColor(CXFA_Color* color);
+  void SetStrokeColor(const CXFA_Color& color);
+  void SetFillColor(const CXFA_Color& color);
   void SetClipRect(const CFX_RectF& rect);
   void StrokePath(CXFA_Path* path, CFX_Matrix* matrix = nullptr);
   void FillPath(CXFA_Path* path,
@@ -81,8 +81,8 @@ class CXFA_Graphics {
     CFX_GraphStateData graphState;
     CFX_Matrix CTM;
     bool isActOnDash;
-    CXFA_Color* strokeColor;
-    CXFA_Color* fillColor;
+    CXFA_Color strokeColor;
+    CXFA_Color fillColor;
   } m_info;
 
   void RenderDeviceSetLineDash(FX_DashStyle dashStyle);