commit 52d8c65408b9c600d829156f7c46f8cc43e4f9c8
author Tom Sepez <tsepez@chromium.org> Wed Sep 23 00:22:44 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Wed Sep 23 00:22:44 2020 +0000
tree cfb24354841bc4a7c3446b70f56180e69e8748de
parent 41fedc0e01833806f4ce587e7e22d527c3dc740d

Use members in CFXA_TabParam sorting code

This is not essential for the first version of cppgc, since we are
not GCing while the stack is still active.

Also, make this a move-only type and avoid indirection through
unqiue_ptrs. The result is a move of three pointers instead of
one, but memory management becomes the container's problem, and
would be correct if we someday had a heap vector.

Change-Id: If76d1d0e782366d5a7d75e4aa4d89df3afe2e2a1
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73890
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

xfa/fxfa/cxfa_ffpageview.cpp

diff --git a/xfa/fxfa/cxfa_ffpageview.cpp b/xfa/fxfa/cxfa_ffpageview.cpp
index a4dc4ec..960c1ab 100644
--- a/xfa/fxfa/cxfa_ffpageview.cpp
+++ b/xfa/fxfa/cxfa_ffpageview.cpp
@@ -152,24 +152,33 @@
   CXFA_TabParam() = default;
   explicit CXFA_TabParam(CXFA_FFWidget* pWidget)
       : m_pItem(pWidget->GetLayoutItem()) {}
-
+  CXFA_TabParam(const CXFA_TabParam&) = delete;
+  CXFA_TabParam(CXFA_TabParam&&) noexcept = default;
   ~CXFA_TabParam() = default;
 
+  CXFA_TabParam& operator=(const CXFA_TabParam&) = delete;
+  CXFA_TabParam& operator=(CXFA_TabParam&&) noexcept = default;
+
+  void Trace(cppgc::Visitor* visitor) const {
+    visitor->Trace(m_pItem);
+    ContainerTrace(visitor, m_Children);
+  }
+
   CXFA_FFWidget* GetWidget() const { return m_pItem->GetFFWidget(); }
-  const std::vector<cppgc::Persistent<CXFA_ContentLayoutItem>>& GetChildren()
+  const std::vector<cppgc::Member<CXFA_ContentLayoutItem>>& GetChildren()
       const {
     return m_Children;
   }
   void ClearChildren() { m_Children.clear(); }
-  void AppendTabParam(CXFA_TabParam* pParam) {
+  void AppendTabParam(const CXFA_TabParam* pParam) {
     m_Children.push_back(pParam->m_pItem);
     m_Children.insert(m_Children.end(), pParam->m_Children.begin(),
                       pParam->m_Children.end());
   }
 
  private:
-  cppgc::Persistent<CXFA_ContentLayoutItem> m_pItem;
-  std::vector<cppgc::Persistent<CXFA_ContentLayoutItem>> m_Children;
+  cppgc::Member<CXFA_ContentLayoutItem> m_pItem;
+  std::vector<cppgc::Member<CXFA_ContentLayoutItem>> m_Children;
 };
 
 void OrderContainer(CXFA_LayoutItemIterator* sIterator,
@@ -178,7 +187,7 @@
                     bool* bCurrentItem,
                     bool* bContentArea,
                     bool bMasterPage) {
-  std::vector<std::unique_ptr<CXFA_TabParam>> tabParams;
+  std::vector<CXFA_TabParam> tabParams;
   CXFA_LayoutItem* pSearchItem = sIterator->MoveToNext();
   while (pSearchItem) {
     if (!pSearchItem->IsContentLayoutItem()) {
@@ -199,10 +208,10 @@
         *bCurrentItem = true;
         break;
       }
-      tabParams.push_back(std::make_unique<CXFA_TabParam>(hWidget));
+      tabParams.emplace_back(hWidget);
       if (IsLayoutElement(pSearchItem->GetFormNode()->GetElementType(), true)) {
-        OrderContainer(sIterator, pSearchItem, tabParams.back().get(),
-                       bCurrentItem, bContentArea, bMasterPage);
+        OrderContainer(sIterator, pSearchItem, &tabParams.back(), bCurrentItem,
+                       bContentArea, bMasterPage);
       }
     }
     if (*bCurrentItem) {
@@ -213,16 +222,15 @@
     }
   }
   std::sort(tabParams.begin(), tabParams.end(),
-            [](const std::unique_ptr<CXFA_TabParam>& arg1,
-               const std::unique_ptr<CXFA_TabParam>& arg2) {
-              const CFX_RectF& rt1 = arg1->GetWidget()->GetWidgetRect();
-              const CFX_RectF& rt2 = arg2->GetWidget()->GetWidgetRect();
+            [](const CXFA_TabParam& arg1, const CXFA_TabParam& arg2) {
+              const CFX_RectF& rt1 = arg1.GetWidget()->GetWidgetRect();
+              const CFX_RectF& rt2 = arg2.GetWidget()->GetWidgetRect();
               if (rt1.top - rt2.top >= kXFAWidgetPrecision)
                 return rt1.top < rt2.top;
               return rt1.left < rt2.left;
             });
-  for (const auto& pParam : tabParams)
-    pContainer->AppendTabParam(pParam.get());
+  for (const auto& param : tabParams)
+    pContainer->AppendTabParam(&param);
 }
 
 }  // namespace
@@ -483,22 +491,22 @@
 CXFA_FFTabOrderPageWidgetIterator::CreateSpaceOrderLayoutItems() {
   std::vector<CXFA_ContentLayoutItem*> items;
   CXFA_LayoutItemIterator sIterator(m_pPageViewLayout.Get());
-  auto pParam = std::make_unique<CXFA_TabParam>();
+  CXFA_TabParam tabparam;
   bool bCurrentItem = false;
   bool bContentArea = false;
-  OrderContainer(&sIterator, nullptr, pParam.get(), &bCurrentItem,
-                 &bContentArea, false);
-  items.reserve(pParam->GetChildren().size());
-  for (const auto& layout_item : pParam->GetChildren())
+  OrderContainer(&sIterator, nullptr, &tabparam, &bCurrentItem, &bContentArea,
+                 false);
+  items.reserve(tabparam.GetChildren().size());
+  for (const auto& layout_item : tabparam.GetChildren())
     items.push_back(layout_item);
 
   sIterator.Reset();
   bCurrentItem = false;
   bContentArea = false;
-  pParam->ClearChildren();
-  OrderContainer(&sIterator, nullptr, pParam.get(), &bCurrentItem,
-                 &bContentArea, true);
-  for (const auto& layout_item : pParam->GetChildren())
+  tabparam.ClearChildren();
+  OrderContainer(&sIterator, nullptr, &tabparam, &bCurrentItem, &bContentArea,
+                 true);
+  for (const auto& layout_item : tabparam.GetChildren())
     items.push_back(layout_item);
 
   return items;