[bindings] Remove custom binding usage in SQLResultSetRowList.

Source/modules/webdatabase/SQLResultSetRowList.cpp

 /*
  * Copyright (C) 2007 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 11 matching lines @@
  * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
  * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
  * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "modules/webdatabase/SQLResultSetRowList.h"
 
+#include "bindings/core/v8/ExceptionMessages.h"
+#include "bindings/core/v8/ToV8.h"
+#include "bindings/core/v8/V8Binding.h"

[vivekg, 2015/03/04 13:40:44]

The V8Binding.h inclusion is not required. 

I wanted to know if I am heading in the right direction so I will remove it in
the next iteration. 

+#include "bindings/modules/v8/ToV8.h"
+#include "core/dom/ExceptionCode.h"
+
 namespace blink {
 
 unsigned SQLResultSetRowList::length() const
 {
     if (m_result.size() == 0)
         return 0;
 
     ASSERT(m_result.size() % m_columns.size() == 0);
 
     return m_result.size() / m_columns.size();
 }
 
+ScriptValue SQLResultSetRowList::item(unsigned index, ExceptionState& exceptionState)

[Yuki, 2015/03/04 14:11:04]

This function should return Vector<pair<String, SQLValue>>.

Do not call any toV8 functions here.  Just create
  Vector<pair<String, SQLValue>> results;
here and fill it, and then return it.

The rest of part should be taken care by the binding code generator.

The current implementation of the binding code generator generates the following
code:
  v8SetReturnValue(info, impl->item(index).v8Value());
This is wrong.  This must be:
  v8SetReturnValue(info, toV8(impl->item(index)));

Then, impl->item(index) returns Vector<pair<String, SQLValue>> and
toV8(Vector<pair<String, SQLValue>>, ...) is called.  Within this toV8, your
another toV8(SQLValue, ...) is called.

[Jens Widell, 2015/03/04 14:18:26]

I disagree. The code generator determines what the C++ implementation should
return based on the return type in the IDL. This function returns "any", so the
code generator will determine that it should return ScriptValue, and so this
function must return something that's compatible with being stored in a local
variable of type ScriptValue.

Relying on toV8() overloading would be convenient in this type of case, but it
would also mean that there would be no typing between what the C++
implementation returns and the declared return type in the IDL.

BTW, the generated code here, because of [RaisesException], is actually

  ScriptValue result = impl->item(index, exceptionState);
  if (exceptionState.hadException()) {
    exceptionState.throwIfNeeded();
    return;
  }
  v8SetReturnValue(info, result.v8Value());

[Jens Widell, 2015/03/04 14:23:41]

That said, we should still perhaps not use toV8() here. We could have a utility
function implement in bindings/ with the signature

  ScriptValue f(const Vector<pair<String, SQLValue>>&)

that this code uses to constructor the return value.

[Yuki, 2015/03/04 14:30:45]

Agreed.  Otherwise, we face to another issue how to get creationContext and
isolate.  v8::Isolate::GetCurrent() is not recommended today, IIUC.

I'm not quite sure if a) ScriptValue was designed to simply hold v8::Value, or
b) it was designed to represent "any" type in IDL.  In the former case, we may
need a new class to represent "any" type.

+{
+    if (index >= length()) {
+        exceptionState.throwDOMException(IndexSizeError, ExceptionMessages::indexExceedsMaximumBound<unsigned>("index", index, length()));
+        return ScriptValue();
+    }
+    unsigned numColumns = m_columns.size();
+    unsigned valuesIndex = index * numColumns;
+
+    Vector<std::pair<String, v8::Handle<v8::Value>>> dataArray;
+    v8::Isolate* isolate = v8::Isolate::GetCurrent();
+
+    for (unsigned i = 0; i < numColumns; ++i)
+        dataArray.append(std::make_pair(m_columns[i], toV8(m_result[valuesIndex + i], isolate)));
+
+    ScriptState* scriptState = ScriptState::current(isolate);
+    ASSERT(scriptState);
+    return ScriptValue(scriptState, toV8(dataArray, scriptState->context()->Global(), isolate));
 }
+
+}