Complete or comment a couple Script_NodeClass functions

xfa/fxfa/parser/xfa_object_imp.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 "xfa/fxfa/parser/xfa_object.h"
 
 #include <memory>
 
@@ skipping 884 matching lines @@
   }
   if (iLength >= 2) {
     iValue = pArguments->GetInt32(1);
   }
   const XFA_ELEMENTINFO* pElementInfo =
       XFA_GetElementByName(wsExpression.AsStringC());
   CXFA_Node* pNode = GetProperty(iValue, pElementInfo->eName);
   FXJSE_Value_Set(pArguments->GetReturnValue(),
                   m_pDocument->GetScriptContext()->GetJSValueFromMap(pNode));
 }
+
 void CXFA_Node::Script_NodeClass_IsPropertySpecified(
     CFXJSE_Arguments* pArguments) {
   int32_t iLength = pArguments->GetLength();
   if (iLength < 1 || iLength > 3) {
     ThrowScriptErrorMessage(XFA_IDS_INCORRECT_NUMBER_OF_METHOD,
                             L"isPropertySpecified");
     return;
   }
   CFX_WideString wsExpression;
   FX_BOOL bParent = TRUE;
   int32_t iIndex = 0;
-  if (iLength >= 1) {
-    CFX_ByteString bsExpression = pArguments->GetUTF8String(0);
-    wsExpression = CFX_WideString::FromUTF8(bsExpression.AsStringC());
-  }
-  if (iLength >= 2) {
-    bParent = pArguments->GetInt32(1) == 0 ? FALSE : TRUE;
-  }
-  if (iLength >= 3) {
+  if (iLength >= 1)

[Tom Sepez, 2016/05/25 17:50:29]

nit {} needed because of line break in next stmt.

[Wei Li, 2016/05/25 18:13:18]

Done.

+    wsExpression =
+        CFX_WideString::FromUTF8(pArguments->GetUTF8String(0).AsStringC());
+  if (iLength >= 2)
+    bParent = pArguments->GetInt32(1) ? TRUE : FALSE;

[Tom Sepez, 2016/05/25 17:50:29]

nit: prefer !!pArguments->GetInt32(1);

[Wei Li, 2016/05/25 18:13:18]

Done.

+  if (iLength >= 3)
     iIndex = pArguments->GetInt32(2);
-  }
   FX_BOOL bHas = FALSE;
   const XFA_ATTRIBUTEINFO* pAttributeInfo =
       XFA_GetAttributeByName(wsExpression.AsStringC());
   CFX_WideString wsValue;
-  if (pAttributeInfo) {
+  if (pAttributeInfo)
     bHas = HasAttribute(pAttributeInfo->eName);
-  }
   if (!bHas) {
     const XFA_ELEMENTINFO* pElementInfo =
         XFA_GetElementByName(wsExpression.AsStringC());
-    bHas = (GetProperty(iIndex, pElementInfo->eName) != NULL);
+    bHas = GetProperty(iIndex, pElementInfo->eName) != nullptr;

[Tom Sepez, 2016/05/25 17:50:29]

nit: again !! might be cleaner.

[Wei Li, 2016/05/25 18:13:18]

Done.

+    if (!bHas && bParent && m_pParent) {

[Tom Sepez, 2016/05/25 17:50:29]

Where did the idea for this addition come from?  Can we talk about it in more
detail in the change description.

[Wei Li, 2016/05/25 18:13:18]

Done.

+      // Also check on the parent.
+      bHas = m_pParent->HasAttribute(pAttributeInfo->eName);
+      if (!bHas)
+        bHas = m_pParent->GetProperty(iIndex, pElementInfo->eName) != nullptr;

[Tom Sepez, 2016/05/25 17:50:29]

nit and here.

[Wei Li, 2016/05/25 18:13:18]

Done.

+    }
   }
   FXJSE_HVALUE hValue = pArguments->GetReturnValue();
-  if (hValue) {
+  if (hValue)
     FXJSE_Value_SetBoolean(hValue, bHas);
-  }
 }
+
 void CXFA_Node::Script_NodeClass_LoadXML(CFXJSE_Arguments* pArguments) {
   int32_t iLength = pArguments->GetLength();
   if (iLength < 1 || iLength > 3) {
     ThrowScriptErrorMessage(XFA_IDS_INCORRECT_NUMBER_OF_METHOD, L"loadXML");
     return;
   }
   CFX_WideString wsExpression;
   FX_BOOL bIgnoreRoot = TRUE;
   FX_BOOL bOverwrite = 0;
   if (iLength >= 1) {
@@ skipping 118 matching lines @@
 }
 
 void CXFA_Node::Script_NodeClass_SaveXML(CFXJSE_Arguments* pArguments) {
   int32_t iLength = pArguments->GetLength();
   if (iLength < 0 || iLength > 1) {
     ThrowScriptErrorMessage(XFA_IDS_INCORRECT_NUMBER_OF_METHOD, L"saveXML");
     return;
   }
   FX_BOOL bPrettyMode = FALSE;
   if (iLength == 1) {
-    CFX_ByteString bsPretty = pArguments->GetUTF8String(0);
-    if (bsPretty != "pretty") {
+    if (pArguments->GetUTF8String(0) != "pretty") {
       ThrowScriptErrorMessage(XFA_IDS_ARGUMENT_MISMATCH);
       return;
     }
     bPrettyMode = TRUE;
   }
   CFX_ByteStringC bsXMLHeader = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n";
-  if (GetPacketID() == XFA_XDPPACKET_Form) {
-    IFX_MemoryStream* pMemoryStream = FX_CreateMemoryStream(TRUE);
-    IFX_Stream* pStream = IFX_Stream::CreateStream(
-        (IFX_FileWrite*)pMemoryStream,
-        FX_STREAMACCESS_Text | FX_STREAMACCESS_Write | FX_STREAMACCESS_Append);
+  if (GetPacketID() == XFA_XDPPACKET_Form ||
+      GetPacketID() == XFA_XDPPACKET_Datasets) {
+    CFDE_XMLNode* pElement = nullptr;
+    if (GetPacketID() == XFA_XDPPACKET_Datasets) {
+      pElement = GetXMLMappingNode();
+      if (!pElement || pElement->GetType() != FDE_XMLNODE_Element) {
+        FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(), bsXMLHeader);
+        return;
+      }
+      XFA_DataExporter_DealWithDataGroupNode(this);
+    }
+    std::unique_ptr<IFX_MemoryStream> pMemoryStream(
+        FX_CreateMemoryStream(TRUE));
+    std::unique_ptr<IFX_Stream> pStream(IFX_Stream::CreateStream(
+        (IFX_FileWrite*)pMemoryStream.get(),
+        FX_STREAMACCESS_Text | FX_STREAMACCESS_Write | FX_STREAMACCESS_Append));
     if (!pStream) {
       FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(), bsXMLHeader);
-      pMemoryStream->Release();
-      pMemoryStream = NULL;
       return;
     }
     pStream->SetCodePage(FX_CODEPAGE_UTF8);
     pStream->WriteData(bsXMLHeader.raw_str(), bsXMLHeader.GetLength());
-    XFA_DataExporter_RegenerateFormFile(this, pStream, NULL, TRUE);
+    if (GetPacketID() == XFA_XDPPACKET_Form)
+      XFA_DataExporter_RegenerateFormFile(this, pStream.get(), NULL, TRUE);
+    else
+      pElement->SaveXMLNode(pStream.get());
+    // TODO(weili): Check whether we need to save pretty print XML. If so, need
+    // to implement it. Right now, just put it here to avoid unused variable
+    // warning.

[dsinclair, 2016/05/25 17:42:38]

Should we file a bug for this and link here?

[Wei Li, 2016/05/25 18:13:18]

Done.

+    (void)bPrettyMode;
     FXJSE_Value_SetUTF8String(
         pArguments->GetReturnValue(),
         CFX_ByteStringC(pMemoryStream->GetBuffer(), pMemoryStream->GetSize()));
-    pStream->Release();
-    pStream = NULL;
-    if (pMemoryStream) {
-      pMemoryStream->Release();
-      pMemoryStream = NULL;
-    }
-    return;
-  }
-  if (GetPacketID() == XFA_XDPPACKET_Datasets) {
-    CFDE_XMLNode* pElement = GetXMLMappingNode();
-    if (!pElement || pElement->GetType() != FDE_XMLNODE_Element) {
-      FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(), bsXMLHeader);
-      return;
-    }
-    XFA_DataExporter_DealWithDataGroupNode(this);
-    IFX_MemoryStream* pMemoryStream = FX_CreateMemoryStream(TRUE);
-    IFX_Stream* pStream = IFX_Stream::CreateStream(
-        (IFX_FileWrite*)pMemoryStream,
-        FX_STREAMACCESS_Text | FX_STREAMACCESS_Write | FX_STREAMACCESS_Append);
-    if (pStream) {
-      pStream->SetCodePage(FX_CODEPAGE_UTF8);
-      pStream->WriteData(bsXMLHeader.raw_str(), bsXMLHeader.GetLength());
-      pElement->SaveXMLNode(pStream);
-      FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(),
-                                CFX_ByteStringC(pMemoryStream->GetBuffer(),
-                                                pMemoryStream->GetSize()));
-      pStream->Release();
-      pStream = NULL;
-    }
-    if (pMemoryStream) {
-      pMemoryStream->Release();
-      pMemoryStream = NULL;
-    }
     return;
   }
   FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(), "");
 }
 
 void CXFA_Node::Script_NodeClass_SetAttribute(CFXJSE_Arguments* pArguments) {
   int32_t iLength = pArguments->GetLength();
   if (iLength != 2) {
     ThrowScriptErrorMessage(XFA_IDS_INCORRECT_NUMBER_OF_METHOD,
                             L"setAttribute");
@@ skipping 4216 matching lines @@
   return m_pAttachNode->InsertChild(pNewNode, pBeforeNode);
 }
 FX_BOOL CXFA_AttachNodeList::Remove(CXFA_Node* pNode) {
   return m_pAttachNode->RemoveChild(pNode);
 }
 CXFA_Node* CXFA_AttachNodeList::Item(int32_t iIndex) {
   return m_pAttachNode->GetChild(
       iIndex, XFA_ELEMENT_UNKNOWN,
       m_pAttachNode->GetClassID() == XFA_ELEMENT_Subform);
 }