Cleanup CPDF_DIBSource::LoadColorInfo() and ValidateDictParam().

core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "render_int.h"
 
 #include <vector>
 
@@ skipping 447 matching lines @@
     if (m_pColorSpace && m_bStdCS) {
       m_pColorSpace->EnableStdConversion(FALSE);
     }
     return ret1;
   }
   if (m_Status == 2) {
     return ContinueLoadMaskDIB(pPause);
   }
   return 0;
 }
-FX_BOOL CPDF_DIBSource::LoadColorInfo(CPDF_Dictionary* pFormResources,
-                                      CPDF_Dictionary* pPageResources) {
-  m_bpc_orig = m_pDict->GetInteger(FX_BSTRC("BitsPerComponent"));
-  if (m_pDict->GetInteger("ImageMask")) {
+
+bool CPDF_DIBSource::LoadColorInfo(const CPDF_Dictionary* pFormResources,
+                                   const CPDF_Dictionary* pPageResources) {
+  m_bpc_orig = m_pDict->GetInteger("BitsPerComponent");
+  if (m_pDict->GetInteger("ImageMask"))
     m_bImageMask = TRUE;
-  }
-  if (m_bImageMask || !m_pDict->KeyExist(FX_BSTRC("ColorSpace"))) {
+
+  if (m_bImageMask || !m_pDict->KeyExist("ColorSpace")) {
     if (!m_bImageMask) {
-      CPDF_Object* pFilter = m_pDict->GetElementValue(FX_BSTRC("Filter"));
+      CPDF_Object* pFilter = m_pDict->GetElementValue("Filter");
       if (pFilter) {
         CFX_ByteString filter;
         if (pFilter->IsName()) {
           filter = pFilter->GetString();
-          if (filter == FX_BSTRC("JPXDecode")) {
-            m_bDoBpcCheck = FALSE;
-            return TRUE;
-          }
         } else if (CPDF_Array* pArray = pFilter->AsArray()) {
-          if (pArray->GetString(pArray->GetCount() - 1) ==
-              FX_BSTRC("JPXDecode")) {
-            m_bDoBpcCheck = FALSE;
-            return TRUE;
-          }
+          filter = pArray->GetString(pArray->GetCount() - 1);
+        }
+
+        if (filter == FX_BSTRC("JPXDecode")) {
+          m_bDoBpcCheck = FALSE;
+          return true;
         }
       }
     }
     m_bImageMask = TRUE;
     m_bpc = m_nComponents = 1;
-    CPDF_Array* pDecode = m_pDict->GetArray(FX_BSTRC("Decode"));
-    m_bDefaultDecode = pDecode == NULL || pDecode->GetInteger(0) == 0;
-    return TRUE;
+    CPDF_Array* pDecode = m_pDict->GetArray("Decode");
+    m_bDefaultDecode = !pDecode || !pDecode->GetInteger(0);
+    return true;
   }
-  CPDF_Object* pCSObj = m_pDict->GetElementValue(FX_BSTRC("ColorSpace"));
-  if (pCSObj == NULL) {
-    return FALSE;
-  }
+
+  CPDF_Object* pCSObj = m_pDict->GetElementValue("ColorSpace");
+  if (!pCSObj)
+    return false;
+
   CPDF_DocPageData* pDocPageData = m_pDocument->GetPageData();
-  if (pFormResources) {
+  if (pFormResources)
     m_pColorSpace = pDocPageData->GetColorSpace(pCSObj, pFormResources);
-  }
-  if (m_pColorSpace == NULL) {
+  if (!m_pColorSpace)
     m_pColorSpace = pDocPageData->GetColorSpace(pCSObj, pPageResources);
-  }
-  if (m_pColorSpace == NULL) {
-    return FALSE;
-  }
+  if (!m_pColorSpace)
+    return false;
+
   m_Family = m_pColorSpace->GetFamily();
   m_nComponents = m_pColorSpace->CountComponents();
   if (m_Family == PDFCS_ICCBASED && pCSObj->IsName()) {
     CFX_ByteString cs = pCSObj->GetString();
-    if (cs == FX_BSTRC("DeviceGray")) {
+    if (cs == "DeviceGray") {
       m_nComponents = 1;
-    } else if (cs == FX_BSTRC("DeviceRGB")) {
+    } else if (cs == "DeviceRGB") {
       m_nComponents = 3;
-    } else if (cs == FX_BSTRC("DeviceCMYK")) {
+    } else if (cs == "DeviceCMYK") {
       m_nComponents = 4;
     }
   }
   ValidateDictParam();
   m_pCompData = GetDecodeAndMaskArray(m_bDefaultDecode, m_bColorKey);
-  if (m_pCompData == NULL) {
-    return FALSE;
-  }
-  return TRUE;
+  return !!m_pCompData;
 }
+
 DIB_COMP_DATA* CPDF_DIBSource::GetDecodeAndMaskArray(FX_BOOL& bDefaultDecode,
                                                      FX_BOOL& bColorKey) {
   if (m_pColorSpace == NULL) {
     return NULL;
   }
   DIB_COMP_DATA* pCompData = FX_Alloc(DIB_COMP_DATA, m_nComponents);
   int max_data = (1 << m_bpc) - 1;
   CPDF_Array* pDecode = m_pDict->GetArray(FX_BSTRC("Decode"));
   if (pDecode) {
     for (FX_DWORD i = 0; i < m_nComponents; i++) {
@@ skipping 374 matching lines @@
         m_pColorSpace->GetRGB(temp_buf, R, G, B);
         FX_Free(temp_buf);
       } else {
         m_pColorSpace->GetRGB(color_value, R, G, B);
       }
       SetPaletteArgb(i, ArgbEncode(255, FXSYS_round(R * 255),
                                    FXSYS_round(G * 255), FXSYS_round(B * 255)));
     }
   }
 }
+
 void CPDF_DIBSource::ValidateDictParam() {
   m_bpc = m_bpc_orig;
-  CPDF_Object* pFilter = m_pDict->GetElementValue(FX_BSTRC("Filter"));
+  CPDF_Object* pFilter = m_pDict->GetElementValue("Filter");
   if (pFilter) {
     if (pFilter->IsName()) {
       CFX_ByteString filter = pFilter->GetString();
-      if (filter == FX_BSTRC("CCITTFaxDecode") ||
-          filter == FX_BSTRC("JBIG2Decode")) {
+      if (filter == "CCITTFaxDecode" || filter == "JBIG2Decode") {
         m_bpc = 1;
         m_nComponents = 1;
-      }
-      if (filter == FX_BSTRC("RunLengthDecode")) {
+      } else if (filter == "RunLengthDecode") {
         if (m_bpc != 1) {
           m_bpc = 8;
         }
-      } else if (filter == FX_BSTRC("DCTDecode")) {
+      } else if (filter == "DCTDecode") {
         m_bpc = 8;
       }
     } else if (CPDF_Array* pArray = pFilter->AsArray()) {
-      if (pArray->GetString(pArray->GetCount() - 1) ==
-              FX_BSTRC("CCITTFaxDecode") ||
-          pArray->GetString(pArray->GetCount() - 1) ==
-              FX_BSTRC("JBIG2Decode")) {
+      CFX_ByteString filter = pArray->GetString(pArray->GetCount() - 1);
+      if (filter == "CCITTFaxDecode" || filter == "JBIG2Decode") {
         m_bpc = 1;
         m_nComponents = 1;
-      }
-      if (pArray->GetString(pArray->GetCount() - 1) == FX_BSTRC("DCTDecode")) {
-        // Previously, pArray->GetString(pArray->GetCount() - 1) ==
-        // FX_BSTRC("RunLengthDecode") was checked in the "if" statement as
-        // well,
-        // but too many documents don't conform to it.
+      } else if (filter == "DCTDecode") {
+        // Previously, filter == "RunLengthDecode" was checked in the "if"
+        // statement as well, but too many documents don't conform to it.
         m_bpc = 8;
       }
     }
   }
-  if (m_bpc != 1 && m_bpc != 2 && m_bpc != 4 && m_bpc != 8 && m_bpc != 16) {
-    m_bpc = 0;
+  switch (m_bpc) {

[Tom Sepez, 2015/12/09 19:27:26]

nit: not sure this is an improvement.  Maybe we need inline helper function
IsAllowedBPCValue(bpd) { return bpc == 1 || bpc ==2 || ... }
and here
if (!IsAllowedBPCValue())
  m_pbp = 0;

or some such.  Clever folks might realize that they are all powers of two and
write bpc > 0 && bpc <= 16 && !(bpc & (bpc - 1)) but that is confusing.

[Lei Zhang, 2015/12/10 08:02:32]

Done.

+    case 1:
+    case 2:
+    case 4:
+    case 8:
+    case 16:
+      break;
+    default:
+      m_bpc = 0;
+      break;
   }
 }
+
 #define NORMALCOLOR_MAX(color, max) \
   (color) > (max) ? (max) : (color) < 0 ? 0 : (color);
 void CPDF_DIBSource::TranslateScanline24bpp(uint8_t* dest_scan,
                                             const uint8_t* src_scan) const {
   if (m_bpc == 0) {
     return;
   }
   int max_data = (1 << m_bpc) - 1;
   if (m_bDefaultDecode) {
     if (m_Family == PDFCS_DEVICERGB || m_Family == PDFCS_CALRGB) {
@@ skipping 674 matching lines @@
 }
 FX_BOOL CPDF_ImageLoader::Continue(void* LoadHandle, IFX_Pause* pPause) {
   return ((CPDF_ProgressiveImageLoaderHandle*)LoadHandle)->Continue(pPause);
 }
 CPDF_ImageLoader::~CPDF_ImageLoader() {
   if (!m_bCached) {
     delete m_pBitmap;
     delete m_pMask;
   }
 }