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;