Change ProcessBreakBeforeOrAfter() to return an Optional struct.
Instead of returning a bool and have 3 out parameters. Along the way:
- Make ProcessBreakBeforeOrAfter() private. Expose ProcessBreakBefore()
and ProcessBreakAfter() public methods.
- Change CXFA_LayoutPageMgr::ExecuteBreakBeforeOrAfter() to return a
struct.
- Mark a bunch of methods, members, and parameters const.
Change-Id: Ia63ae6a8d4bbc7496e43141cb6178236d9541f03
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/49470
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/fxjs/xfa/cjx_object.cpp b/fxjs/xfa/cjx_object.cpp
index e69d5e5..7a5e082 100644
--- a/fxjs/xfa/cjx_object.cpp
+++ b/fxjs/xfa/cjx_object.cpp
@@ -351,12 +351,12 @@
}
}
-int32_t CJX_Object::GetInteger(XFA_Attribute eAttr) {
+int32_t CJX_Object::GetInteger(XFA_Attribute eAttr) const {
return TryInteger(eAttr, true).value_or(0);
}
Optional<int32_t> CJX_Object::TryInteger(XFA_Attribute eAttr,
- bool bUseDefault) {
+ bool bUseDefault) const {
void* pKey = GetMapKey_Element(GetXFAObject()->GetElementType(), eAttr);
Optional<void*> value = GetMapModuleValue(pKey);
if (value.has_value())
diff --git a/fxjs/xfa/cjx_object.h b/fxjs/xfa/cjx_object.h
index a348cb7..6394b28 100644
--- a/fxjs/xfa/cjx_object.h
+++ b/fxjs/xfa/cjx_object.h
@@ -175,9 +175,9 @@
Optional<WideString> TryNamespace();
- Optional<int32_t> TryInteger(XFA_Attribute eAttr, bool bUseDefault);
+ Optional<int32_t> TryInteger(XFA_Attribute eAttr, bool bUseDefault) const;
void SetInteger(XFA_Attribute eAttr, int32_t iValue, bool bNotify);
- int32_t GetInteger(XFA_Attribute eAttr);
+ int32_t GetInteger(XFA_Attribute eAttr) const;
Optional<WideString> TryCData(XFA_Attribute eAttr, bool bUseDefault) const;
void SetCData(XFA_Attribute eAttr,
diff --git a/xfa/fxfa/layout/cxfa_itemlayoutprocessor.cpp b/xfa/fxfa/layout/cxfa_itemlayoutprocessor.cpp
index 2d37da0..4b7d287 100644
--- a/xfa/fxfa/layout/cxfa_itemlayoutprocessor.cpp
+++ b/xfa/fxfa/layout/cxfa_itemlayoutprocessor.cpp
@@ -1692,18 +1692,18 @@
fContentCalculatedHeight -= item->m_sSize.height;
}
- CXFA_Node* pLeaderNode = nullptr;
- CXFA_Node* pTrailerNode = nullptr;
- bool bCreatePage = false;
- if (!bUseBreakControl || !m_pPageMgr ||
- !m_pPageMgr->ProcessBreakBeforeOrAfter(m_pCurChildNode, true,
- pLeaderNode, pTrailerNode,
- &bCreatePage) ||
- GetFormNode()->GetElementType() == XFA_Element::Form ||
- !bCreatePage) {
+ if (!bUseBreakControl || !m_pPageMgr)
+ break;
+
+ Optional<CXFA_LayoutPageMgr::BreakData> break_data =
+ m_pPageMgr->ProcessBreakBefore(m_pCurChildNode);
+ if (!break_data.has_value() || !break_data.value().bCreatePage ||
+ GetFormNode()->GetElementType() == XFA_Element::Form) {
break;
}
+ CXFA_Node* pLeaderNode = break_data.value().pLeader;
+ CXFA_Node* pTrailerNode = break_data.value().pTrailer;
if (JudgeLeaderOrTrailerForOccur(pLeaderNode))
AddPendingNode(pLeaderNode, true);
@@ -1731,17 +1731,19 @@
goto SuspendAndCreateNewRow;
}
case Stage::kBreakAfter: {
- CXFA_Node* pLeaderNode = nullptr;
- CXFA_Node* pTrailerNode = nullptr;
- bool bCreatePage = false;
- if (!bUseBreakControl || !m_pPageMgr ||
- !m_pPageMgr->ProcessBreakBeforeOrAfter(m_pCurChildNode, false,
- pLeaderNode, pTrailerNode,
- &bCreatePage) ||
+ if (!bUseBreakControl || !m_pPageMgr)
+ break;
+
+ Optional<CXFA_LayoutPageMgr::BreakData> break_data =
+ m_pPageMgr->ProcessBreakAfter(m_pCurChildNode);
+ if (!break_data.has_value() ||
GetFormNode()->GetElementType() == XFA_Element::Form) {
break;
}
+ CXFA_Node* pLeaderNode = break_data.value().pLeader;
+ CXFA_Node* pTrailerNode = break_data.value().pTrailer;
+ bool bCreatePage = break_data.value().bCreatePage;
if (JudgeLeaderOrTrailerForOccur(pTrailerNode)) {
auto pTempProcessor = pdfium::MakeUnique<CXFA_ItemLayoutProcessor>(
pTrailerNode, nullptr);
diff --git a/xfa/fxfa/layout/cxfa_itemlayoutprocessor.h b/xfa/fxfa/layout/cxfa_itemlayoutprocessor.h
index 2974a2f..eb32791 100644
--- a/xfa/fxfa/layout/cxfa_itemlayoutprocessor.h
+++ b/xfa/fxfa/layout/cxfa_itemlayoutprocessor.h
@@ -188,7 +188,7 @@
float m_fLastRowWidth = 0;
float m_fLastRowY = 0;
float m_fWidthLimit = 0;
- CXFA_Node* m_pFormNode;
+ CXFA_Node* const m_pFormNode;
CXFA_Node* m_pCurChildNode = nullptr;
CXFA_Node* m_pKeepHeadNode = nullptr;
CXFA_Node* m_pKeepTailNode = nullptr;
diff --git a/xfa/fxfa/layout/cxfa_layoutpagemgr.cpp b/xfa/fxfa/layout/cxfa_layoutpagemgr.cpp
index e651257..df70266 100644
--- a/xfa/fxfa/layout/cxfa_layoutpagemgr.cpp
+++ b/xfa/fxfa/layout/cxfa_layoutpagemgr.cpp
@@ -446,10 +446,10 @@
bool CXFA_LayoutPageMgr::PrepareFirstPage(CXFA_Node* pRootSubform) {
bool bProBreakBefore = false;
- CXFA_Node* pBreakBeforeNode = nullptr;
+ const CXFA_Node* pBreakBeforeNode = nullptr;
while (pRootSubform) {
- for (CXFA_Node* pBreakNode = pRootSubform->GetFirstChild(); pBreakNode;
- pBreakNode = pBreakNode->GetNextSibling()) {
+ for (const CXFA_Node* pBreakNode = pRootSubform->GetFirstChild();
+ pBreakNode; pBreakNode = pBreakNode->GetNextSibling()) {
XFA_Element eType = pBreakNode->GetElementType();
if (eType == XFA_Element::BreakBefore ||
(eType == XFA_Element::Break &&
@@ -471,12 +471,12 @@
XFA_Element::Subform);
}
}
- CXFA_Node* pLeader;
- CXFA_Node* pTrailer;
- if (pBreakBeforeNode &&
- ExecuteBreakBeforeOrAfter(pBreakBeforeNode, true, pLeader, pTrailer)) {
- m_CurrentContainerRecordIter = m_ProposedContainerRecords.begin();
- return true;
+ if (pBreakBeforeNode) {
+ BreakData ret = ExecuteBreakBeforeOrAfter(pBreakBeforeNode, true);
+ if (ret.bCreatePage) {
+ m_CurrentContainerRecordIter = m_ProposedContainerRecords.begin();
+ return true;
+ }
}
return AppendNewPage(true);
}
@@ -759,11 +759,10 @@
return bRet;
}
-bool CXFA_LayoutPageMgr::ExecuteBreakBeforeOrAfter(
- CXFA_Node* pCurNode,
- bool bBefore,
- CXFA_Node*& pBreakLeaderTemplate,
- CXFA_Node*& pBreakTrailerTemplate) {
+CXFA_LayoutPageMgr::BreakData CXFA_LayoutPageMgr::ExecuteBreakBeforeOrAfter(
+ const CXFA_Node* pCurNode,
+ bool bBefore) {
+ BreakData ret = {nullptr, nullptr, false};
XFA_Element eType = pCurNode->GetElementType();
switch (eType) {
case XFA_Element::BreakBefore:
@@ -777,7 +776,7 @@
CXFA_Script* pScript =
pCurNode->GetFirstChildByClass<CXFA_Script>(XFA_Element::Script);
if (pScript && !RunBreakTestScript(pScript))
- return false;
+ break;
WideString wsTarget =
pCurNode->JSObject()->GetCData(XFA_Attribute::Target);
@@ -785,14 +784,13 @@
ResolveBreakTarget(m_pTemplatePageSetRoot, true, wsTarget);
wsBreakTrailer = pCurNode->JSObject()->GetCData(XFA_Attribute::Trailer);
wsBreakLeader = pCurNode->JSObject()->GetCData(XFA_Attribute::Leader);
- pBreakLeaderTemplate =
- ResolveBreakTarget(pContainer, true, wsBreakLeader);
- pBreakTrailerTemplate =
- ResolveBreakTarget(pContainer, true, wsBreakTrailer);
+ ret.pLeader = ResolveBreakTarget(pContainer, true, wsBreakLeader);
+ ret.pTrailer = ResolveBreakTarget(pContainer, true, wsBreakTrailer);
if (RunBreak(eType,
pCurNode->JSObject()->GetEnum(XFA_Attribute::TargetType),
pTarget, bStartNew)) {
- return true;
+ ret.bCreatePage = true;
+ break;
}
if (!m_ProposedContainerRecords.empty() &&
m_CurrentContainerRecordIter == m_ProposedContainerRecords.begin() &&
@@ -807,7 +805,7 @@
pParentNode->GetElementType() != XFA_Element::Form) {
break;
}
- return true;
+ ret.bCreatePage = true;
}
break;
}
@@ -822,64 +820,68 @@
pCurNode->JSObject()->GetEnum(
bBefore ? XFA_Attribute::Before : XFA_Attribute::After),
pTarget, bStartNew)) {
- return true;
+ ret.bCreatePage = true;
}
break;
}
default:
break;
}
- return false;
+ return ret;
}
-bool CXFA_LayoutPageMgr::ProcessBreakBeforeOrAfter(
- CXFA_Node* pBreakNode,
- bool bBefore,
- CXFA_Node*& pBreakLeaderNode,
- CXFA_Node*& pBreakTrailerNode,
- bool* pCreatePage) {
- CXFA_Node* pLeaderTemplate = nullptr;
- CXFA_Node* pTrailerTemplate = nullptr;
+Optional<CXFA_LayoutPageMgr::BreakData> CXFA_LayoutPageMgr::ProcessBreakBefore(
+ const CXFA_Node* pBreakNode) {
+ return ProcessBreakBeforeOrAfter(pBreakNode, /*before=*/true);
+}
+
+Optional<CXFA_LayoutPageMgr::BreakData> CXFA_LayoutPageMgr::ProcessBreakAfter(
+ const CXFA_Node* pBreakNode) {
+ return ProcessBreakBeforeOrAfter(pBreakNode, /*before=*/false);
+}
+
+Optional<CXFA_LayoutPageMgr::BreakData>
+CXFA_LayoutPageMgr::ProcessBreakBeforeOrAfter(const CXFA_Node* pBreakNode,
+ bool bBefore) {
CXFA_Node* pFormNode = pBreakNode->GetContainerParent();
if (!pFormNode->PresenceRequiresSpace())
- return false;
+ return pdfium::nullopt;
- *pCreatePage = ExecuteBreakBeforeOrAfter(pBreakNode, bBefore, pLeaderTemplate,
- pTrailerTemplate);
+ BreakData break_data = ExecuteBreakBeforeOrAfter(pBreakNode, bBefore);
CXFA_Document* pDocument = pBreakNode->GetDocument();
CXFA_Node* pDataScope = nullptr;
pFormNode = pFormNode->GetContainerParent();
- if (pLeaderTemplate) {
- if (!pLeaderTemplate->IsContainerNode())
- return false;
+ if (break_data.pLeader) {
+ if (!break_data.pLeader->IsContainerNode())
+ return pdfium::nullopt;
if (!pDataScope)
pDataScope = XFA_DataMerge_FindDataScope(pFormNode);
- pBreakLeaderNode = pDocument->DataMerge_CopyContainer(
- pLeaderTemplate, pFormNode, pDataScope, true, true, true);
- if (!pBreakLeaderNode)
- return false;
+ break_data.pLeader = pDocument->DataMerge_CopyContainer(
+ break_data.pLeader, pFormNode, pDataScope, true, true, true);
+ if (!break_data.pLeader)
+ return pdfium::nullopt;
- pDocument->DataMerge_UpdateBindingRelations(pBreakLeaderNode);
- SetLayoutGeneratedNodeFlag(pBreakLeaderNode);
+ pDocument->DataMerge_UpdateBindingRelations(break_data.pLeader);
+ SetLayoutGeneratedNodeFlag(break_data.pLeader);
}
- if (pTrailerTemplate) {
- if (!pTrailerTemplate->IsContainerNode())
- return false;
+ if (break_data.pTrailer) {
+ if (!break_data.pTrailer->IsContainerNode())
+ return pdfium::nullopt;
if (!pDataScope)
pDataScope = XFA_DataMerge_FindDataScope(pFormNode);
- pBreakTrailerNode = pDocument->DataMerge_CopyContainer(
- pTrailerTemplate, pFormNode, pDataScope, true, true, true);
- if (!pBreakTrailerNode)
- return false;
+ break_data.pTrailer = pDocument->DataMerge_CopyContainer(
+ break_data.pTrailer, pFormNode, pDataScope, true, true, true);
+ if (!break_data.pTrailer)
+ return pdfium::nullopt;
- pDocument->DataMerge_UpdateBindingRelations(pBreakTrailerNode);
- SetLayoutGeneratedNodeFlag(pBreakTrailerNode);
+ pDocument->DataMerge_UpdateBindingRelations(break_data.pTrailer);
+ SetLayoutGeneratedNodeFlag(break_data.pTrailer);
}
- return true;
+ return break_data;
}
CXFA_Node* CXFA_LayoutPageMgr::ProcessBookendLeader(
diff --git a/xfa/fxfa/layout/cxfa_layoutpagemgr.h b/xfa/fxfa/layout/cxfa_layoutpagemgr.h
index 0180181..0006761 100644
--- a/xfa/fxfa/layout/cxfa_layoutpagemgr.h
+++ b/xfa/fxfa/layout/cxfa_layoutpagemgr.h
@@ -12,6 +12,7 @@
#include <map>
#include <vector>
+#include "third_party/base/optional.h"
#include "xfa/fxfa/layout/cxfa_itemlayoutprocessor.h"
class CXFA_ContainerRecord;
@@ -20,6 +21,12 @@
class CXFA_LayoutPageMgr {
public:
+ struct BreakData {
+ CXFA_Node* pLeader;
+ CXFA_Node* pTrailer;
+ bool bCreatePage;
+ };
+
explicit CXFA_LayoutPageMgr(CXFA_LayoutProcessor* pLayoutProcessor);
~CXFA_LayoutPageMgr();
@@ -37,11 +44,8 @@
inline CXFA_ContainerLayoutItem* GetRootLayoutItem() const {
return m_pPageSetLayoutItemRoot;
}
- bool ProcessBreakBeforeOrAfter(CXFA_Node* pBreakNode,
- bool bBefore,
- CXFA_Node*& pBreakLeaderNode,
- CXFA_Node*& pBreakTrailerNode,
- bool* pCreatePage);
+ Optional<BreakData> ProcessBreakBefore(const CXFA_Node* pBreakNode);
+ Optional<BreakData> ProcessBreakAfter(const CXFA_Node* pBreakNode);
bool ProcessOverflow(CXFA_Node* pFormNode,
CXFA_Node*& pLeaderNode,
CXFA_Node*& pTrailerNode,
@@ -81,10 +85,9 @@
bool bLeader);
CXFA_Node* ResolveBookendLeaderOrTrailer(const CXFA_Node* pBookendNode,
bool bLeader);
- bool ExecuteBreakBeforeOrAfter(CXFA_Node* pCurNode,
- bool bBefore,
- CXFA_Node*& pBreakLeaderTemplate,
- CXFA_Node*& pBreakTrailerTemplate);
+ Optional<BreakData> ProcessBreakBeforeOrAfter(const CXFA_Node* pBreakNode,
+ bool bBefore);
+ BreakData ExecuteBreakBeforeOrAfter(const CXFA_Node* pCurNode, bool bBefore);
int32_t CreateMinPageRecord(CXFA_Node* pPageArea,
bool bTargetPageArea,
