Introduce safer CJS_Return::Success() and Failure().

Avoid the possibility of ever re-introducing the issue noticed
last week.

Remove some redundant JSGetStringFromID() calls.

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

1 files changed, 27 insertions, 4 deletions

diff --git a/fxjs/cjs_return.h b/fxjs/cjs_return.h
index b165f3f11c..99a2af60a6 100644
--- a/fxjs/cjs_return.h
+++ b/fxjs/cjs_return.h
@@ -13,10 +13,28 @@
 
 class CJS_Return {
  public:
-  CJS_Return();                               // Successful but empty return.
-  explicit CJS_Return(v8::Local<v8::Value>);  // Successful return with value.
-  explicit CJS_Return(const WideString&);     // Error with custom message.
-  explicit CJS_Return(JSMessage id);          // Error with stock message.
+  // Wrap constructors with static methods so we can apply WARN_UNUSED_RESULT,
+  // otherwise we can't catch places where someone mistakenly writes:
+  //
+  //     if (error)
+  //       CJS_Return(JS_ERROR_CODE);
+  //
+  // instead of
+  //
+  //     if (error)
+  //       return CJS_Return(JS_ERROR_CODE);
+  //
+  static CJS_Return Success() WARN_UNUSED_RESULT { return CJS_Return(); }
+  static CJS_Return Success(v8::Local<v8::Value> value) WARN_UNUSED_RESULT {
+    return CJS_Return(value);
+  }
+  static CJS_Return Failure(const WideString& str) WARN_UNUSED_RESULT {
+    return CJS_Return(str);
+  }
+  static CJS_Return Failure(JSMessage id) WARN_UNUSED_RESULT {
+    return CJS_Return(id);
+  }
+
   CJS_Return(const CJS_Return&);
   ~CJS_Return();
 
@@ -27,6 +45,11 @@ class CJS_Return {
   v8::Local<v8::Value> Return() const { return return_; }
 
  private:
+  CJS_Return();                               // Successful but empty return.
+  explicit CJS_Return(v8::Local<v8::Value>);  // Successful return with value.
+  explicit CJS_Return(const WideString&);     // Error with custom message.
+  explicit CJS_Return(JSMessage id);          // Error with stock message.
+
   Optional<WideString> error_;
   v8::Local<v8::Value> return_;
 };