This originally started as a comment from Yuki in https://codereview.chromium.org/2825033003/. I'm creating the ticket to keep track of all the items mentioned there (and also because it makes more sense to discuss it here instead). From https://codereview.chromium.org/2825033003/#msg11:

----
Hmm, now I think it would be better to keep ToImplArray.  Why is it better to
remove?

TL;DR: What about just renaming ToImplArray to a better name, and continue using
it?

I see that SQLTransaction::executeSql is the only client of ToImplArray, but
potentially there could be more clients in future.  Using ToImplArray would be
safer / more robust than implementing it on each client side.  I'm afraid that
clients wouldn't handle error cases appropriately.

My current understanding is that:

* NativeValueTraits::NativeValue is responsible to convert a v8::Value to a
value of an arbitrary implementation type T.

* ScriptValue::To is just an alias of NativeValueTraits::NativeValue.  I don't
understand why we need this function now.  Not sure, but we could want to remove
this.

* ScriptValue is designed to hold a raw v8::Value.  So, I think it makes sense
to treat a ScriptValue as if it's a v8::Value, i.e. Having a following overload.
  template <typename T>
  T NativeValue(isolate, scriptValue, exceptionState) {
    return NativeValue(isolate, scriptValue.V8Value(), exceptionState)
  }

* SQLTransaction::executeSql needs a kind of "map" higher-order function.
  Vector<ScriptValue> input;
  Vector<SQLValue> output;
  output = map(NativeValueTraits::NativeValue, input);
ToImplArray is a kind of non-general "map" function.

* Probably, it's not a bad idea to provide this kind of "map" function.

Thus, I'm now thinking it would be good to rename ToImplArray to "a map
higher-order function from Vector<v8:Value or ScriptValue> to Vector<T>", and
continue using it.

What do you think?

https://codereview.chromium.org/2825033003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webdatabase/SQLTransaction.cpp (right):

https://codereview.chromium.org/2825033003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webdatabase/SQLTransaction.cpp:324:
script_state->GetIsolate(), script_value, exception_state));
We need to handle a possible exception here.

The original code seems wrong, too, because the original code also ignored a
possible exception.

The code in this CL could be worse because it is completely ignoring a possible
exception.  The original code at least used an empty vector in case of error.
----
 

This originally started as a comment from Yuki in https://codereview.chromium.org/2825033003/. I'm creating the ticket to keep track of all the items mentioned there (and also because it makes more sense to discuss it here instead). From https://codereview.chromium.org/2825033003/#msg11:

----
Hmm, now I think it would be better to keep ToImplArray.  Why is it better to
remove?

TL;DR: What about just renaming ToImplArray to a better name, and continue using
it?

I see that SQLTransaction::executeSql is the only client of ToImplArray, but
potentially there could be more clients in future.  Using ToImplArray would be
safer / more robust than implementing it on each client side.  I'm afraid that
clients wouldn't handle error cases appropriately.

My current understanding is that:

* NativeValueTraits::NativeValue is responsible to convert a v8::Value to a
value of an arbitrary implementation type T.

* ScriptValue::To is just an alias of NativeValueTraits::NativeValue.  I don't
understand why we need this function now.  Not sure, but we could want to remove
this.

* ScriptValue is designed to hold a raw v8::Value.  So, I think it makes sense
to treat a ScriptValue as if it's a v8::Value, i.e. Having a following overload.
  template <typename T>
  T NativeValue(isolate, scriptValue, exceptionState) {
    return NativeValue(isolate, scriptValue.V8Value(), exceptionState)
  }

* SQLTransaction::executeSql needs a kind of "map" higher-order function.
  Vector<ScriptValue> input;
  Vector<SQLValue> output;
  output = map(NativeValueTraits::NativeValue, input);
ToImplArray is a kind of non-general "map" function.

* Probably, it's not a bad idea to provide this kind of "map" function.

Thus, I'm now thinking it would be good to rename ToImplArray to "a map
higher-order function from Vector<v8:Value or ScriptValue> to Vector<T>", and
continue using it.

What do you think?
----