From 55be02e2d6deb857fcf404c78b3633a3eac57e6c Mon Sep 17 00:00:00 2001 From: tsepez Date: Mon, 12 Sep 2016 11:21:42 -0700 Subject: Make wrapped JS Document observe C++-side object. First of several patches to ensure JS objects don't track stale C++ objects. Review-Url: https://codereview.chromium.org/2323903002 --- fpdfsdk/include/fsdk_mgr.h | 3 +- fpdfsdk/javascript/Document.cpp | 186 ++++++++++++++++++++++++++++++---------- fpdfsdk/javascript/Document.h | 5 +- 3 files changed, 144 insertions(+), 50 deletions(-) diff --git a/fpdfsdk/include/fsdk_mgr.h b/fpdfsdk/include/fsdk_mgr.h index f37e64fe41..38b06a9b5a 100644 --- a/fpdfsdk/include/fsdk_mgr.h +++ b/fpdfsdk/include/fsdk_mgr.h @@ -14,6 +14,7 @@ #include "core/fpdfapi/fpdf_page/include/cpdf_page.h" #include "core/fpdfapi/fpdf_parser/include/cpdf_document.h" #include "core/fpdfdoc/include/cpdf_occontext.h" +#include "core/fxcrt/include/cfx_observable.h" #include "fpdfsdk/cfx_systemhandler.h" #include "fpdfsdk/include/cpdfsdk_annot.h" #include "fpdfsdk/include/fsdk_actionhandler.h" @@ -444,7 +445,7 @@ class CPDFDoc_Environment final { std::unique_ptr m_pSysHandler; }; -class CPDFSDK_Document { +class CPDFSDK_Document : public CFX_Observable { public: static CPDFSDK_Document* FromFPDFFormHandle(FPDF_FORMHANDLE hHandle); diff --git a/fpdfsdk/javascript/Document.cpp b/fpdfsdk/javascript/Document.cpp index cb79ab4589..fe623e960f 100644 --- a/fpdfsdk/javascript/Document.cpp +++ b/fpdfsdk/javascript/Document.cpp @@ -166,6 +166,10 @@ FX_BOOL Document::numFields(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSREADONLY); return FALSE; } + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CPDFSDK_InterForm* pInterForm = m_pDocument->GetInterForm(); CPDF_InterForm* pPDFForm = pInterForm->GetInterForm(); vp << (int)pPDFForm->CountFields(); @@ -175,14 +179,14 @@ FX_BOOL Document::numFields(IJS_Context* cc, FX_BOOL Document::dirty(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError) { + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } if (vp.IsGetting()) { - if (m_pDocument->GetChangeMark()) - vp << true; - else - vp << false; + vp << !!m_pDocument->GetChangeMark(); } else { bool bChanged = false; - vp >> bChanged; if (bChanged) @@ -190,7 +194,6 @@ FX_BOOL Document::dirty(IJS_Context* cc, else m_pDocument->ClearChangeMark(); } - return TRUE; } @@ -206,6 +209,10 @@ FX_BOOL Document::ADBE(IJS_Context* cc, FX_BOOL Document::pageNum(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError) { + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } if (vp.IsGetting()) { if (CPDFSDK_PageView* pPageView = m_pDocument->GetCurrentView()) { vp << pPageView->GetPageIndex(); @@ -281,7 +288,10 @@ FX_BOOL Document::getField(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSPARAMERROR); return FALSE; } - + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CJS_Context* pContext = static_cast(cc); CJS_Runtime* pRuntime = pContext->GetJSRuntime(); CFX_WideString wideName = params[0].ToCFXWideString(pRuntime); @@ -312,6 +322,10 @@ FX_BOOL Document::getNthFieldName(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSPARAMERROR); return FALSE; } + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CJS_Context* pContext = static_cast(cc); CJS_Runtime* pRuntime = pContext->GetJSRuntime(); int nIndex = params[0].ToInt(pRuntime); @@ -364,12 +378,18 @@ FX_BOOL Document::mailForm(IJS_Context* cc, const std::vector& params, CJS_Value& vRet, CFX_WideString& sError) { + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } + if (!m_pDocument->GetPermissions(FPDFPERM_EXTRACT_ACCESS)) { + sError = JSGetStringFromID(IDS_STRING_JSNOPERMISSION); + return FALSE; + } + CJS_Context* pContext = static_cast(cc); CJS_Runtime* pRuntime = pContext->GetJSRuntime(); - if (!m_pDocument->GetPermissions(FPDFPERM_EXTRACT_ACCESS)) - return FALSE; - int iLength = params.size(); FX_BOOL bUI = iLength > 0 ? params[0].ToBool(pRuntime) : TRUE; CFX_WideString cTo = iLength > 1 ? params[1].ToCFXWideString(pRuntime) : L""; @@ -397,6 +417,11 @@ FX_BOOL Document::print(IJS_Context* cc, const std::vector& params, CJS_Value& vRet, CFX_WideString& sError) { + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } + CJS_Context* pContext = static_cast(cc); CJS_Runtime* pRuntime = pContext->GetJSRuntime(); @@ -465,15 +490,19 @@ FX_BOOL Document::removeField(IJS_Context* cc, const std::vector& params, CJS_Value& vRet, CFX_WideString& sError) { + if (params.size() != 1) { + sError = JSGetStringFromID(IDS_STRING_JSPARAMERROR); + return FALSE; + } + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } if (!(m_pDocument->GetPermissions(FPDFPERM_MODIFY) || m_pDocument->GetPermissions(FPDFPERM_ANNOT_FORM))) { sError = JSGetStringFromID(IDS_STRING_JSNOPERMISSION); return FALSE; } - if (params.size() != 1) { - sError = JSGetStringFromID(IDS_STRING_JSPARAMERROR); - return FALSE; - } CJS_Context* pContext = static_cast(cc); CJS_Runtime* pRuntime = pContext->GetJSRuntime(); CFX_WideString sFieldName = params[0].ToCFXWideString(pRuntime); @@ -516,12 +545,16 @@ FX_BOOL Document::resetForm(IJS_Context* cc, const std::vector& params, CJS_Value& vRet, CFX_WideString& sError) { - CJS_Context* pContext = static_cast(cc); - + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } if (!(m_pDocument->GetPermissions(FPDFPERM_MODIFY) || m_pDocument->GetPermissions(FPDFPERM_ANNOT_FORM) || - m_pDocument->GetPermissions(FPDFPERM_FILL_FORM))) + m_pDocument->GetPermissions(FPDFPERM_FILL_FORM))) { + sError = JSGetStringFromID(IDS_STRING_JSNOPERMISSION); return FALSE; + } CPDFSDK_InterForm* pInterForm = m_pDocument->GetInterForm(); CPDF_InterForm* pPDFForm = pInterForm->GetInterForm(); @@ -533,6 +566,7 @@ FX_BOOL Document::resetForm(IJS_Context* cc, return TRUE; } + CJS_Context* pContext = static_cast(cc); CJS_Runtime* pRuntime = pContext->GetJSRuntime(); switch (params[0].GetType()) { @@ -586,6 +620,10 @@ FX_BOOL Document::submitForm(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSPARAMERROR); return FALSE; } + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CJS_Context* pContext = static_cast(cc); CJS_Runtime* pRuntime = pContext->GetJSRuntime(); CJS_Array aFields; @@ -653,11 +691,7 @@ FX_BOOL Document::submitForm(IJS_Context* cc, } void Document::AttachDoc(CPDFSDK_Document* pDoc) { - m_pDocument = pDoc; -} - -CPDFSDK_Document* Document::GetReaderDoc() { - return m_pDocument; + m_pDocument.Reset(pDoc); } FX_BOOL Document::bookmarkRoot(IJS_Context* cc, @@ -740,7 +774,10 @@ FX_BOOL Document::info(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSREADONLY); return FALSE; } - + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CPDF_Dictionary* pDictionary = m_pDocument->GetPDFDocument()->GetInfo(); if (!pDictionary) return FALSE; @@ -790,6 +827,10 @@ FX_BOOL Document::getPropertyInternal(IJS_Context* cc, CJS_PropValue& vp, const CFX_ByteString& propName, CFX_WideString& sError) { + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CPDF_Dictionary* pDictionary = m_pDocument->GetPDFDocument()->GetInfo(); if (!pDictionary) return FALSE; @@ -797,9 +838,10 @@ FX_BOOL Document::getPropertyInternal(IJS_Context* cc, if (vp.IsGetting()) { vp << pDictionary->GetUnicodeTextBy(propName); } else { - if (!m_pDocument->GetPermissions(FPDFPERM_MODIFY)) + if (!m_pDocument->GetPermissions(FPDFPERM_MODIFY)) { + sError = JSGetStringFromID(IDS_STRING_JSNOPERMISSION); return FALSE; - + } CFX_WideString csProperty; vp >> csProperty; pDictionary->SetAtString(propName, PDF_EncodeText(csProperty)); @@ -823,12 +865,17 @@ FX_BOOL Document::creator(IJS_Context* cc, FX_BOOL Document::delay(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError) { + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } if (vp.IsGetting()) { vp << m_bDelay; } else { - if (!m_pDocument->GetPermissions(FPDFPERM_MODIFY)) + if (!m_pDocument->GetPermissions(FPDFPERM_MODIFY)) { + sError = JSGetStringFromID(IDS_STRING_JSNOPERMISSION); return FALSE; - + } vp >> m_bDelay; if (m_bDelay) { m_DelayData.clear(); @@ -836,7 +883,7 @@ FX_BOOL Document::delay(IJS_Context* cc, std::list> DelayDataToProcess; DelayDataToProcess.swap(m_DelayData); for (const auto& pData : DelayDataToProcess) - Field::DoDelay(m_pDocument, pData.get()); + Field::DoDelay(m_pDocument.Get(), pData.get()); } } return TRUE; @@ -869,9 +916,10 @@ FX_BOOL Document::subject(IJS_Context* cc, FX_BOOL Document::title(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError) { - if (!m_pDocument || !m_pDocument->GetUnderlyingDocument()) + if (!m_pDocument || !m_pDocument->GetUnderlyingDocument()) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); return FALSE; - + } return getPropertyInternal(cc, vp, "Title", sError); } @@ -882,6 +930,10 @@ FX_BOOL Document::numPages(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSREADONLY); return FALSE; } + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } vp << m_pDocument->GetPageCount(); return TRUE; } @@ -926,6 +978,10 @@ FX_BOOL Document::URL(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSREADONLY); return FALSE; } + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } vp << m_pDocument->GetPath(); return TRUE; } @@ -944,19 +1000,18 @@ FX_BOOL Document::baseURL(IJS_Context* cc, FX_BOOL Document::calculate(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError) { + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CPDFSDK_InterForm* pInterForm = m_pDocument->GetInterForm(); if (vp.IsGetting()) { - if (pInterForm->IsCalculateEnabled()) - vp << true; - else - vp << false; + vp << !!pInterForm->IsCalculateEnabled(); } else { bool bCalculate; vp >> bCalculate; - pInterForm->EnableCalculate(bCalculate); } - return TRUE; } @@ -967,6 +1022,10 @@ FX_BOOL Document::documentFileName(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSREADONLY); return FALSE; } + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CFX_WideString wsFilePath = m_pDocument->GetPath(); int32_t i = wsFilePath.GetLength() - 1; for (; i >= 0; i--) { @@ -988,6 +1047,10 @@ FX_BOOL Document::path(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSREADONLY); return FALSE; } + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } vp << app::SysPathToPDFPath(m_pDocument->GetPath()); return TRUE; } @@ -1033,6 +1096,10 @@ FX_BOOL Document::getAnnot(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSPARAMERROR); return FALSE; } + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CJS_Context* pContext = static_cast(cc); CJS_Runtime* pRuntime = pContext->GetJSRuntime(); int nPageNo = params[0].ToInt(pRuntime); @@ -1078,13 +1145,16 @@ FX_BOOL Document::getAnnots(IJS_Context* cc, const std::vector& params, CJS_Value& vRet, CFX_WideString& sError) { + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CJS_Context* pContext = static_cast(cc); + CJS_Runtime* pRuntime = pContext->GetJSRuntime(); // TODO(tonikitoo): Add support supported parameters as per // the PDF spec. - CJS_Runtime* pRuntime = pContext->GetJSRuntime(); - int nPageNo = m_pDocument->GetPageCount(); CJS_Array annots; @@ -1303,13 +1373,17 @@ FX_BOOL Document::calculateNow(IJS_Context* cc, const std::vector& params, CJS_Value& vRet, CFX_WideString& sError) { + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } if (!(m_pDocument->GetPermissions(FPDFPERM_MODIFY) || m_pDocument->GetPermissions(FPDFPERM_ANNOT_FORM) || - m_pDocument->GetPermissions(FPDFPERM_FILL_FORM))) + m_pDocument->GetPermissions(FPDFPERM_FILL_FORM))) { + sError = JSGetStringFromID(IDS_STRING_JSNOPERMISSION); return FALSE; - - CPDFSDK_InterForm* pInterForm = m_pDocument->GetInterForm(); - pInterForm->OnCalculate(); + } + m_pDocument->GetInterForm()->OnCalculate(); return TRUE; } @@ -1323,12 +1397,18 @@ FX_BOOL Document::getPageNthWord(IJS_Context* cc, const std::vector& params, CJS_Value& vRet, CFX_WideString& sError) { - // TODO(tsepez): check maximum allowable params. + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } if (!m_pDocument->GetPermissions(FPDFPERM_EXTRACT_ACCESS)) { sError = JSGetStringFromID(IDS_STRING_JSNOPERMISSION); return FALSE; } CJS_Runtime* pRuntime = CJS_Runtime::FromContext(cc); + + // TODO(tsepez): check maximum allowable params. + int nPageNo = params.size() > 0 ? params[0].ToInt(pRuntime) : 0; int nWordNo = params.size() > 1 ? params[1].ToInt(pRuntime) : 0; bool bStrip = params.size() > 2 ? params[2].ToBool(pRuntime) : true; @@ -1376,9 +1456,14 @@ FX_BOOL Document::getPageNthWordQuads(IJS_Context* cc, const std::vector& params, CJS_Value& vRet, CFX_WideString& sError) { - if (!m_pDocument->GetPermissions(FPDFPERM_EXTRACT_ACCESS)) + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); return FALSE; - + } + if (!m_pDocument->GetPermissions(FPDFPERM_EXTRACT_ACCESS)) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } return FALSE; } @@ -1386,6 +1471,10 @@ FX_BOOL Document::getPageNumWords(IJS_Context* cc, const std::vector& params, CJS_Value& vRet, CFX_WideString& sError) { + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } if (!m_pDocument->GetPermissions(FPDFPERM_EXTRACT_ACCESS)) { sError = JSGetStringFromID(IDS_STRING_JSNOPERMISSION); return FALSE; @@ -1572,7 +1661,10 @@ FX_BOOL Document::gotoNamedDest(IJS_Context* cc, sError = JSGetStringFromID(IDS_STRING_JSPARAMERROR); return FALSE; } - + if (!m_pDocument) { + sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT); + return FALSE; + } CJS_Runtime* pRuntime = CJS_Runtime::FromContext(cc); CFX_WideString wideName = params[0].ToCFXWideString(pRuntime); CFX_ByteString utf8Name = wideName.UTF8Encode(); @@ -1626,7 +1718,7 @@ void Document::DoFieldDelay(const CFX_WideString& sFieldName, } for (const auto& pData : DelayDataForFieldAndControlIndex) - Field::DoDelay(m_pDocument, pData.get()); + Field::DoDelay(m_pDocument.Get(), pData.get()); } CJS_Document* Document::GetCJSDoc() const { diff --git a/fpdfsdk/javascript/Document.h b/fpdfsdk/javascript/Document.h index feef228c6e..a35358d4df 100644 --- a/fpdfsdk/javascript/Document.h +++ b/fpdfsdk/javascript/Document.h @@ -13,6 +13,7 @@ #include "core/fpdfapi/fpdf_page/include/cpdf_pageobject.h" #include "core/fpdfapi/fpdf_page/include/cpdf_textobject.h" +#include "fpdfsdk/include/fsdk_mgr.h" #include "fpdfsdk/javascript/JS_Define.h" class PrintParamsObj : public CJS_EmbedObj { @@ -270,7 +271,7 @@ class Document : public CJS_EmbedObj { FX_BOOL URL(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError); void AttachDoc(CPDFSDK_Document* pDoc); - CPDFSDK_Document* GetReaderDoc(); + CPDFSDK_Document* GetReaderDoc() const { return m_pDocument.Get(); } void AddDelayData(CJS_DelayData* pData); void DoFieldDelay(const CFX_WideString& sFieldName, int nControlIndex); CJS_Document* GetCJSDoc() const; @@ -285,7 +286,7 @@ class Document : public CJS_EmbedObj { const CFX_ByteString& propName, CFX_WideString& sError); - CPDFSDK_Document* m_pDocument; + CPDFSDK_Document::ObservedPtr m_pDocument; CFX_WideString m_cwBaseURL; std::list> m_DelayData; std::list> m_IconList; -- cgit v1.2.3