Return error information from pdfium to JS.

This implements the previously unimplemented JS_Error() function.

Along the way:
- fix some IWYU when the include order in global.cpp was perturbed.
- remove some uses of JS_ErrorString, to increase transparency.
- use vp.IsSetting() in place of !vp.IsGetting() for clarity.
- specify an error string on several error return paths.
- add an error string for writing readonly properties.
- rename an error string constant to reflect the actual message.
- replace calls to variadic Format() with a function doing string appends.
- remove unused JS_GetClassName()

R=thestig@chromium.org

Review URL: https://codereview.chromium.org/963193003

5 files changed, 65 insertions, 46 deletions

diff --git a/fpdfsdk/src/javascript/Document.cpp b/fpdfsdk/src/javascript/Document.cpp
index c9f17ba75c..404e1df146 100644
--- a/fpdfsdk/src/javascript/Document.cpp
+++ b/fpdfsdk/src/javascript/Document.cpp
@@ -16,7 +16,7 @@
 #include "../../include/javascript/app.h"
 #include "../../include/javascript/Field.h"
 #include "../../include/javascript/Icon.h"
-#include "../../include/javascript/Field.h"
+#include "../../include/javascript/resource.h"
 
 #include "../../../third_party/base/numerics/safe_math.h"
 
@@ -192,18 +192,14 @@ Document::~Document()
 //the total number of fileds in document.
 FX_BOOL Document::numFields(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString& sError)
 {
-	if (!vp.IsGetting()) return FALSE;
-
-	ASSERT(m_pDocument != NULL);
-
+	if (vp.IsSetting()) {
+		CJS_Context* pContext = static_cast<CJS_Context*>(cc);
+		sError = JSGetStringFromID(pContext, IDS_STRING_JSREADONLY);
+		return FALSE;
+	}
 	CPDFSDK_InterForm *pInterForm = m_pDocument->GetInterForm();
-	ASSERT(pInterForm != NULL);
-
 	CPDF_InterForm *pPDFForm = pInterForm->GetInterForm();
-	ASSERT(pPDFForm != NULL);
-
 	vp << (int)pPDFForm->CountFields();
-
 	return TRUE;
 }
 
@@ -878,7 +874,7 @@ FX_BOOL Document::info(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString& sEr
 	CFX_WideString cwTrapped		= pDictionary->GetUnicodeText("Trapped");
 
 	v8::Isolate* isolate = GetIsolate(cc);
-	if (!vp.IsSetting())
+	if (vp.IsGetting())
 	{
 		CJS_Context* pContext = (CJS_Context *)cc;
 		CJS_Runtime* pRuntime = pContext->GetJSRuntime();
@@ -1135,16 +1131,13 @@ FX_BOOL Document::title(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString& sE
 
 FX_BOOL Document::numPages(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString& sError)
 {
-	if (vp.IsGetting())
-	{
-		ASSERT(m_pDocument != NULL);
-		vp << m_pDocument->GetPageCount();
-		return TRUE;
-	}
-	else
-	{
+	if (vp.IsSetting()) {
+		CJS_Context* pContext = static_cast<CJS_Context*>(cc);
+		sError = JSGetStringFromID(pContext, IDS_STRING_JSREADONLY);
 		return FALSE;
 	}
+	vp << m_pDocument->GetPageCount();
+	return TRUE;
 }
 
 FX_BOOL Document::external(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString& sError)
@@ -1156,8 +1149,11 @@ FX_BOOL Document::external(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString&
 
 FX_BOOL Document::filesize(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString& sError)
 {
-	if (!vp.IsGetting())return FALSE;
-
+	if (vp.IsSetting()) {
+		CJS_Context* pContext = static_cast<CJS_Context*>(cc);
+		sError = JSGetStringFromID(pContext, IDS_STRING_JSREADONLY);
+		return FALSE;
+	}
 	vp << 0;
 	return TRUE;
 }
@@ -1213,11 +1209,12 @@ FX_BOOL Document::calculate(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString
 
 FX_BOOL Document::documentFileName(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString& sError)
 {
-	if (!vp.IsGetting())
+	if (vp.IsSetting()) {
+		CJS_Context* pContext = static_cast<CJS_Context*>(cc);
+		sError = JSGetStringFromID(pContext, IDS_STRING_JSREADONLY);
 		return FALSE;
-
+	}
 	CFX_WideString wsFilePath = m_pDocument->GetPath();
-
 	FX_INT32 i = wsFilePath.GetLength() - 1;
 	for ( ; i >= 0; i-- )
 	{
@@ -1282,10 +1279,12 @@ CFX_WideString Document::CutString(CFX_WideString cbFrom)
 
 FX_BOOL Document::path(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString& sError)
 {
-	if (!vp.IsGetting()) return FALSE;
-
+	if (vp.IsSetting()) {
+		CJS_Context* pContext = static_cast<CJS_Context*>(cc);
+		sError = JSGetStringFromID(pContext, IDS_STRING_JSREADONLY);
+		return FALSE;
+	}
 	vp << app::SysPathToPDFPath(m_pDocument->GetPath());
-
 	return TRUE;
 }
 
@@ -1491,8 +1490,11 @@ FX_BOOL Document::addIcon(IFXJS_Context* cc, const CJS_Parameters& params, CJS_V
 
 FX_BOOL Document::icons(IFXJS_Context* cc, CJS_PropValue& vp, JS_ErrorString& sError)
 {
-	if (vp.IsSetting())
+	if (vp.IsSetting()) {
+		CJS_Context* pContext = static_cast<CJS_Context*>(cc);
+		sError = JSGetStringFromID(pContext, IDS_STRING_JSREADONLY);
 		return FALSE;
+	}
 
 	if (!m_pIconTree)
 	{
@@ -1619,7 +1621,8 @@ FX_BOOL Document::getPageNthWord(IFXJS_Context* cc, const CJS_Parameters& params
 
 	if (nPageNo < 0 || nPageNo >= pDocument->GetPageCount())
 	{
-		//sError = JSGetStringFromID(IDS_STRING_JSPARAMERROR);
+		CJS_Context* pContext = static_cast<CJS_Context*>(cc);
+		sError = JSGetStringFromID(pContext, IDS_STRING_JSPARAMERROR);
 		return FALSE;
 	}
 
@@ -1688,7 +1691,8 @@ FX_BOOL Document::getPageNumWords(IFXJS_Context* cc, const CJS_Parameters& param
 
 	if (nPageNo < 0 || nPageNo >= pDocument->GetPageCount())
 	{
-		//sError = JSGetStringFromID(IDS_STRING_JSPARAMERROR);
+		CJS_Context* pContext = static_cast<CJS_Context*>(cc);
+		sError = JSGetStringFromID(pContext, IDS_STRING_JSPARAMERROR);
 		return FALSE;
 	}
 
diff --git a/fpdfsdk/src/javascript/console.cpp b/fpdfsdk/src/javascript/console.cpp
index d79ac3405d..e8a2c4c4a0 100644
--- a/fpdfsdk/src/javascript/console.cpp
+++ b/fpdfsdk/src/javascript/console.cpp
@@ -10,9 +10,7 @@
 #include "../../include/javascript/JS_Object.h"
 #include "../../include/javascript/JS_Value.h"
 #include "../../include/javascript/console.h"
-//#include "../../include/javascript/JS_Module.h"
 #include "../../include/javascript/JS_EventHandler.h"
-//#include "../../include/javascript/JS_ResMgr.h"
 #include "../../include/javascript/JS_Context.h"
 
 /* ------------------------ console ------------------------ */
diff --git a/fpdfsdk/src/javascript/global.cpp b/fpdfsdk/src/javascript/global.cpp
index 587ba85219..cc18b2906c 100644
--- a/fpdfsdk/src/javascript/global.cpp
+++ b/fpdfsdk/src/javascript/global.cpp
@@ -4,15 +4,16 @@
  
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
-#include "../../include/javascript/JavaScript.h"
 #include "../../include/javascript/IJavaScript.h"
+#include "../../include/javascript/JS_Context.h"
 #include "../../include/javascript/JS_Define.h"
+#include "../../include/javascript/JS_EventHandler.h"
+#include "../../include/javascript/JS_GlobalData.h"
 #include "../../include/javascript/JS_Object.h"
 #include "../../include/javascript/JS_Value.h"
-#include "../../include/javascript/JS_GlobalData.h"
+#include "../../include/javascript/JavaScript.h"
 #include "../../include/javascript/global.h"
-#include "../../include/javascript/JS_EventHandler.h"
-#include "../../include/javascript/JS_Context.h"
+#include "../../include/javascript/resource.h"
 
 /* ---------------------------- global ---------------------------- */
 
@@ -270,9 +271,10 @@ FX_BOOL global_alternate::DoProperty(IFXJS_Context* cc, FX_LPCWSTR propname, CJS
 
 FX_BOOL global_alternate::setPersistent(IFXJS_Context* cc, const CJS_Parameters& params, CJS_Value& vRet, JS_ErrorString& sError)
 {
+	CJS_Context* pContext = static_cast<CJS_Context*>(cc);
 	if (params.size() != 2)
 	{
-		//sError = JSGetStringFromID(IDS_STRING_JSPARAMERROR);	
+		sError = JSGetStringFromID(pContext, IDS_STRING_JSPARAMERROR);
 		return FALSE;
 	}
 
@@ -288,7 +290,7 @@ FX_BOOL global_alternate::setPersistent(IFXJS_Context* cc, const CJS_Parameters&
 		}
 	}
 
-	//sError = JSGetStringFromID(IDS_JSPARAM_INCORRECT);	
+	sError = JSGetStringFromID(pContext, IDS_STRING_JSNOGLOBAL);
 	return FALSE;
 }
 
diff --git a/fpdfsdk/src/javascript/resource.cpp b/fpdfsdk/src/javascript/resource.cpp
index 1c453dcda7..be24ccd66c 100644
--- a/fpdfsdk/src/javascript/resource.cpp
+++ b/fpdfsdk/src/javascript/resource.cpp
@@ -39,9 +39,24 @@ CFX_WideString JSGetStringFromID(CJS_Context* pContext, FX_UINT id)
         return L"The second parameter can't be converted to a Date.";
     case IDS_STRING_JSPRINT2:
         return L"The second parameter is an invalid Date!";
-    case IDS_JSPARAM_INCORRECT:
+    case IDS_STRING_JSNOGLOBAL:
         return L"Global value not found.";
+    case IDS_STRING_JSREADONLY:
+        return L"Cannot assign to readonly property.";
     default:
         return L"";
     }
 }
+
+CFX_WideString JSFormatErrorString(const char* class_name,
+                                   const char* property_name,
+                                   const CFX_WideString& details) {
+    CFX_WideString result = CFX_WideString::FromLocal(class_name);
+    if (property_name) {
+        result += L".";
+        result += CFX_WideString::FromLocal(property_name);
+    }
+    result += L": ";
+    result += details;
+    return result;
+}
diff --git a/fpdfsdk/src/jsapi/fxjs_v8.cpp b/fpdfsdk/src/jsapi/fxjs_v8.cpp
index a3f7b6ddd3..edade5fb94 100644
--- a/fpdfsdk/src/jsapi/fxjs_v8.cpp
+++ b/fpdfsdk/src/jsapi/fxjs_v8.cpp
@@ -457,9 +457,14 @@ int JS_GetObjDefnID(IJS_Runtime * pJSRuntime, const wchar_t* pObjName)
 	return -1;
 }
 
-void JS_Error(v8::Value * pError,const wchar_t * main,const wchar_t * sub)
+void JS_Error(v8::Isolate* isolate, const CFX_WideString& message)
 {
-
+    // Conversion from pdfium's wchar_t wide-strings to v8's uint16_t
+    // wide-strings isn't handled by v8, so use UTF8 as a common
+    // intermediate format.
+    CFX_ByteString utf8_message = message.UTF8Encode();
+    isolate->ThrowException(v8::String::NewFromUtf8(isolate,
+                                                    utf8_message.c_str()));
 }
 
 unsigned JS_CalcHash(const wchar_t* main, unsigned nLen)
@@ -491,11 +496,6 @@ const wchar_t*	JS_GetTypeof(v8::Handle<v8::Value> pObj)
 	return NULL;
 
 }
-const wchar_t*	JS_GetClassname(v8::Handle<v8::Object> pObj)
-{
-	return NULL;
-}
-
 void JS_SetPrivate(v8::Handle<v8::Object> pObj, void* p)
 {
 	JS_SetPrivate(NULL, pObj, p);