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);
}