commit 2fb992d05a1d5e88f0414bfcaf5fc071a8c43a82
author Tom Sepez <tsepez@chromium.org> Fri Mar 20 19:45:59 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Fri Mar 20 19:45:59 2020 +0000
tree 3c9cfee27d3f6adb423cabb7fa5f890df48ffed6
parent fc407350ad453a707c69bf42ece1e19ed1683f29

Make CFXJSE_Value use fxv8 helpers for aggregate type access.

Continue thinning CFXJSE_Value with an eye towards its deprecation.

-- Add GetValue() helper to avoid some redundant code.
-- Correct return value from fxv8 by calling FromJust().

Change-Id: I3b010bc97b8a348a0fbd2e6e5ec7ac3d2a5a40bd
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/67596
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

fxjs/fxv8.cpp

diff --git a/fxjs/fxv8.cpp b/fxjs/fxv8.cpp
index 1c7abd8..b655d2d 100644
--- a/fxjs/fxv8.cpp
+++ b/fxjs/fxv8.cpp
@@ -166,7 +166,8 @@
     return false;
 
   v8::Local<v8::String> name = NewStringHelper(pIsolate, bsUTF8PropertyName);
-  return pObj->Set(pIsolate->GetCurrentContext(), name, pPut).IsJust();
+  v8::Maybe<bool> result = pObj->Set(pIsolate->GetCurrentContext(), name, pPut);
+  return result.IsJust() && result.FromJust();
 }
 
 bool ReentrantPutArrayElementHelper(v8::Isolate* pIsolate,
@@ -175,7 +176,10 @@
                                     v8::Local<v8::Value> pValue) {
   if (pArray.IsEmpty())
     return false;
-  return pArray->Set(pIsolate->GetCurrentContext(), index, pValue).IsJust();
+
+  v8::Maybe<bool> result =
+      pArray->Set(pIsolate->GetCurrentContext(), index, pValue);
+  return result.IsJust() && result.FromJust();
 }
 
 v8::Local<v8::Value> ReentrantGetArrayElementHelper(v8::Isolate* pIsolate,

fxjs/xfa/cfxjse_value.cpp

diff --git a/fxjs/xfa/cfxjse_value.cpp b/fxjs/xfa/cfxjse_value.cpp
index 6479b4a..e8705e8 100644
--- a/fxjs/xfa/cfxjse_value.cpp
+++ b/fxjs/xfa/cfxjse_value.cpp
@@ -105,16 +105,12 @@
   CFXJSE_ScopeUtil_IsolateHandleRootContext scope(GetIsolate());
   v8::Local<v8::Array> hArrayObject =
       v8::Array::New(GetIsolate(), values.size());
-  v8::Local<v8::Context> context = GetIsolate()->GetCurrentContext();
   uint32_t count = 0;
   for (auto& v : values) {
     if (v->IsEmpty())
       v->SetUndefined();
-    hArrayObject
-        ->Set(
-            context, count++,
-            v8::Local<v8::Value>::New(GetIsolate(), v.get()->DirectGetValue()))
-        .FromJust();
+    fxv8::ReentrantPutArrayElementHelper(GetIsolate(), hArrayObject, count++,
+                                         v->GetValue());
   }
   m_hValue.Reset(GetIsolate(), hArrayObject);
 }
@@ -127,57 +123,40 @@
 
 bool CFXJSE_Value::SetObjectProperty(ByteStringView szPropName,
                                      CFXJSE_Value* lpPropValue) {
-  ASSERT(lpPropValue);
   if (lpPropValue->IsEmpty())
     return false;
 
   CFXJSE_ScopeUtil_IsolateHandleRootContext scope(GetIsolate());
-  v8::Local<v8::Value> hObject =
-      v8::Local<v8::Value>::New(GetIsolate(), m_hValue);
+  v8::Local<v8::Value> hObject = GetValue();
   if (!hObject->IsObject())
     return false;
 
-  v8::Local<v8::String> hPropName =
-      fxv8::NewStringHelper(GetIsolate(), szPropName);
-  v8::Local<v8::Value> hPropValue =
-      v8::Local<v8::Value>::New(GetIsolate(), lpPropValue->DirectGetValue());
-  v8::Maybe<bool> result = hObject.As<v8::Object>()->Set(
-      GetIsolate()->GetCurrentContext(), hPropName, hPropValue);
-  return result.IsJust() && result.FromJust();
+  return fxv8::ReentrantPutObjectPropertyHelper(
+      GetIsolate(), hObject.As<v8::Object>(), szPropName,
+      lpPropValue->GetValue());
 }
 
 bool CFXJSE_Value::GetObjectProperty(ByteStringView szPropName,
                                      CFXJSE_Value* lpPropValue) {
-  ASSERT(lpPropValue);
   CFXJSE_ScopeUtil_IsolateHandleRootContext scope(GetIsolate());
-  v8::Local<v8::Value> hObject =
-      v8::Local<v8::Value>::New(GetIsolate(), m_hValue);
+  v8::Local<v8::Value> hObject = GetValue();
   if (!hObject->IsObject())
     return false;
 
-  v8::Local<v8::String> hPropName =
-      fxv8::NewStringHelper(GetIsolate(), szPropName);
-  v8::Local<v8::Value> hPropValue =
-      hObject.As<v8::Object>()
-          ->Get(GetIsolate()->GetCurrentContext(), hPropName)
-          .ToLocalChecked();
-  lpPropValue->ForceSetValue(hPropValue);
+  lpPropValue->ForceSetValue(fxv8::ReentrantGetObjectPropertyHelper(
+      GetIsolate(), hObject.As<v8::Object>(), szPropName));
   return true;
 }
 
 bool CFXJSE_Value::GetObjectPropertyByIdx(uint32_t uPropIdx,
                                           CFXJSE_Value* lpPropValue) {
   CFXJSE_ScopeUtil_IsolateHandleRootContext scope(GetIsolate());
-  v8::Local<v8::Value> hObject =
-      v8::Local<v8::Value>::New(GetIsolate(), m_hValue);
-  if (!hObject->IsObject())
+  v8::Local<v8::Value> hObject = GetValue();
+  if (!hObject->IsArray())
     return false;
 
-  v8::Local<v8::Value> hPropValue =
-      hObject.As<v8::Object>()
-          ->Get(GetIsolate()->GetCurrentContext(), uPropIdx)
-          .ToLocalChecked();
-  lpPropValue->ForceSetValue(hPropValue);
+  lpPropValue->ForceSetValue(fxv8::ReentrantGetArrayElementHelper(
+      GetIsolate(), hObject.As<v8::Array>(), uPropIdx));
   return true;
 }
 
@@ -268,6 +247,10 @@
   return true;
 }
 
+v8::Local<v8::Value> CFXJSE_Value::GetValue() const {
+  return v8::Local<v8::Value>::New(GetIsolate(), m_hValue);
+}
+
 bool CFXJSE_Value::IsEmpty() const {
   return m_hValue.IsEmpty();
 }

fxjs/xfa/cfxjse_value.h

diff --git a/fxjs/xfa/cfxjse_value.h b/fxjs/xfa/cfxjse_value.h
index 5503c28..2bf0bf4 100644
--- a/fxjs/xfa/cfxjse_value.h
+++ b/fxjs/xfa/cfxjse_value.h
@@ -66,6 +66,7 @@
                             CFXJSE_Value* lpPropValue);
   bool SetFunctionBind(CFXJSE_Value* lpOldFunction, CFXJSE_Value* lpNewThis);
 
+  v8::Local<v8::Value> GetValue() const;
   const v8::Global<v8::Value>& DirectGetValue() const { return m_hValue; }
   void ForceSetValue(v8::Local<v8::Value> hValue) {
     m_hValue.Reset(GetIsolate(), hValue);