Commit 9293bf20 authored by tkent@chromium.org's avatar tkent@chromium.org

Revert of DatabaseBackend objects should be destructed in the originator...

Revert of DatabaseBackend objects should be destructed in the originator thread (https://codereview.chromium.org/183883005/)

Reason for revert:
Unnecessary change


Original issue's description:
> DatabaseBackend objects should be destructed in the originator thread.
> 
> Before this CL, it was possible that DatabaseThread held the last
> references of DatabaseBackend objects and it was released in the
> database thread.
> We must destruct objects in the originator thread in Oilpan.
> 
> Implementation:
> DatabaseThread::recordDatabaseClosed doesn't remove a RefPtr<
> DatabaseBackend>, and we release RefPtr<DatabaseBackend> objects
> in ~DatabaseThread, which is executed in the main/worker thread.
> Note that there are no ways to close a database explicitly, and
> DatabaseBackend objects are never destructed before their
> DatabaseContext is stopped. DatabaseThread::recordDatabaseClosed
> is actually unnecessary.
> 
> DatabaseThread doesn't know DatabaseBackend open status any longer.
> We remove DatabaseThread::isDatabaseOpen, and its existing
> callsites use DatabaseBackend::open() instead.
> 
> 
> BUG=347902
> TEST=none; no visible behavior changes hopefully.
> R=michaeln@chromium.org
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168362

TBR=michaeln@chromium.org,oilpan-reviews@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=347902

Review URL: https://codereview.chromium.org/178103007

git-svn-id: svn://svn.chromium.org/blink/trunk@168431 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 3513a2fb
...@@ -82,7 +82,7 @@ void DatabaseTask::run() ...@@ -82,7 +82,7 @@ void DatabaseTask::run()
ASSERT(!m_complete); ASSERT(!m_complete);
#endif #endif
if (!m_synchronizer && !m_database->opened()) { if (!m_synchronizer && !m_database->databaseContext()->databaseThread()->isDatabaseOpen(m_database.get())) {
taskCancelled(); taskCancelled();
#if !LOG_DISABLED #if !LOG_DISABLED
m_complete = true; m_complete = true;
......
...@@ -48,8 +48,6 @@ DatabaseThread::DatabaseThread() ...@@ -48,8 +48,6 @@ DatabaseThread::DatabaseThread()
DatabaseThread::~DatabaseThread() DatabaseThread::~DatabaseThread()
{ {
ASSERT(m_thread);
ASSERT(!isDatabaseThread());
bool terminationRequested; bool terminationRequested;
{ {
MutexLocker lock(m_terminationRequestedMutex); MutexLocker lock(m_terminationRequestedMutex);
...@@ -98,8 +96,11 @@ void DatabaseThread::cleanupDatabaseThread() ...@@ -98,8 +96,11 @@ void DatabaseThread::cleanupDatabaseThread()
// Close the databases that we ran transactions on. This ensures that if any transactions are still open, they are rolled back and we don't leave the database in an // Close the databases that we ran transactions on. This ensures that if any transactions are still open, they are rolled back and we don't leave the database in an
// inconsistent or locked state. // inconsistent or locked state.
if (m_openDatabaseSet.size() > 0) { if (m_openDatabaseSet.size() > 0) {
DatabaseSet::iterator end = m_openDatabaseSet.end(); // As the call to close will modify the original set, we must take a copy to iterate over.
for (DatabaseSet::iterator it = m_openDatabaseSet.begin(); it != end; ++it) DatabaseSet openSetCopy;
openSetCopy.swap(m_openDatabaseSet);
DatabaseSet::iterator end = openSetCopy.end();
for (DatabaseSet::iterator it = openSetCopy.begin(); it != end; ++it)
(*it).get()->close(); (*it).get()->close();
} }
...@@ -121,9 +122,17 @@ void DatabaseThread::recordDatabaseClosed(DatabaseBackend* database) ...@@ -121,9 +122,17 @@ void DatabaseThread::recordDatabaseClosed(DatabaseBackend* database)
MutexLocker lock(m_terminationRequestedMutex); MutexLocker lock(m_terminationRequestedMutex);
#endif #endif
ASSERT(isDatabaseThread()); ASSERT(isDatabaseThread());
ASSERT_UNUSED(database, database); ASSERT(database);
ASSERT(m_terminationRequested || m_openDatabaseSet.contains(database)); ASSERT(m_terminationRequested || m_openDatabaseSet.contains(database));
// We'll clear m_openDatabaseSet in the destructor. m_openDatabaseSet.remove(database);
}
bool DatabaseThread::isDatabaseOpen(DatabaseBackend* database)
{
ASSERT(isDatabaseThread());
ASSERT(database);
MutexLocker lock(m_terminationRequestedMutex);
return !m_terminationRequested && m_openDatabaseSet.contains(database);
} }
void DatabaseThread::scheduleTask(PassOwnPtr<DatabaseTask> task) void DatabaseThread::scheduleTask(PassOwnPtr<DatabaseTask> task)
......
...@@ -61,6 +61,7 @@ public: ...@@ -61,6 +61,7 @@ public:
void recordDatabaseOpen(DatabaseBackend*); void recordDatabaseOpen(DatabaseBackend*);
void recordDatabaseClosed(DatabaseBackend*); void recordDatabaseClosed(DatabaseBackend*);
bool isDatabaseOpen(DatabaseBackend*);
bool isDatabaseThread() { return m_thread && m_thread->isCurrentThread(); } bool isDatabaseThread() { return m_thread && m_thread->isCurrentThread(); }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment