commit 7ea611fb82308b4b8f9dc150a9d79334435c53b6
author Ryan Harrison <rharrison@chromium.org> Tue Sep 18 17:50:35 2018 +0000
committer Chromium commit bot <commit-bot@chromium.org> Tue Sep 18 17:50:35 2018 +0000
tree de7b509e8d036d47c6cf3e49394f98ca198bfd92
parent 6fc8d877d7450905623c6abb1094ab99f5bd80cb

Transfer ownership of nodes to top-level XML doc

For XFA, XML nodes are owned by the XML doc that they were created by,
but references to them are stored elsewhere. For a PDF document there
is one top-level XML document created and retained when the initial
XFA XML is parsed. Another can be created if loadXML is called by
JS. In the existing code the XML doc that owns the newly created nodes
is local to loadXML. So the nodes are destroyed right after putting
refernces to them into the main XFA data structures.

This CL adds in a method to transfer ownership of the XML nodes from
one doc to another, and uses it to correctly retain the newly created
nodes, by having them owned by the top-level XML doc.

BUG=chromium:884664

Change-Id: Id29b4edbfe44aefb9713328e4e217e830f7e9e14
Reviewed-on: https://pdfium-review.googlesource.com/42690
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>

core/fxcrt/xml/cfx_xmldocument.cpp

diff --git a/core/fxcrt/xml/cfx_xmldocument.cpp b/core/fxcrt/xml/cfx_xmldocument.cpp
index f6e5555..5ce4aed 100644
--- a/core/fxcrt/xml/cfx_xmldocument.cpp
+++ b/core/fxcrt/xml/cfx_xmldocument.cpp
@@ -12,3 +12,10 @@
 }
 
 CFX_XMLDocument::~CFX_XMLDocument() = default;
+
+void CFX_XMLDocument::AppendNodesFrom(CFX_XMLDocument* other) {
+  nodes_.reserve(nodes_.size() + other->nodes_.size());
+  nodes_.insert(nodes_.end(), std::make_move_iterator(other->nodes_.begin()),
+                std::make_move_iterator(other->nodes_.end()));
+  other->nodes_.clear();
+}

core/fxcrt/xml/cfx_xmldocument.h

diff --git a/core/fxcrt/xml/cfx_xmldocument.h b/core/fxcrt/xml/cfx_xmldocument.h
index a568f83..7d6f02b 100644
--- a/core/fxcrt/xml/cfx_xmldocument.h
+++ b/core/fxcrt/xml/cfx_xmldocument.h
@@ -29,6 +29,12 @@
     return static_cast<T*>(nodes_.back().get());
   }
 
+  // Transfers ownership of entries in |nodes_| from |other| to |this|.
+  // This is used in CJX_Node::loadXML to transfer ownership of the newly
+  // created nodes to the top-level XML doc for the PDF, after parsing an XML
+  // blob.
+  void AppendNodesFrom(CFX_XMLDocument* other);
+
  private:
   std::vector<std::unique_ptr<CFX_XMLNode>> nodes_;
   UnownedPtr<CFX_XMLElement> root_;

fxjs/xfa/cjx_node.cpp

diff --git a/fxjs/xfa/cjx_node.cpp b/fxjs/xfa/cjx_node.cpp
index 8876628..49cd041 100644
--- a/fxjs/xfa/cjx_node.cpp
+++ b/fxjs/xfa/cjx_node.cpp
@@ -221,6 +221,10 @@
   if (!pXMLNode)
     return CJS_Result::Success();
 
+  CFX_XMLDocument* top_xml_doc =
+      GetXFANode()->GetDocument()->GetNotify()->GetHDOC()->GetXMLDocument();
+  top_xml_doc->AppendNodesFrom(pParser->GetXMLDoc().get());
+
   if (bIgnoreRoot &&
       (pXMLNode->GetType() != FX_XMLNODE_Element ||
        XFA_RecognizeRichText(static_cast<CFX_XMLElement*>(pXMLNode)))) {
@@ -239,19 +243,10 @@
     CFX_XMLNode* pThisXMLRoot = GetXFANode()->GetXMLMappingNode();
     CFX_XMLNode* clone;
     if (pThisXMLRoot) {
-      clone = pThisXMLRoot->Clone(GetXFANode()
-                                      ->GetDocument()
-                                      ->GetNotify()
-                                      ->GetHDOC()
-                                      ->GetXMLDocument());
+      clone = pThisXMLRoot->Clone(top_xml_doc);
     } else {
-      clone = GetXFANode()
-                  ->GetDocument()
-                  ->GetNotify()
-                  ->GetHDOC()
-                  ->GetXMLDocument()
-                  ->CreateNode<CFX_XMLElement>(
-                      WideString(GetXFANode()->GetClassName()));
+      clone = top_xml_doc->CreateNode<CFX_XMLElement>(
+          WideString(GetXFANode()->GetClassName()));
     }
     pFakeXMLRoot = clone;
   }