From 5daf07afe5b76e702d053aaca648b977ec3bb663 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 24 Oct 2017 21:46:57 -0400 Subject: Make NewNull return an actual Null This CL updates the CFXJS_Engine::NewNull method to return a real v8::Null instead of an empty v8::Local. This also adds a NewUndefined and returns undefined in most of the places null was returned previously. Change-Id: If1a96bf253057892a3b709cbc72f8825c52503c3 Reviewed-on: https://pdfium-review.googlesource.com/16730 Reviewed-by: Tom Sepez Commit-Queue: dsinclair --- fpdfsdk/javascript/Document.cpp | 8 ++++---- fpdfsdk/javascript/PublicMethods.cpp | 2 +- fpdfsdk/javascript/app.cpp | 2 +- fpdfsdk/javascript/global.cpp | 11 ++++------- fpdfsdk/javascript/util.cpp | 2 +- fxjs/fxjs_v8.cpp | 6 +++++- fxjs/fxjs_v8.h | 1 + fxjs/fxjs_v8_embeddertest.cpp | 16 +++++++++++++++- 8 files changed, 32 insertions(+), 16 deletions(-) diff --git a/fpdfsdk/javascript/Document.cpp b/fpdfsdk/javascript/Document.cpp index 9404ded0c8..2217f448f2 100644 --- a/fpdfsdk/javascript/Document.cpp +++ b/fpdfsdk/javascript/Document.cpp @@ -213,7 +213,7 @@ bool Document::set_dirty(CJS_Runtime* pRuntime, bool Document::get_ADBE(CJS_Runtime* pRuntime, CJS_Value* vp, WideString* sError) { - vp->Set(pRuntime->NewNull()); + vp->Set(pRuntime->NewUndefined()); return true; } @@ -312,7 +312,7 @@ bool Document::getField(CJS_Runtime* pRuntime, CPDFSDK_InterForm* pInterForm = m_pFormFillEnv->GetInterForm(); CPDF_InterForm* pPDFForm = pInterForm->GetInterForm(); if (pPDFForm->CountFields(wideName) <= 0) { - vRet.Set(pRuntime->NewNull()); + vRet.Set(pRuntime->NewUndefined()); return true; } @@ -1321,7 +1321,7 @@ bool Document::getAnnot3D(CJS_Runtime* pRuntime, const std::vector>& params, CJS_Value& vRet, WideString& sError) { - vRet.Set(pRuntime->NewNull()); + vRet.Set(pRuntime->NewUndefined()); return true; } @@ -1387,7 +1387,7 @@ bool Document::get_icons(CJS_Runtime* pRuntime, CJS_Value* vp, WideString* sError) { if (m_IconNames.empty()) { - vp->Set(pRuntime->NewNull()); + vp->Set(pRuntime->NewUndefined()); return true; } diff --git a/fpdfsdk/javascript/PublicMethods.cpp b/fpdfsdk/javascript/PublicMethods.cpp index c0441a82a3..667127fe88 100644 --- a/fpdfsdk/javascript/PublicMethods.cpp +++ b/fpdfsdk/javascript/PublicMethods.cpp @@ -1840,7 +1840,7 @@ bool CJS_PublicMethods::AFExtractNums( else vRet = CJS_Value(nums.ToV8Value()); } else { - vRet.Set(pRuntime->NewNull()); + vRet.Set(pRuntime->NewUndefined()); } return true; diff --git a/fpdfsdk/javascript/app.cpp b/fpdfsdk/javascript/app.cpp index ca440f64b3..cbc3145b0f 100644 --- a/fpdfsdk/javascript/app.cpp +++ b/fpdfsdk/javascript/app.cpp @@ -237,7 +237,7 @@ bool app::get_active_docs(CJS_Runtime* pRuntime, else vp->Set(aDocs.ToV8Value()); } else { - vp->Set(pRuntime->NewNull()); + vp->Set(pRuntime->NewUndefined()); } return true; diff --git a/fpdfsdk/javascript/global.cpp b/fpdfsdk/javascript/global.cpp index 040c6e6067..5dc6ac9d78 100644 --- a/fpdfsdk/javascript/global.cpp +++ b/fpdfsdk/javascript/global.cpp @@ -109,7 +109,8 @@ void JSSpecialPropGet(const char* class_name, pRuntime->Error(JSFormatErrorString(class_name, "GetProperty", L"")); return; } - info.GetReturnValue().Set(value.ToV8Value()); + if (!value.ToV8Value().IsEmpty()) + info.GetReturnValue().Set(value.ToV8Value()); } template @@ -284,16 +285,12 @@ bool JSGlobalAlternate::GetProperty(CJS_Runtime* pRuntime, const wchar_t* propname, CJS_Value* vp) { auto it = m_MapGlobal.find(ByteString::FromUnicode(propname)); - if (it == m_MapGlobal.end()) { - vp->Set(pRuntime->NewNull()); + if (it == m_MapGlobal.end()) return true; - } JSGlobalData* pData = it->second.get(); - if (pData->bDeleted) { - vp->Set(pRuntime->NewNull()); + if (pData->bDeleted) return true; - } switch (pData->nType) { case JS_GlobalDataType::NUMBER: diff --git a/fpdfsdk/javascript/util.cpp b/fpdfsdk/javascript/util.cpp index ecec8d173d..202a1ead7f 100644 --- a/fpdfsdk/javascript/util.cpp +++ b/fpdfsdk/javascript/util.cpp @@ -398,7 +398,7 @@ bool util::scand(CJS_Runtime* pRuntime, if (!std::isnan(dDate)) { vRet = CJS_Value(CJS_Date(pRuntime, dDate).ToV8Value()); } else { - vRet.Set(pRuntime->NewNull()); + vRet.Set(pRuntime->NewUndefined()); } return true; diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp index 4c6398870d..441848205f 100644 --- a/fxjs/fxjs_v8.cpp +++ b/fxjs/fxjs_v8.cpp @@ -664,7 +664,11 @@ v8::Local CFXJS_Engine::NewString(const WideStringView& str) { } v8::Local CFXJS_Engine::NewNull() { - return v8::Local(); + return v8::Null(m_isolate); +} + +v8::Local CFXJS_Engine::NewUndefined() { + return v8::Undefined(m_isolate); } v8::Local CFXJS_Engine::NewDate(double d) { diff --git a/fxjs/fxjs_v8.h b/fxjs/fxjs_v8.h index f5e52411f3..eee85e031d 100644 --- a/fxjs/fxjs_v8.h +++ b/fxjs/fxjs_v8.h @@ -179,6 +179,7 @@ class CFXJS_Engine { v8::Local GetThisObj(); v8::Local NewNull(); + v8::Local NewUndefined(); v8::Local NewArray(); v8::Local NewNumber(int number); v8::Local NewNumber(double number); diff --git a/fxjs/fxjs_v8_embeddertest.cpp b/fxjs/fxjs_v8_embeddertest.cpp index 53fe8f268b..d975264d1d 100644 --- a/fxjs/fxjs_v8_embeddertest.cpp +++ b/fxjs/fxjs_v8_embeddertest.cpp @@ -98,11 +98,25 @@ TEST_F(FXJSV8EmbedderTest, NewNull) { EXPECT_FALSE(engine()->ToBoolean(nullz)); EXPECT_EQ(0, engine()->ToInt32(nullz)); EXPECT_EQ(0.0, engine()->ToDouble(nullz)); - EXPECT_EQ(L"", engine()->ToWideString(nullz)); + EXPECT_EQ(L"null", engine()->ToWideString(nullz)); EXPECT_TRUE(engine()->ToObject(nullz).IsEmpty()); EXPECT_TRUE(engine()->ToArray(nullz).IsEmpty()); } +TEST_F(FXJSV8EmbedderTest, NewUndefined) { + v8::Isolate::Scope isolate_scope(isolate()); + v8::HandleScope handle_scope(isolate()); + v8::Context::Scope context_scope(GetV8Context()); + + auto undef = engine()->NewUndefined(); + EXPECT_FALSE(engine()->ToBoolean(undef)); + EXPECT_EQ(0, engine()->ToInt32(undef)); + EXPECT_TRUE(std::isnan(engine()->ToDouble(undef))); + EXPECT_EQ(L"undefined", engine()->ToWideString(undef)); + EXPECT_TRUE(engine()->ToObject(undef).IsEmpty()); + EXPECT_TRUE(engine()->ToArray(undef).IsEmpty()); +} + TEST_F(FXJSV8EmbedderTest, NewBoolean) { v8::Isolate::Scope isolate_scope(isolate()); v8::HandleScope handle_scope(isolate()); -- cgit v1.2.3