Tidy CJS_EventRecorder

- make m_pValue private.
- add HasValue() method.
- move simple accessors to header file.
- pack fields somewhat tighter while keeping like items grouped.

Change-Id: Id66d32b4896acfca6d3c22184756cfd9a1065ca0
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/55912
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fxjs/cjs_event.cpp b/fxjs/cjs_event.cpp
index 6399f13..a16ee6c 100644
--- a/fxjs/cjs_event.cpp
+++ b/fxjs/cjs_event.cpp
@@ -283,7 +283,7 @@
   if (pEvent->Type() != "Field")
     return CJS_Result::Failure(L"Bad event type.");
 
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(JSMessage::kBadObjectError);
 
   return CJS_Result::Success(
@@ -297,7 +297,7 @@
   if (pEvent->Type() != "Field")
     return CJS_Result::Failure(L"Bad event type.");
 
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(JSMessage::kBadObjectError);
 
   if (vp.IsEmpty())
diff --git a/fxjs/cjs_eventrecorder.cpp b/fxjs/cjs_eventrecorder.cpp
index ad67b8b..af752b5 100644
--- a/fxjs/cjs_eventrecorder.cpp
+++ b/fxjs/cjs_eventrecorder.cpp
@@ -363,7 +363,6 @@
 
 void CJS_EventRecorder::Initialize(JS_EVENT_T type) {
   m_eEventType = type;
-
   m_strTargetName.clear();
   m_strSourceName.clear();
   m_pWideStrChange = nullptr;
@@ -382,11 +381,9 @@
   m_bFieldFull = false;
   m_pbRc = nullptr;
   m_bRcDu = false;
-
   m_pTargetBookMark = nullptr;
   m_pTargetFormFillEnv.Reset();
   m_pTargetAnnot.Reset();
-
   m_bValid = true;
 }
 
@@ -394,10 +391,6 @@
   m_bValid = false;
 }
 
-bool CJS_EventRecorder::IsValid() const {
-  return m_bValid;
-}
-
 bool CJS_EventRecorder::IsUserGesture() const {
   switch (m_eEventType) {
     case JET_FIELD_MOUSEDOWN:
@@ -414,30 +407,7 @@
 }
 
 WideString& CJS_EventRecorder::Change() {
-  if (m_pWideStrChange) {
-    return *m_pWideStrChange;
-  }
-  return m_WideStrChangeDu;
-}
-
-WideString CJS_EventRecorder::ChangeEx() const {
-  return m_WideStrChangeEx;
-}
-
-int CJS_EventRecorder::CommitKey() const {
-  return m_nCommitKey;
-}
-
-bool CJS_EventRecorder::FieldFull() const {
-  return m_bFieldFull;
-}
-
-bool CJS_EventRecorder::KeyDown() const {
-  return m_bKeyDown;
-}
-
-bool CJS_EventRecorder::Modifier() const {
-  return m_bModifier;
+  return m_pWideStrChange ? *m_pWideStrChange : m_WideStrChangeDu;
 }
 
 ByteStringView CJS_EventRecorder::Name() const {
@@ -587,15 +557,3 @@
   int& target = m_pISelStart ? *m_pISelStart : m_nSelStartDu;
   target = value;
 }
-
-bool CJS_EventRecorder::Shift() const {
-  return m_bShift;
-}
-
-WideString& CJS_EventRecorder::Value() {
-  return *m_pValue;
-}
-
-bool CJS_EventRecorder::WillCommit() const {
-  return m_bWillCommit;
-}
diff --git a/fxjs/cjs_eventrecorder.h b/fxjs/cjs_eventrecorder.h
index af0b467..5d52edc 100644
--- a/fxjs/cjs_eventrecorder.h
+++ b/fxjs/cjs_eventrecorder.h
@@ -138,18 +138,18 @@
   void OnExternal_Exec();
 
   void Destroy();
-  bool IsValid() const;
 
   JS_EVENT_T EventType() const { return m_eEventType; }
+  bool IsValid() const { return m_bValid; }
   bool IsUserGesture() const;
   WideString& Change();
-  WideString ChangeEx() const;
+  WideString ChangeEx() const { return m_WideStrChangeEx; }
   WideString SourceName() const { return m_strSourceName; }
   WideString TargetName() const { return m_strTargetName; }
-  int CommitKey() const;
-  bool FieldFull() const;
-  bool KeyDown() const;
-  bool Modifier() const;
+  int CommitKey() const { return m_nCommitKey; }
+  bool FieldFull() const { return m_bFieldFull; }
+  bool KeyDown() const { return m_bKeyDown; }
+  bool Modifier() const { return m_bModifier; }
   ByteStringView Name() const;
   ByteStringView Type() const;
   bool& Rc();
@@ -157,42 +157,44 @@
   int SelStart() const;
   void SetSelEnd(int value);
   void SetSelStart(int value);
-  bool Shift() const;
-  WideString& Value();
-  bool WillCommit() const;
+  bool Shift() const { return m_bShift; }
+  bool HasValue() const { return !!m_pValue; }
+  WideString& Value() { return *m_pValue; }
+  bool WillCommit() const { return m_bWillCommit; }
   CPDFSDK_FormFillEnvironment* GetFormFillEnvironment() const {
     return m_pTargetFormFillEnv.Get();
   }
+
+  void SetValueForTest(WideString* pStr) { m_pValue = pStr; }
   void SetRCForTest(bool* pRC) { m_pbRc = pRC; }
   void SetStrChangeForTest(WideString* pStrChange) {
     m_pWideStrChange = pStrChange;
   }
   void ResetWillCommitForTest() { m_bWillCommit = false; }
 
-  UnownedPtr<WideString> m_pValue;
-
  private:
   void Initialize(JS_EVENT_T type);
 
   JS_EVENT_T m_eEventType = JET_UNKNOWN;
   bool m_bValid = false;
+  UnownedPtr<WideString> m_pValue;
   WideString m_strSourceName;
   WideString m_strTargetName;
-  UnownedPtr<WideString> m_pWideStrChange;
   WideString m_WideStrChangeDu;
   WideString m_WideStrChangeEx;
+  UnownedPtr<WideString> m_pWideStrChange;
   int m_nCommitKey = -1;
   bool m_bKeyDown = false;
   bool m_bModifier = false;
   bool m_bShift = false;
-  UnownedPtr<int> m_pISelEnd;
   int m_nSelEndDu = 0;
-  UnownedPtr<int> m_pISelStart;
   int m_nSelStartDu = 0;
+  UnownedPtr<int> m_pISelEnd;
+  UnownedPtr<int> m_pISelStart;
   bool m_bWillCommit = false;
   bool m_bFieldFull = false;
-  UnownedPtr<bool> m_pbRc;
   bool m_bRcDu = false;
+  UnownedPtr<bool> m_pbRc;
   UnownedPtr<CPDF_Bookmark> m_pTargetBookMark;
   CPDFSDK_FormFillEnvironment::ObservedPtr m_pTargetFormFillEnv;
   CPDFSDK_Annot::ObservedPtr m_pTargetAnnot;
diff --git a/fxjs/cjs_publicmethods.cpp b/fxjs/cjs_publicmethods.cpp
index 50d25b3..b97c2af 100644
--- a/fxjs/cjs_publicmethods.cpp
+++ b/fxjs/cjs_publicmethods.cpp
@@ -589,7 +589,7 @@
 
   CJS_EventContext* pEventContext = pRuntime->GetCurrentEventContext();
   CJS_EventRecorder* pEvent = pEventContext->GetEventRecorder();
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(WideString::FromASCII("No event handler"));
 
   WideString& Value = pEvent->Value();
@@ -696,7 +696,7 @@
 
   CJS_EventContext* pContext = pRuntime->GetCurrentEventContext();
   CJS_EventRecorder* pEvent = pContext->GetEventRecorder();
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(JSMessage::kBadObjectError);
 
   WideString& val = pEvent->Value();
@@ -785,7 +785,7 @@
 
   CJS_EventRecorder* pEvent =
       pRuntime->GetCurrentEventContext()->GetEventRecorder();
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(JSMessage::kBadObjectError);
 
   // Acrobat will accept this. Anything larger causes it to throw an error.
@@ -881,7 +881,7 @@
 
   CJS_EventContext* pContext = pRuntime->GetCurrentEventContext();
   CJS_EventRecorder* pEvent = pContext->GetEventRecorder();
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(JSMessage::kBadObjectError);
 
   WideString& val = pEvent->Value();
@@ -959,7 +959,7 @@
   if (!pEvent->WillCommit())
     return CJS_Result::Success();
 
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(JSMessage::kBadObjectError);
 
   const WideString& strValue = pEvent->Value();
@@ -1053,7 +1053,7 @@
 
   CJS_EventRecorder* pEvent =
       pRuntime->GetCurrentEventContext()->GetEventRecorder();
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(JSMessage::kBadObjectError);
 
   const WideString& wsSource = pEvent->Value();
@@ -1089,7 +1089,7 @@
 
   CJS_EventContext* pContext = pRuntime->GetCurrentEventContext();
   CJS_EventRecorder* pEvent = pContext->GetEventRecorder();
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(JSMessage::kBadObjectError);
 
   const WideString& valEvent = pEvent->Value();
@@ -1171,7 +1171,7 @@
 
   CJS_EventRecorder* pEvent =
       pRuntime->GetCurrentEventContext()->GetEventRecorder();
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(JSMessage::kBadObjectError);
 
   const char* cFormat = "";
@@ -1208,7 +1208,7 @@
       pRuntime->GetCurrentEventContext()->GetEventRecorder();
 
   WideString swValue;
-  if (pEventRecorder->m_pValue)
+  if (pEventRecorder->HasValue())
     swValue = pEventRecorder->Value();
 
   if (pEventRecorder->WillCommit())
@@ -1364,7 +1364,7 @@
   dValue = floor(dValue * FXSYS_pow(10, 6) + 0.49) / FXSYS_pow(10, 6);
 
   CJS_EventContext* pContext = pRuntime->GetCurrentEventContext();
-  if (pContext->GetEventRecorder()->m_pValue) {
+  if (pContext->GetEventRecorder()->HasValue()) {
     pContext->GetEventRecorder()->Value() =
         pRuntime->ToWideString(pRuntime->NewNumber(dValue));
   }
@@ -1382,7 +1382,7 @@
 
   CJS_EventContext* pContext = pRuntime->GetCurrentEventContext();
   CJS_EventRecorder* pEvent = pContext->GetEventRecorder();
-  if (!pEvent->m_pValue)
+  if (!pEvent->HasValue())
     return CJS_Result::Failure(JSMessage::kBadObjectError);
 
   if (pEvent->Value().IsEmpty())
diff --git a/fxjs/cjs_publicmethods_embeddertest.cpp b/fxjs/cjs_publicmethods_embeddertest.cpp
index aefcb63..4fbe2d3 100644
--- a/fxjs/cjs_publicmethods_embeddertest.cpp
+++ b/fxjs/cjs_publicmethods_embeddertest.cpp
@@ -190,7 +190,8 @@
   runtime.NewEventContext();
 
   WideString result;
-  runtime.GetCurrentEventContext()->GetEventRecorder()->m_pValue = &result;
+  runtime.GetCurrentEventContext()->GetEventRecorder()->SetValueForTest(
+      &result);
 
   auto ary = runtime.NewArray();
   runtime.PutArrayElement(ary, 0, runtime.NewString("Calc1_A"));
@@ -203,7 +204,8 @@
   CJS_Result ret = CJS_PublicMethods::AFSimple_Calculate(&runtime, params);
   UnloadPage(page);
 
-  runtime.GetCurrentEventContext()->GetEventRecorder()->m_pValue = nullptr;
+  runtime.GetCurrentEventContext()->GetEventRecorder()->SetValueForTest(
+      nullptr);
 
   ASSERT_TRUE(!ret.HasError());
   ASSERT_TRUE(!ret.HasReturn());
@@ -229,7 +231,7 @@
   WideString result = L"-10";
   WideString change = L"";
 
-  handler->m_pValue = &result;
+  handler->SetValueForTest(&result);
   handler->SetRCForTest(&valid);
   handler->SetStrChangeForTest(&change);
 
@@ -250,7 +252,7 @@
 
   // Keep the *SAN bots happy. One of these is an UnownedPtr, another seems to
   // used during destruction. Clear them all to be safe and consistent.
-  handler->m_pValue = nullptr;
+  handler->SetValueForTest(nullptr);
   handler->SetRCForTest(nullptr);
   handler->SetStrChangeForTest(nullptr);
 }