Make Document::m_IconList a vector of IconElements. (try 2)chromium/2896

It does not need to be a std::list. This time keep the elements as
unique_ptrs.

Previous attempt: https://codereview.chromium.org/2428743004/

Review-Url: https://chromiumcodereview.appspot.com/2428373004

2 files changed, 33 insertions, 31 deletions

diff --git a/fpdfsdk/javascript/Document.cpp b/fpdfsdk/javascript/Document.cpp
index 0b08f66f33..0c2e3f7399 100644
--- a/fpdfsdk/javascript/Document.cpp
+++ b/fpdfsdk/javascript/Document.cpp
@@ -33,6 +33,7 @@
 #include "fpdfsdk/javascript/cjs_runtime.h"
 #include "fpdfsdk/javascript/resource.h"
 #include "third_party/base/numerics/safe_math.h"
+#include "third_party/base/ptr_util.h"
 
 BEGIN_JS_STATIC_CONST(CJS_PrintParamsObj)
 END_JS_STATIC_CONST()
@@ -1256,8 +1257,8 @@ FX_BOOL Document::addIcon(IJS_Context* cc,
     return FALSE;
   }
 
-  m_IconList.push_back(std::unique_ptr<IconElement>(
-      new IconElement(swIconName, (Icon*)pEmbedObj)));
+  m_Icons.push_back(pdfium::MakeUnique<IconElement>(
+      swIconName, static_cast<Icon*>(pEmbedObj)));
   return TRUE;
 }
 
@@ -1270,7 +1271,7 @@ FX_BOOL Document::icons(IJS_Context* cc,
   }
 
   CJS_Runtime* pRuntime = CJS_Runtime::FromContext(cc);
-  if (m_IconList.empty()) {
+  if (m_Icons.empty()) {
     vp.GetJSValue()->SetNull(pRuntime);
     return TRUE;
   }
@@ -1278,7 +1279,7 @@ FX_BOOL Document::icons(IJS_Context* cc,
   CJS_Array Icons;
 
   int i = 0;
-  for (const auto& pIconElement : m_IconList) {
+  for (const auto& pIconElement : m_Icons) {
     v8::Local<v8::Object> pObj =
         pRuntime->NewFxDynamicObj(CJS_Icon::g_nObjDefnID);
     if (pObj.IsEmpty())
@@ -1289,7 +1290,7 @@ FX_BOOL Document::icons(IJS_Context* cc,
     if (!pJS_Icon)
       return FALSE;
 
-    Icon* pIcon = (Icon*)pJS_Icon->GetEmbedObject();
+    Icon* pIcon = static_cast<Icon*>(pJS_Icon->GetEmbedObject());
     if (!pIcon)
       return FALSE;
 
@@ -1311,35 +1312,35 @@ FX_BOOL Document::getIcon(IJS_Context* cc,
     return FALSE;
   }
 
-  if (m_IconList.empty())
+  if (m_Icons.empty())
     return FALSE;
 
   CJS_Runtime* pRuntime = CJS_Runtime::FromContext(cc);
   CFX_WideString swIconName = params[0].ToCFXWideString(pRuntime);
 
-  for (const auto& pIconElement : m_IconList) {
-    if (pIconElement->IconName == swIconName) {
-      Icon* pRetIcon = pIconElement->IconStream;
+  for (const auto& pIconElement : m_Icons) {
+    if (pIconElement->IconName != swIconName)
+      continue;
 
-      v8::Local<v8::Object> pObj =
-          pRuntime->NewFxDynamicObj(CJS_Icon::g_nObjDefnID);
-      if (pObj.IsEmpty())
-        return FALSE;
+    v8::Local<v8::Object> pObj =
+        pRuntime->NewFxDynamicObj(CJS_Icon::g_nObjDefnID);
+    if (pObj.IsEmpty())
+      return FALSE;
 
-      CJS_Icon* pJS_Icon =
-          static_cast<CJS_Icon*>(pRuntime->GetObjectPrivate(pObj));
-      if (!pJS_Icon)
-        return FALSE;
+    CJS_Icon* pJS_Icon =
+        static_cast<CJS_Icon*>(pRuntime->GetObjectPrivate(pObj));
+    if (!pJS_Icon)
+      return FALSE;
 
-      Icon* pIcon = (Icon*)pJS_Icon->GetEmbedObject();
-      if (!pIcon)
-        return FALSE;
+    Icon* pIcon = (Icon*)pJS_Icon->GetEmbedObject();
+    if (!pIcon)
+      return FALSE;
 
-      pIcon->SetIconName(swIconName);
-      pIcon->SetStream(pRetIcon->GetStream());
-      vRet = CJS_Value(pRuntime, pJS_Icon);
-      return TRUE;
-    }
+    pIcon->SetIconName(swIconName);
+    pIcon->SetStream(pIconElement->IconStream->GetStream());
+
+    vRet = CJS_Value(pRuntime, pJS_Icon);
+    return TRUE;
   }
 
   return FALSE;
diff --git a/fpdfsdk/javascript/Document.h b/fpdfsdk/javascript/Document.h
index a72316c3d1..129411a431 100644
--- a/fpdfsdk/javascript/Document.h
+++ b/fpdfsdk/javascript/Document.h
@@ -18,7 +18,7 @@
 
 class PrintParamsObj : public CJS_EmbedObj {
  public:
-  PrintParamsObj(CJS_Object* pJSObject);
+  explicit PrintParamsObj(CJS_Object* pJSObject);
   ~PrintParamsObj() override {}
 
  public:
@@ -34,7 +34,8 @@ class PrintParamsObj : public CJS_EmbedObj {
 
 class CJS_PrintParamsObj : public CJS_Object {
  public:
-  CJS_PrintParamsObj(v8::Local<v8::Object> pObject) : CJS_Object(pObject) {}
+  explicit CJS_PrintParamsObj(v8::Local<v8::Object> pObject)
+      : CJS_Object(pObject) {}
   ~CJS_PrintParamsObj() override {}
 
   DECLARE_JS_CLASS();
@@ -47,8 +48,8 @@ struct IconElement {
   IconElement(const CFX_WideString& name, Icon* stream)
       : IconName(name), IconStream(stream) {}
 
-  CFX_WideString IconName;
-  Icon* IconStream;
+  const CFX_WideString IconName;
+  Icon* const IconStream;
 };
 
 struct CJS_DelayData;
@@ -57,7 +58,7 @@ struct CJS_AnnotObj;
 
 class Document : public CJS_EmbedObj {
  public:
-  Document(CJS_Object* pJSObject);
+  explicit Document(CJS_Object* pJSObject);
   ~Document() override;
 
   FX_BOOL ADBE(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError);
@@ -291,7 +292,7 @@ class Document : public CJS_EmbedObj {
   CPDFSDK_FormFillEnvironment::ObservedPtr m_pFormFillEnv;
   CFX_WideString m_cwBaseURL;
   std::list<std::unique_ptr<CJS_DelayData>> m_DelayData;
-  std::list<std::unique_ptr<IconElement>> m_IconList;
+  std::vector<std::unique_ptr<IconElement>> m_Icons;
   bool m_bDelay;
 };