bindings: Add toBoolean() to V8Binding.h

Source/bindings/tests/results/core/V8TestDictionary.cpp

Index: Source/bindings/tests/results/core/V8TestDictionary.cpp
diff --git a/Source/bindings/tests/results/core/V8TestDictionary.cpp b/Source/bindings/tests/results/core/V8TestDictionary.cpp
index b66fc3163d1c8f05051b0fb52f71768ca3287746..6c94a96bcd97898f2239169361ca6f2e2901ab8a 100644
--- a/Source/bindings/tests/results/core/V8TestDictionary.cpp
+++ b/Source/bindings/tests/results/core/V8TestDictionary.cpp
@@ -45,7 +45,9 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
     if (booleanMemberValue.IsEmpty() || booleanMemberValue->IsUndefined()) {
         // Do nothing.
     } else {
-        bool booleanMember = booleanMemberValue->BooleanValue();
+        bool booleanMember = toBoolean(isolate, booleanMemberValue, exceptionState);

[Yuki, 2015/04/10 06:17:29]

This code looks to me a lesser version of Maybe.  The caller can ignore a
possible exception.  People may forget to check if an exception is thrown or
not.

Why don't you use the following style that you used for other part?

  bool booleanMember;
  if (!v8CallToBoolean(isolate, booleanMemberValue, booleanMember,
exceptionState))
      return;

where v8CallToBoolean is annotated with "warn unused return value".

I think that the above style is closer to the mind of V8's Maybe and your
design.

[bashi, 2015/04/10 06:39:54]

It will require substantial changes in the code generator.
- We use v8_value_to_cpp_value() here and v8_value_to_cpp_value() isn't allowed
to return statements (need to return an expression).
- If we want to allow v8_value_to_cpp_value() returns statements, we need to
modify all places where we use v8_value_to_cpp_value(), which requires many
changes.
- We don't have a v8Call() variant which takes exceptionState. If we want to add
it, it would look like:
  bool v8Call(..., ExceptionState& exceptionState) {
    v8::TryCatch block;
    if (!...) {
      exceptionState.rethrowV8Exception(block.Exception());
      return false;
    }
    ...
  }
  and we will have two TryCatch in the call path, which looks redundant. To
avoid having two TryCatch, we will need additional logic.
- We already use this patterns like toDouble().
- We might want to change the current design for handling exception (i.e. using
exceptionState to detect errors), but it will also require a bunch of changes
and would be a 
separate issue.

Given that, I choose this design.

[Yuki, 2015/04/10 07:01:43]

Thanks for the explanation.  Reinvention of exception handling costs us.

+        if (exceptionState.hadException())
+            return;
         impl.setBooleanMember(booleanMember);
     }
 
@@ -57,7 +59,9 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
     if (createValue.IsEmpty() || createValue->IsUndefined()) {
         // Do nothing.
     } else {
-        bool create = createValue->BooleanValue();
+        bool create = toBoolean(isolate, createValue, exceptionState);
+        if (exceptionState.hadException())
+            return;
         impl.setCreateMember(create);
     }
 
@@ -70,7 +74,9 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
         // Do nothing.
     } else {
         UseCounter::countDeprecationIfNotPrivateScript(isolate, callingExecutionContext(isolate), UseCounter::CreateMember);
-        bool deprecatedCreateMember = deprecatedCreateMemberValue->BooleanValue();
+        bool deprecatedCreateMember = toBoolean(isolate, deprecatedCreateMemberValue, exceptionState);
+        if (exceptionState.hadException())
+            return;
         impl.setCreateMember(deprecatedCreateMember);
     }