commit 891aaf63b3490989a189bcc51b18647073f988d4
author Lei Zhang <thestig@chromium.org> Wed Dec 20 15:18:26 2017 -0800
committer Chromium commit bot <commit-bot@chromium.org> Thu Dec 21 19:28:25 2017 +0000
tree caeb000b1bf284bd6c73b70aa2953cfd071ebae0
parent f2af8326501d72bf3fb23f0c62ade28310836ba9

Clean up CPDF_Color.

- Add IsPatternInternal().
- Mark some method parameters const.
- Explain relationship between member variables.

Change-Id: I650e18012341199985a4537e08c787e46cf8fef7
Reviewed-on: https://pdfium-review.googlesource.com/21912
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>

core/fpdfapi/page/cpdf_color.cpp

diff --git a/core/fpdfapi/page/cpdf_color.cpp b/core/fpdfapi/page/cpdf_color.cpp
index 471d7ee..d364e8e 100644
--- a/core/fpdfapi/page/cpdf_color.cpp
+++ b/core/fpdfapi/page/cpdf_color.cpp
@@ -11,7 +11,7 @@
 #include "core/fpdfapi/parser/cpdf_document.h"
 #include "core/fxcrt/fx_system.h"
 
-CPDF_Color::CPDF_Color() : m_pCS(nullptr), m_pBuffer(nullptr) {}
+CPDF_Color::CPDF_Color() {}
 
 CPDF_Color::~CPDF_Color() {
   ReleaseBuffer();
@@ -19,15 +19,15 @@
 }
 
 bool CPDF_Color::IsPattern() const {
-  return m_pCS && m_pCS->GetFamily() == PDFCS_PATTERN;
+  return m_pCS && IsPatternInternal();
 }
 
 void CPDF_Color::ReleaseBuffer() {
   if (!m_pBuffer)
     return;
 
-  if (m_pCS->GetFamily() == PDFCS_PATTERN) {
-    PatternValue* pvalue = (PatternValue*)m_pBuffer;
+  if (IsPatternInternal()) {
+    PatternValue* pvalue = reinterpret_cast<PatternValue*>(m_pBuffer);
     CPDF_Pattern* pPattern =
         pvalue->m_pCountedPattern ? pvalue->m_pCountedPattern->get() : nullptr;
     if (pPattern) {
@@ -55,6 +55,10 @@
   m_pCS = nullptr;
 }
 
+bool CPDF_Color::IsPatternInternal() const {
+  return m_pCS->GetFamily() == PDFCS_PATTERN;
+}
+
 void CPDF_Color::SetColorSpace(CPDF_ColorSpace* pCS) {
   if (m_pCS == pCS) {
     if (!m_pBuffer)
@@ -68,21 +72,21 @@
   ReleaseColorSpace();
 
   m_pCS = pCS;
-  if (m_pCS) {
+  if (pCS) {
     m_pBuffer = pCS->CreateBuf();
     pCS->GetDefaultColor(m_pBuffer);
   }
 }
 
-void CPDF_Color::SetValue(float* comps) {
+void CPDF_Color::SetValue(const float* comps) {
   if (!m_pBuffer)
     return;
-  if (m_pCS->GetFamily() != PDFCS_PATTERN)
+  if (!IsPatternInternal())
     memcpy(m_pBuffer, comps, m_pCS->CountComponents() * sizeof(float));
 }
 
 void CPDF_Color::SetValue(CPDF_Pattern* pPattern,
-                          float* comps,
+                          const float* comps,
                           uint32_t ncomps) {
   if (ncomps > kMaxPatternColorComps)
     return;
@@ -94,7 +98,7 @@
   }
 
   CPDF_DocPageData* pDocPageData = nullptr;
-  PatternValue* pvalue = (PatternValue*)m_pBuffer;
+  PatternValue* pvalue = reinterpret_cast<PatternValue*>(m_pBuffer);
   if (pvalue->m_pPattern) {
     pDocPageData = pvalue->m_pPattern->document()->GetPageData();
     pDocPageData->ReleasePattern(pvalue->m_pPattern->pattern_obj());
@@ -130,7 +134,7 @@
   }
   m_pBuffer = m_pCS->CreateBuf();
   memcpy(m_pBuffer, pSrc->m_pBuffer, m_pCS->GetBufSize());
-  if (m_pCS->GetFamily() != PDFCS_PATTERN)
+  if (!IsPatternInternal())
     return;
 
   PatternValue* pValue = reinterpret_cast<PatternValue*>(m_pBuffer);
@@ -159,7 +163,7 @@
 }
 
 CPDF_Pattern* CPDF_Color::GetPattern() const {
-  if (!m_pBuffer || m_pCS->GetFamily() != PDFCS_PATTERN)
+  if (!m_pBuffer || !IsPatternInternal())
     return nullptr;
 
   PatternValue* pvalue = reinterpret_cast<PatternValue*>(m_pBuffer);

core/fpdfapi/page/cpdf_color.h

diff --git a/core/fpdfapi/page/cpdf_color.h b/core/fpdfapi/page/cpdf_color.h
index cb73528..2eac7cf 100644
--- a/core/fpdfapi/page/cpdf_color.h
+++ b/core/fpdfapi/page/cpdf_color.h
@@ -23,8 +23,8 @@
   void Copy(const CPDF_Color* pSrc);
 
   void SetColorSpace(CPDF_ColorSpace* pCS);
-  void SetValue(float* comp);
-  void SetValue(CPDF_Pattern* pPattern, float* comp, uint32_t ncomps);
+  void SetValue(const float* comp);
+  void SetValue(CPDF_Pattern* pPattern, const float* comp, uint32_t ncomps);
 
   bool GetRGB(int* R, int* G, int* B) const;
   CPDF_Pattern* GetPattern() const;
@@ -33,9 +33,13 @@
  protected:
   void ReleaseBuffer();
   void ReleaseColorSpace();
+  bool IsPatternInternal() const;
 
-  CPDF_ColorSpace* m_pCS;
-  float* m_pBuffer;
+  // TODO(thestig): Convert this to a smart pointer or vector.
+  // |m_pBuffer| is created by |m_pCS|, so if it is non-null, then so is
+  // |m_pCS|.
+  float* m_pBuffer = nullptr;
+  CPDF_ColorSpace* m_pCS = nullptr;
 };
 
 #endif  // CORE_FPDFAPI_PAGE_CPDF_COLOR_H_