IndexedDB: Various C++11isms and cleanup

Source/modules/indexeddb/IDBTransaction.cpp

 /*
  * Copyright (C) 2010 Google 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 72 matching lines @@
 
 } // namespace
 
 IDBTransaction::IDBTransaction(ScriptState* scriptState, int64_t id, const HashSet<String>& objectStoreNames, WebIDBTransactionMode mode, IDBDatabase* db, IDBOpenDBRequest* openDBRequest, const IDBDatabaseMetadata& previousMetadata)
     : ActiveDOMObject(scriptState->executionContext())
     , m_id(id)
     , m_database(db)
     , m_objectStoreNames(objectStoreNames)
     , m_openDBRequest(openDBRequest)
     , m_mode(mode)
-    , m_state(Active)
-    , m_hasPendingActivity(true)
-    , m_contextStopped(false)
     , m_previousMetadata(previousMetadata)
 {
     if (mode == WebIDBTransactionModeVersionChange) {
         // Not active until the callback.
         m_state = Inactive;
     }
 
     if (m_state == Active)
         V8PerIsolateData::from(scriptState->isolate())->addEndOfScopeTask(DeactivateTransactionTask::create(this));
     m_database->transactionCreated(this);
@@ skipping 99 matching lines @@
     if (m_state == Finishing || m_state == Finished) {
         exceptionState.throwDOMException(InvalidStateError, IDBDatabase::transactionFinishedErrorMessage);
         return;
     }
 
     m_state = Finishing;
 
     if (m_contextStopped)
         return;
 
-    while (!m_requestList.isEmpty()) {
-        IDBRequest* request = *m_requestList.begin();
-        m_requestList.remove(request);
+    for (IDBRequest* request : m_requestList)

[cmumford, 2015/09/03 23:21:03]

I seem to remember that aborting one request can sometimes abort (or maybe
enqueue) another. Are you sure this is safe?

[jsbell, 2015/09/03 23:29:54]

Looking at what IDBRequest::abort() does, it should be safe since there are no
synchronous side effects. It can enqueue events but they are dispatched
asynchronously.

I had similar reservations, thinking about changing destruction timing, but now
that everything is GC'd the destruction all happens later anyway.

         request->abort();
-    }
+    m_requestList.clear();
 
     if (backendDB())
         backendDB()->abort(m_id);
 }
 
 void IDBTransaction::registerRequest(IDBRequest* request)
 {
     ASSERT(request);
     ASSERT(m_state == Active);
     m_requestList.add(request);
@@ skipping 14 matching lines @@
         return;
     }
 
     ASSERT(m_state != Finished);
     if (m_state != Finishing) {
         ASSERT(error);
         setError(error);
 
         // Abort was not triggered by front-end, so outstanding requests must
         // be aborted now.
-        while (!m_requestList.isEmpty()) {
-            IDBRequest* request = *m_requestList.begin();
-            m_requestList.remove(request);
+        for (IDBRequest* request : m_requestList)
             request->abort();
-        }
+        m_requestList.clear();
+
         m_state = Finishing;
     }
 
     if (isVersionChange()) {
-        for (IDBObjectStoreMetadataMap::iterator it = m_objectStoreCleanupMap.begin(); it != m_objectStoreCleanupMap.end(); ++it)
-            it->key->setMetadata(it->value);
+        for (auto& it : m_objectStoreCleanupMap)
+            it.key->setMetadata(it.value);
         m_database->setMetadata(m_previousMetadata);
         m_database->close();
     }
     m_objectStoreCleanupMap.clear();
 
     // Enqueue events before notifying database, as database may close which enqueues more events and order matters.
     enqueueEvent(Event::createBubble(EventTypeNames::abort));
 
     m_database->transactionFinished(this);
 }
@@ skipping 82 matching lines @@
         m_state = Finished;
         return false;
     }
     ASSERT(m_state != Finished);
     ASSERT(m_hasPendingActivity);
     ASSERT(executionContext());
     ASSERT(event->target() == this);
     m_state = Finished;
 
     // Break reference cycles.
-    for (IDBObjectStoreMap::iterator it = m_objectStoreMap.begin(); it != m_objectStoreMap.end(); ++it)
-        it->value->transactionFinished();
+    for (auto& it : m_objectStoreMap)
+        it.value->transactionFinished();
     m_objectStoreMap.clear();
-    for (IDBObjectStoreSet::iterator it = m_deletedObjectStores.begin(); it != m_deletedObjectStores.end(); ++it)
-        (*it)->transactionFinished();
+    for (auto& it : m_deletedObjectStores)
+        it->transactionFinished();
     m_deletedObjectStores.clear();
 
     WillBeHeapVector<RefPtrWillBeMember<EventTarget>> targets;
     targets.append(this);
     targets.append(db());
 
     // FIXME: When we allow custom event dispatching, this will probably need to change.
     ASSERT(event->type() == EventTypeNames::complete || event->type() == EventTypeNames::abort);
     bool returnValue = IDBEventDispatcher::dispatch(event.get(), targets);
     // FIXME: Try to construct a test where |this| outlives openDBRequest and we
@@ skipping 26 matching lines @@
     event->setTarget(this);
     eventQueue->enqueueEvent(event);
 }
 
 WebIDBDatabase* IDBTransaction::backendDB() const
 {
     return m_database->backend();
 }
 
 } // namespace blink