Oilpan: Prepare to move SQLResultSet and SQLResultSetRowList to oilpan heap.

Oilpan: Prepare to move SQLResultSet and SQLResultSetRowList to oilpan heap.

Without this CL, a SQLResultSet object in SQLStatementBackend can be constructed
in a database thread, moved to main/worker thread, and referred by JavaScript
object in the main/worker thread.

We can't move object ownership over threads in Oilpan. So, SQLStatementBackend
should construct a SQLResultSet object in the SQLStatementBackend constructor,
which runs in main/worker thread. SQLResultSet should have a validity flag to
represent nullness in the code without this CL.

Note:
* Change the return type of AbstractSQLStatementBackend::sqlResultSet from
  PassRefPtr<SQLResultSet> to SQLResultSet* because the function doesn't release
  the ownership of the SQLResultSet object.

* Change the order of data members of SQLResultSet to pack better.

BUG=347902
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169929

[tkent, 2014-03-25 07:45:08]

Please review this

[haraken, 2014-03-25 07:55:30]

LGTM

> a SQLResultSet object in SQLStatementBackend can be constructed
> in a database thread, moved to main/worker thread, and referred by JavaScript
> object in the main/worker thread.

I'm just curious but at what point were you asserted? I just want to understand
how this kind of threading error is detected.

https://codereview.chromium.org/207453006/diff/1/Source/modules/webdatabase/S...
File Source/modules/webdatabase/SQLStatementBackend.cpp (right):

https://codereview.chromium.org/207453006/diff/1/Source/modules/webdatabase/S...
Source/modules/webdatabase/SQLStatementBackend.cpp:210: m_resultSet->setValid();

In my understanding, setValid() should be always coupled with setRowsAffected().
If so, shall we set m_valid to true in setRowsAffected()? Then setValid() won't
be necessary.

[tkent, 2014-03-25 08:34:06]

> > a SQLResultSet object in SQLStatementBackend can be constructed
> > in a database thread, moved to main/worker thread, and referred by
JavaScript
> > object in the main/worker thread.
> 
> I'm just curious but at what point were you asserted? I just want to
understand
> how this kind of threading error is detected.

In this case, the following assertion in Persistent(T*) failed when A DOM
wrapper for a SQLResultSet was created

    Persistent(T* raw) : m_raw(raw)
    {
        COMPILE_ASSERT_IS_GARBAGE_COLLECTED(T,
NonGarbageCollectedObjectInPersistent);
#ifndef NDEBUG
        // For global persistent handles we cannot check that the
        // pointer is in the heap because that would involve
        // inspecting the heap of running threads.
        bool isGlobalPersistent = WTF::IsSubclass<RootsAccessor,
GlobalPersistents>::value;
        ASSERT(!raw || isGlobalPersistent ||
ThreadStateFor<ThreadingTrait<T>::Affinity>::state()->contains(raw));
#endif
    }

because it tried to set a pointer allocated in a database thread to a Persistent
in a context thread.

[tkent, 2014-03-25 08:40:05]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2014-03-25 08:40:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tkent@chromium.org/207453006/10001

[tkent, 2014-03-25 08:40:32]

https://codereview.chromium.org/207453006/diff/1/Source/modules/webdatabase/S...
File Source/modules/webdatabase/SQLStatementBackend.cpp (right):

https://codereview.chromium.org/207453006/diff/1/Source/modules/webdatabase/S...
Source/modules/webdatabase/SQLStatementBackend.cpp:210: m_resultSet->setValid();
On 2014/03/25 07:55:30, haraken wrote:
> 
> In my understanding, setValid() should be always coupled with
setRowsAffected().
> If so, shall we set m_valid to true in setRowsAffected()? Then setValid()
won't
> be necessary.

Right.  Done.

[commit-bot: I haz the power, 2014-03-25 09:41:34]

Change committed as 169929