Small cleanup of CPDF_Annot
-- Remove one form of constructor and inline Init() in the other.
-- Remove stale comment about dictionary ownership.
-- Make AppearanceMode an enum class.
-- Remove some extraneous scopers inside class.
-- const qualify some members.
-- Pack a little tighter.
Change-Id: I22331cda3cb7edbe14345e49fe5f400ae1f0da72
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/84432
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfdoc/cpdf_annot.cpp b/core/fpdfdoc/cpdf_annot.cpp
index fbddbe6..ca4d3a5 100644
--- a/core/fpdfdoc/cpdf_annot.cpp
+++ b/core/fpdfdoc/cpdf_annot.cpp
@@ -64,9 +64,9 @@
return nullptr;
const char* ap_entry = "N";
- if (eMode == CPDF_Annot::Down)
+ if (eMode == CPDF_Annot::AppearanceMode::kDown)
ap_entry = "D";
- else if (eMode == CPDF_Annot::Rollover)
+ else if (eMode == CPDF_Annot::AppearanceMode::kRollover)
ap_entry = "R";
if (bFallbackToNormal && !pAP->KeyExist(ap_entry))
ap_entry = "N";
@@ -95,30 +95,21 @@
} // namespace
-CPDF_Annot::CPDF_Annot(RetainPtr<CPDF_Dictionary> pDict,
- CPDF_Document* pDocument)
- : m_pAnnotDict(std::move(pDict)), m_pDocument(pDocument) {
- Init();
-}
-
CPDF_Annot::CPDF_Annot(CPDF_Dictionary* pDict, CPDF_Document* pDocument)
- : m_pAnnotDict(pDict), m_pDocument(pDocument) {
- Init();
+ : m_pAnnotDict(pDict),
+ m_pDocument(pDocument),
+ m_nSubtype(StringToAnnotSubtype(
+ m_pAnnotDict->GetStringFor(pdfium::annotation::kSubtype))),
+ m_bIsTextMarkupAnnotation(IsTextMarkupAnnotation(m_nSubtype)),
+ m_bHasGeneratedAP(
+ m_pAnnotDict->GetBooleanFor(kPDFiumKey_HasGeneratedAP, false)) {
+ GenerateAPIfNeeded();
}
CPDF_Annot::~CPDF_Annot() {
ClearCachedAP();
}
-void CPDF_Annot::Init() {
- m_nSubtype = StringToAnnotSubtype(
- m_pAnnotDict->GetStringFor(pdfium::annotation::kSubtype));
- m_bIsTextMarkupAnnotation = IsTextMarkupAnnotation(m_nSubtype);
- m_bHasGeneratedAP =
- m_pAnnotDict->GetBooleanFor(kPDFiumKey_HasGeneratedAP, false);
- GenerateAPIfNeeded();
-}
-
void CPDF_Annot::GenerateAPIfNeeded() {
if (!ShouldGenerateAP())
return;
diff --git a/core/fpdfdoc/cpdf_annot.h b/core/fpdfdoc/cpdf_annot.h
index 5bb677a..8988c56 100644
--- a/core/fpdfdoc/cpdf_annot.h
+++ b/core/fpdfdoc/cpdf_annot.h
@@ -31,8 +31,8 @@
class CPDF_Annot {
public:
- enum AppearanceMode { Normal, Rollover, Down };
- enum class Subtype {
+ enum class AppearanceMode { kNormal, kRollover, kDown };
+ enum class Subtype : uint8_t {
UNKNOWN = 0,
TEXT,
LINK,
@@ -64,8 +64,8 @@
REDACT
};
- static CPDF_Annot::Subtype StringToAnnotSubtype(const ByteString& sSubtype);
- static ByteString AnnotSubtypeToString(CPDF_Annot::Subtype nSubtype);
+ static Subtype StringToAnnotSubtype(const ByteString& sSubtype);
+ static ByteString AnnotSubtypeToString(Subtype nSubtype);
static CFX_FloatRect RectFromQuadPointsArray(const CPDF_Array* pArray,
size_t nIndex);
static CFX_FloatRect BoundingRectFromQuadPoints(
@@ -74,12 +74,10 @@
size_t nIndex);
static size_t QuadPointCount(const CPDF_Array* pArray);
- // The second constructor does not take ownership of the dictionary.
- CPDF_Annot(RetainPtr<CPDF_Dictionary> pDict, CPDF_Document* pDocument);
CPDF_Annot(CPDF_Dictionary* pDict, CPDF_Document* pDocument);
~CPDF_Annot();
- CPDF_Annot::Subtype GetSubtype() const;
+ Subtype GetSubtype() const;
uint32_t GetFlags() const;
CFX_FloatRect GetRect() const;
CPDF_Document* GetDocument() const { return m_pDocument.Get(); }
@@ -108,7 +106,6 @@
void SetPopupAnnot(CPDF_Annot* pAnnot) { m_pPopupAnnot = pAnnot; }
private:
- void Init();
void GenerateAPIfNeeded();
bool ShouldGenerateAP() const;
bool ShouldDrawAnnotation() const;
@@ -117,14 +114,14 @@
RetainPtr<CPDF_Dictionary> const m_pAnnotDict;
UnownedPtr<CPDF_Document> const m_pDocument;
- CPDF_Annot::Subtype m_nSubtype;
std::map<CPDF_Stream*, std::unique_ptr<CPDF_Form>> m_APMap;
// If non-null, then this is not a popup annotation.
UnownedPtr<CPDF_Annot> m_pPopupAnnot;
+ const Subtype m_nSubtype;
+ const bool m_bIsTextMarkupAnnotation;
// |m_bOpenState| is only set for popup annotations.
bool m_bOpenState = false;
bool m_bHasGeneratedAP;
- bool m_bIsTextMarkupAnnotation;
};
// Get the AP in an annotation dict for a given appearance mode.
diff --git a/core/fpdfdoc/cpdf_annotlist.cpp b/core/fpdfdoc/cpdf_annotlist.cpp
index e8b4c09..3ea2f5f 100644
--- a/core/fpdfdoc/cpdf_annotlist.cpp
+++ b/core/fpdfdoc/cpdf_annotlist.cpp
@@ -116,8 +116,7 @@
pAnnotDict->SetRectFor(pdfium::annotation::kRect, popupRect);
pAnnotDict->SetNewFor<CPDF_Number>(pdfium::annotation::kF, 0);
- auto pPopupAnnot =
- std::make_unique<CPDF_Annot>(std::move(pAnnotDict), pDocument);
+ auto pPopupAnnot = std::make_unique<CPDF_Annot>(pAnnotDict.Get(), pDocument);
pAnnot->SetPopupAnnot(pPopupAnnot.get());
return pPopupAnnot;
}
@@ -263,9 +262,11 @@
continue;
}
if (pContext) {
- pAnnot->DrawInContext(pPage, pContext, matrix, CPDF_Annot::Normal);
+ pAnnot->DrawInContext(pPage, pContext, matrix,
+ CPDF_Annot::AppearanceMode::kNormal);
} else if (!pAnnot->DrawAppearance(pPage, pDevice, matrix,
- CPDF_Annot::Normal, pOptions)) {
+ CPDF_Annot::AppearanceMode::kNormal,
+ pOptions)) {
pAnnot->DrawBorder(pDevice, &matrix, pOptions);
}
}
diff --git a/fpdfsdk/cpdfsdk_baannothandler.cpp b/fpdfsdk/cpdfsdk_baannothandler.cpp
index 1ad6b74..c8ad3b1 100644
--- a/fpdfsdk/cpdfsdk_baannothandler.cpp
+++ b/fpdfsdk/cpdfsdk_baannothandler.cpp
@@ -77,8 +77,8 @@
const CPDF_Annot::Subtype annot_type = pAnnot->GetAnnotSubtype();
if (bDrawAnnots && annot_type == CPDF_Annot::Subtype::POPUP) {
- pAnnot->AsBAAnnot()->DrawAppearance(pDevice, mtUser2Device,
- CPDF_Annot::Normal, nullptr);
+ pAnnot->AsBAAnnot()->DrawAppearance(
+ pDevice, mtUser2Device, CPDF_Annot::AppearanceMode::kNormal, nullptr);
return;
}
diff --git a/fpdfsdk/cpdfsdk_widget.cpp b/fpdfsdk/cpdfsdk_widget.cpp
index ed2c13d..8bed925 100644
--- a/fpdfsdk/cpdfsdk_widget.cpp
+++ b/fpdfsdk/cpdfsdk_widget.cpp
@@ -347,9 +347,9 @@
// Choose the right sub-ap
const char* ap_entry = "N";
- if (mode == CPDF_Annot::Down)
+ if (mode == CPDF_Annot::AppearanceMode::kDown)
ap_entry = "D";
- else if (mode == CPDF_Annot::Rollover)
+ else if (mode == CPDF_Annot::AppearanceMode::kRollover)
ap_entry = "R";
if (!pAP->KeyExist(ap_entry))
ap_entry = "N";
@@ -557,7 +557,7 @@
pFormField->CheckControl(pFormField->GetControlIndex(pFormCtrl), bChecked,
NotificationOption::kDoNotNotify);
#ifdef PDF_ENABLE_XFA
- if (!IsWidgetAppearanceValid(CPDF_Annot::Normal))
+ if (!IsWidgetAppearanceValid(CPDF_Annot::AppearanceMode::kNormal))
ResetXFAAppearance(CPDFSDK_Widget::kValueChanged);
Synchronize(true);
#endif // PDF_ENABLE_XFA
@@ -671,8 +671,8 @@
if ((fieldType == FormFieldType::kCheckBox ||
fieldType == FormFieldType::kRadioButton) &&
- mode == CPDF_Annot::Normal &&
- !IsWidgetAppearanceValid(CPDF_Annot::Normal)) {
+ mode == CPDF_Annot::AppearanceMode::kNormal &&
+ !IsWidgetAppearanceValid(CPDF_Annot::AppearanceMode::kNormal)) {
CFX_GraphStateData gsd;
gsd.m_LineWidth = 0.0f;
diff --git a/fpdfsdk/cpdfsdk_widgethandler.cpp b/fpdfsdk/cpdfsdk_widgethandler.cpp
index 7886191..62ab33a 100644
--- a/fpdfsdk/cpdfsdk_widgethandler.cpp
+++ b/fpdfsdk/cpdfsdk_widgethandler.cpp
@@ -87,8 +87,8 @@
const CFX_Matrix& mtUser2Device,
bool bDrawAnnots) {
if (pAnnot->IsSignatureWidget()) {
- pAnnot->AsBAAnnot()->DrawAppearance(pDevice, mtUser2Device,
- CPDF_Annot::Normal, nullptr);
+ pAnnot->AsBAAnnot()->DrawAppearance(
+ pDevice, mtUser2Device, CPDF_Annot::AppearanceMode::kNormal, nullptr);
} else {
m_pInteractiveFormFiller->OnDraw(pPageView, pAnnot, pDevice, mtUser2Device);
}
diff --git a/fpdfsdk/formfiller/cffl_button.cpp b/fpdfsdk/formfiller/cffl_button.cpp
index f80bb1c..26eb0bc 100644
--- a/fpdfsdk/formfiller/cffl_button.cpp
+++ b/fpdfsdk/formfiller/cffl_button.cpp
@@ -66,32 +66,34 @@
CPDFSDK_Widget* pWidget = ToCPDFSDKWidget(pAnnot);
CPDF_FormControl* pCtrl = pWidget->GetFormControl();
if (pCtrl->GetHighlightingMode() != CPDF_FormControl::kPush) {
- pWidget->DrawAppearance(pDevice, mtUser2Device, CPDF_Annot::Normal,
- nullptr);
+ pWidget->DrawAppearance(pDevice, mtUser2Device,
+ CPDF_Annot::AppearanceMode::kNormal, nullptr);
return;
}
if (m_bMouseDown) {
- if (pWidget->IsWidgetAppearanceValid(CPDF_Annot::Down)) {
- pWidget->DrawAppearance(pDevice, mtUser2Device, CPDF_Annot::Down,
- nullptr);
+ if (pWidget->IsWidgetAppearanceValid(CPDF_Annot::AppearanceMode::kDown)) {
+ pWidget->DrawAppearance(pDevice, mtUser2Device,
+ CPDF_Annot::AppearanceMode::kDown, nullptr);
} else {
- pWidget->DrawAppearance(pDevice, mtUser2Device, CPDF_Annot::Normal,
- nullptr);
+ pWidget->DrawAppearance(pDevice, mtUser2Device,
+ CPDF_Annot::AppearanceMode::kNormal, nullptr);
}
return;
}
if (m_bMouseIn) {
- if (pWidget->IsWidgetAppearanceValid(CPDF_Annot::Rollover)) {
- pWidget->DrawAppearance(pDevice, mtUser2Device, CPDF_Annot::Rollover,
- nullptr);
+ if (pWidget->IsWidgetAppearanceValid(
+ CPDF_Annot::AppearanceMode::kRollover)) {
+ pWidget->DrawAppearance(pDevice, mtUser2Device,
+ CPDF_Annot::AppearanceMode::kRollover, nullptr);
} else {
- pWidget->DrawAppearance(pDevice, mtUser2Device, CPDF_Annot::Normal,
- nullptr);
+ pWidget->DrawAppearance(pDevice, mtUser2Device,
+ CPDF_Annot::AppearanceMode::kNormal, nullptr);
}
return;
}
- pWidget->DrawAppearance(pDevice, mtUser2Device, CPDF_Annot::Normal, nullptr);
+ pWidget->DrawAppearance(pDevice, mtUser2Device,
+ CPDF_Annot::AppearanceMode::kNormal, nullptr);
}
void CFFL_Button::OnDrawDeactive(CPDFSDK_PageView* pPageView,
diff --git a/fpdfsdk/formfiller/cffl_formfield.cpp b/fpdfsdk/formfiller/cffl_formfield.cpp
index 7718c4e..531d5af 100644
--- a/fpdfsdk/formfiller/cffl_formfield.cpp
+++ b/fpdfsdk/formfiller/cffl_formfield.cpp
@@ -68,15 +68,16 @@
if (!CFFL_InteractiveFormFiller::IsVisible(pWidget))
return;
- pWidget->DrawAppearance(pDevice, mtUser2Device, CPDF_Annot::Normal, nullptr);
+ pWidget->DrawAppearance(pDevice, mtUser2Device,
+ CPDF_Annot::AppearanceMode::kNormal, nullptr);
}
void CFFL_FormField::OnDrawDeactive(CPDFSDK_PageView* pPageView,
CPDFSDK_Annot* pAnnot,
CFX_RenderDevice* pDevice,
const CFX_Matrix& mtUser2Device) {
- ToCPDFSDKWidget(pAnnot)->DrawAppearance(pDevice, mtUser2Device,
- CPDF_Annot::Normal, nullptr);
+ ToCPDFSDKWidget(pAnnot)->DrawAppearance(
+ pDevice, mtUser2Device, CPDF_Annot::AppearanceMode::kNormal, nullptr);
}
void CFFL_FormField::OnMouseEnter(CPDFSDK_PageView* pPageView) {}
diff --git a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
index 75a080e..f86fa53 100644
--- a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
+++ b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
@@ -85,8 +85,8 @@
if (pFormField) {
pFormField->OnDrawDeactive(pPageView, pAnnot, pDevice, mtUser2Device);
} else {
- pWidget->DrawAppearance(pDevice, mtUser2Device, CPDF_Annot::Normal,
- nullptr);
+ pWidget->DrawAppearance(pDevice, mtUser2Device,
+ CPDF_Annot::AppearanceMode::kNormal, nullptr);
}
if (!IsReadOnly(pWidget) && IsFillingAllowed(pWidget))
diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp
index 0de6c6b..803e274 100644
--- a/fpdfsdk/fpdf_annot.cpp
+++ b/fpdfsdk/fpdf_annot.cpp
@@ -123,13 +123,13 @@
// These checks ensure the consistency of annotation appearance mode values
// across core/ and public.
-static_assert(static_cast<int>(CPDF_Annot::AppearanceMode::Normal) ==
+static_assert(static_cast<int>(CPDF_Annot::AppearanceMode::kNormal) ==
FPDF_ANNOT_APPEARANCEMODE_NORMAL,
"CPDF_Annot::AppearanceMode::Normal value mismatch");
-static_assert(static_cast<int>(CPDF_Annot::AppearanceMode::Rollover) ==
+static_assert(static_cast<int>(CPDF_Annot::AppearanceMode::kRollover) ==
FPDF_ANNOT_APPEARANCEMODE_ROLLOVER,
"CPDF_Annot::AppearanceMode::Rollover value mismatch");
-static_assert(static_cast<int>(CPDF_Annot::AppearanceMode::Down) ==
+static_assert(static_cast<int>(CPDF_Annot::AppearanceMode::kDown) ==
FPDF_ANNOT_APPEARANCEMODE_DOWN,
"CPDF_Annot::AppearanceMode::Down value mismatch");
@@ -162,7 +162,7 @@
"CPDF_Object::kReference value mismatch");
bool HasAPStream(CPDF_Dictionary* pAnnotDict) {
- return !!GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+ return !!GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::kNormal);
}
void UpdateContentStream(CPDF_Form* pForm, CPDF_Stream* pStream) {
@@ -212,7 +212,7 @@
// Update BBox entry in appearance stream based on the bounding rectangle
// of the annotation's quadpoints.
CPDF_Stream* pStream =
- GetAnnotAP(annot_dict, CPDF_Annot::AppearanceMode::Normal);
+ GetAnnotAP(annot_dict, CPDF_Annot::AppearanceMode::kNormal);
if (pStream) {
CFX_FloatRect boundingRect =
CPDF_Annot::BoundingRectFromQuadPoints(annot_dict);
@@ -459,7 +459,7 @@
// Check that the annotation already has an appearance stream, since an
// existing object is to be updated.
CPDF_Stream* pStream =
- GetAnnotAP(pAnnot->GetAnnotDict(), CPDF_Annot::AppearanceMode::Normal);
+ GetAnnotAP(pAnnot->GetAnnotDict(), CPDF_Annot::AppearanceMode::kNormal);
if (!pStream)
return false;
@@ -532,11 +532,11 @@
// If the annotation does not have an AP stream yet, generate and set it.
CPDF_Dictionary* pAnnotDict = pAnnot->GetAnnotDict();
CPDF_Stream* pStream =
- GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+ GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::kNormal);
if (!pStream) {
CPDF_GenerateAP::GenerateEmptyAP(pAnnot->GetPage()->GetDocument(),
pAnnotDict);
- pStream = GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+ pStream = GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::kNormal);
if (!pStream)
return false;
}
@@ -574,7 +574,7 @@
if (!pAnnot->HasForm()) {
CPDF_Stream* pStream =
- GetAnnotAP(pAnnot->GetAnnotDict(), CPDF_Annot::AppearanceMode::Normal);
+ GetAnnotAP(pAnnot->GetAnnotDict(), CPDF_Annot::AppearanceMode::kNormal);
if (!pStream)
return 0;
@@ -591,7 +591,7 @@
if (!pAnnot->HasForm()) {
CPDF_Stream* pStream =
- GetAnnotAP(pAnnot->GetAnnotDict(), CPDF_Annot::AppearanceMode::Normal);
+ GetAnnotAP(pAnnot->GetAnnotDict(), CPDF_Annot::AppearanceMode::kNormal);
if (!pStream)
return nullptr;
@@ -615,7 +615,7 @@
// Check that the annotation already has an appearance stream, since an
// existing object is to be deleted.
CPDF_Stream* pStream =
- GetAnnotAP(pAnnot->GetAnnotDict(), CPDF_Annot::AppearanceMode::Normal);
+ GetAnnotAP(pAnnot->GetAnnotDict(), CPDF_Annot::AppearanceMode::kNormal);
if (!pStream)
return false;
@@ -814,7 +814,7 @@
return true;
CPDF_Stream* pStream =
- GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+ GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::kNormal);
if (pStream && newRect.Contains(pStream->GetDict()->GetRectFor("BBox")))
pStream->GetDict()->SetRectFor("BBox", newRect);
return true;