Remove unnecesary usage of RefPtr and PassRefPtr of DatabaseContext.

Source/modules/webdatabase/DatabaseManager.cpp

 /*
  * Copyright (C) 2012 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
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 73 matching lines @@
     DatabaseCreationCallbackTask(PassRefPtrWillBeRawPtr<Database> database, PassOwnPtr<DatabaseCallback> callback)
         : m_database(database)
         , m_creationCallback(callback)
     {
     }
 
     RefPtrWillBePersistent<Database> m_database;
     OwnPtr<DatabaseCallback> m_creationCallback;
 };
 
-PassRefPtr<DatabaseContext> DatabaseManager::existingDatabaseContextFor(ExecutionContext* context)
+DatabaseContext* DatabaseManager::existingDatabaseContextFor(ExecutionContext* context)
 {
     MutexLocker locker(m_contextMapLock);
 
     ASSERT(m_databaseContextRegisteredCount >= 0);
     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)
 {
     MutexLocker locker(m_contextMapLock);
     ExecutionContext* context = databaseContext->executionContext();
     m_contextMap.set(context, databaseContext);
 #if !ASSERT_DISABLED
     m_databaseContextRegisteredCount++;
 #endif
@@ skipping 46 matching lines @@
     WTF_LOG(StorageAPI, "Database %s for origin %s not allowed to be established", name.ascii().data(),
         context->securityOrigin()->toString().ascii().data());
 }
 
 PassRefPtrWillBeRawPtr<DatabaseBackendBase> DatabaseManager::openDatabaseBackend(ExecutionContext* context,
     DatabaseType type, const String& name, const String& expectedVersion, const String& displayName,
     unsigned long estimatedSize, bool setVersionInNewDatabase, DatabaseError& error, String& errorMessage)
 {
     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) {
         ASSERT(error != DatabaseError::None);
 
         switch (error) {
         case DatabaseError::GenericSecurityError:
             logOpenDatabaseError(context, name);
             return nullptr;
 
@@ skipping 17 matching lines @@
     ASSERT(error == DatabaseError::None);
 
     bool setVersionInNewDatabase = !creationCallback;
     RefPtrWillBeRawPtr<DatabaseBackendBase> backend = openDatabaseBackend(context, DatabaseType::Async, name,
         expectedVersion, displayName, estimatedSize, setVersionInNewDatabase, error, errorMessage);
     if (!backend)
         return nullptr;
 
     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()) {
         WTF_LOG(StorageAPI, "Scheduling DatabaseCreationCallbackTask for database %p\n", database.get());
         database->executionContext()->postTask(DatabaseCreationCallbackTask::create(database, creationCallback));
     }
 
     ASSERT(database);
     return database.release();
 }
@@ skipping 28 matching lines @@
     return m_server->fullPathForDatabase(origin, name, createIfDoesNotExist);
 }
 
 void DatabaseManager::closeDatabasesImmediately(const String& originIdentifier, const String& name)
 {
     m_server->closeDatabasesImmediately(originIdentifier, name);
 }
 
 void DatabaseManager::interruptAllDatabasesForContext(DatabaseContext* databaseContext)
 {
-    m_server->interruptAllDatabasesForContext(databaseContext->backend().get());
+    m_server->interruptAllDatabasesForContext(databaseContext->backend());
 }
 
 void DatabaseManager::logErrorMessage(ExecutionContext* context, const String& message)
 {
     context->addConsoleMessage(StorageMessageSource, ErrorMessageLevel, message);
 }
 
 } // namespace WebCore