Remove unnecesary usage of RefPtr and PassRefPtr of DatabaseContext.

Source/modules/webdatabase/DatabaseManager.cpp

Index: Source/modules/webdatabase/DatabaseManager.cpp
diff --git a/Source/modules/webdatabase/DatabaseManager.cpp b/Source/modules/webdatabase/DatabaseManager.cpp
index 41f413d1a85237f87645844d7d6f3caf4e402860..181eaf4df6dfed68d47ca2efa5bdb0fa80cd533c 100644
--- a/Source/modules/webdatabase/DatabaseManager.cpp
+++ b/Source/modules/webdatabase/DatabaseManager.cpp
@@ -91,7 +91,7 @@ private:
     OwnPtr<DatabaseCallback> m_creationCallback;
 };
 
-PassRefPtr<DatabaseContext> DatabaseManager::existingDatabaseContextFor(ExecutionContext* context)
+DatabaseContext* DatabaseManager::existingDatabaseContextFor(ExecutionContext* context)
 {
     MutexLocker locker(m_contextMapLock);
 
@@ -99,27 +99,17 @@ PassRefPtr<DatabaseContext> DatabaseManager::existingDatabaseContextFor(Executio
     ASSERT(m_databaseContextInstanceCount >= 0);
     ASSERT(m_databaseContextRegisteredCount <= m_databaseContextInstanceCount);
 
-    RefPtr<DatabaseContext> databaseContext = adoptRef(m_contextMap.get(context));
-    if (databaseContext) {
-        // If we're instantiating a new DatabaseContext, the new instance would
-        // carry a new refCount of 1. The client expects this and will simply
-        // adoptRef the databaseContext without ref'ing it.
-        //     However, instead of instantiating a new instance, we're reusing
-        // an existing one that corresponds to the specified ExecutionContext.
-        // Hence, that new refCount need to be attributed to the reused instance
-        // to ensure that the refCount is accurate when the client adopts the ref.
-        // We do this by ref'ing the reused databaseContext before returning it.
-        databaseContext->ref();
-    }
-    return databaseContext.release();

[tkent, 2014/03/26 04:41:18]

This code block is exactly equivalent to "return
PassRefPtr<DatabaseContext>(m_contextMap.get(context));"

[haraken, 2014/03/26 04:57:24]

I don't fully understand this.

  RefPtr<DatabaseContext> databaseContext = adoptRef(m_contextMap.get(context));
  return databaseContext.release();

will be equivalent to:

  return
PassRefPtr<DatabaseContext>(m_contextMap.get(context));

On the other hand,

  RefPtr<DatabaseContext> databaseContext = adoptRef(m_contextMap.get(context));
  if (databaseContext)
    databaseContext->ref();
  return databaseContext.release();

won't be equivalent to:

  return
PassRefPtr<DatabaseContext>(m_contextMap.get(context));

[tkent, 2014/03/26 05:15:46]

No.  the former is equivalent to just adoptRef(...), which doesn't update the
reference counter.  PassRefPtr<T>(T*) increments the reference counter.

[haraken, 2014/03/26 05:37:50]

sorry, I don't yet get it.

PassRefPtr<Foo> func1() {
  RefPtr<Foo> foo = adoptRef(new Foo()); // refcount becomes 1.
  return foo.release(); // refcount stays 1.
}

void func2() {
  RefPtr<Foo> foo = func1();  // refcount stays 1.
}

PassRefPtr<Foo> func3() {
  Foo* foo = new Foo(); // refcount is 0.
  return PassRefPtr<Foo>(foo); // refcount becomes 1.
}

void func4() {
  RefPtr<Foo> foo = func3();  // refcount stays 1.
}

Given the above:

  RefPtr<Foo> foo = adoptRef(new Foo());
  return foo.release();

is equivalent to:

  return PassRefPtr<Foo>(new Foo());

Am I missing something?

[tkent, 2014/03/26 05:50:20]

Incorrect.
new Foo() creates a Foo with m_refCount==1.
PassRefPtr<Foo>(foo) makes m_refCount 2.
So, m_refCount will be 2.

+    return m_contextMap.get(context);
 }
 
-PassRefPtr<DatabaseContext> DatabaseManager::databaseContextFor(ExecutionContext* context)
+DatabaseContext* DatabaseManager::databaseContextFor(ExecutionContext* context)
 {
-    RefPtr<DatabaseContext> databaseContext = existingDatabaseContextFor(context);
-    if (!databaseContext)
-        databaseContext = DatabaseContext::create(context);
-    return databaseContext.release();
+    if (DatabaseContext* databaseContext = existingDatabaseContextFor(context))
+        return databaseContext;
+    // We don't need to hold a reference returned by DatabaseContext::create
+    // because a DatabaseContext refers to itself internally. See
+    // DataabseContext::create.

[haraken, 2014/03/26 04:57:24]

Typo: DatabaseContext::create

[tkent, 2014/03/26 05:15:46]

Will fix in the next CL.

+    return DatabaseContext::create(context).get();
 }
 
 void DatabaseManager::registerDatabaseContext(DatabaseContext* databaseContext)
@@ -186,11 +176,8 @@ PassRefPtrWillBeRawPtr<DatabaseBackendBase> DatabaseManager::openDatabaseBackend
 {
     ASSERT(error == DatabaseError::None);
 
-    RefPtr<DatabaseContext> databaseContext = databaseContextFor(context);
-    RefPtr<DatabaseContext> backendContext = databaseContext->backend();
-
     RefPtrWillBeRawPtr<DatabaseBackendBase> backend = m_server->openDatabase(
-        backendContext, type, name, expectedVersion,
+        databaseContextFor(context)->backend(), type, name, expectedVersion,
         displayName, estimatedSize, setVersionInNewDatabase, error, errorMessage);
 
     if (!backend) {
@@ -228,8 +215,7 @@ PassRefPtrWillBeRawPtr<Database> DatabaseManager::openDatabase(ExecutionContext*
 
     RefPtrWillBeRawPtr<Database> database = Database::create(context, backend);
 
-    RefPtr<DatabaseContext> databaseContext = databaseContextFor(context);
-    databaseContext->setHasOpenDatabases();
+    databaseContextFor(context)->setHasOpenDatabases();
     DatabaseClient::from(context)->didOpenDatabase(database, context->securityOrigin()->host(), name, expectedVersion);
 
     if (backend->isNew() && creationCallback.get()) {
@@ -278,7 +264,7 @@ void DatabaseManager::closeDatabasesImmediately(const String& originIdentifier,
 
 void DatabaseManager::interruptAllDatabasesForContext(DatabaseContext* databaseContext)
 {
-    m_server->interruptAllDatabasesForContext(databaseContext->backend().get());
+    m_server->interruptAllDatabasesForContext(databaseContext->backend());
 }
 
 void DatabaseManager::logErrorMessage(ExecutionContext* context, const String& message)