IDBObserver: Lifetime Management: Adding Observer

third_party/WebKit/Source/modules/indexeddb/IDBObserver.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/indexeddb/IDBObserver.h"
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/modules/v8/ToV8ForModules.h"
 #include "bindings/modules/v8/V8BindingForModules.h"
+#include "core/dom/ExceptionCode.h"
+#include "modules/indexeddb/IDBDatabase.h"
 #include "modules/indexeddb/IDBObserverCallback.h"
 #include "modules/indexeddb/IDBObserverInit.h"
+#include "modules/indexeddb/IDBTransaction.h"
+#include "modules/indexeddb/WebIDBObserverImpl.h"
 
 namespace blink {
 
 IDBObserver* IDBObserver::create(IDBObserverCallback& callback, const IDBObserverInit& options)
 {
     return new IDBObserver(callback, options);
 }
 
 IDBObserver::IDBObserver(IDBObserverCallback& callback, const IDBObserverInit& options)
     : m_callback(&callback)
     , m_transaction(options.transaction())
     , m_values(options.values())
     , m_noRecords(options.noRecords())
 {
 }
 
+IDBObserver::~IDBObserver() {}
+
 void IDBObserver::observe(IDBDatabase* database, IDBTransaction* transaction, ExceptionState& exceptionState)
 {
-    // TODO(palakj): Finish implementation.
+    if (transaction->isFinished() || transaction->isFinishing()) {
+        exceptionState.throwDOMException(TransactionInactiveError, IDBDatabase::transactionFinishedErrorMessage);
+        return;
+    }
+    if (!transaction->isActive()) {
+        exceptionState.throwDOMException(TransactionInactiveError, IDBDatabase::transactionInactiveErrorMessage);
+        return;
+    }
+    if (!database->backend()) {
+        exceptionState.throwDOMException(InvalidStateError, IDBDatabase::databaseClosedErrorMessage);
+        return;
+    }
+
+    std::unique_ptr<WebIDBObserverImpl> observer = WebIDBObserverImpl::create(this);
+    WebIDBObserverImpl* observerPtr = observer.get();

[cmumford, 2016/07/06 22:57:56]

Can you remove observerPtr and instead have WebIDBDatabaseImpl::addObserver()
set the observer ID? Technically after addObserver() is called the database now
owns that object, and it's not guaranteed to even still exist, or have the same
address right?

[palakj1, 2016/07/07 00:19:53]

That's true, technically speaking. If we want it to be completely correct, it is
the dispatcher and not the WebIDBDatabaseImpl that owns the observer object. I
can do the adding id part there.
We just wanted to have less overhead in content::indexed_db_dispatcher.cc.

+    int32_t observerId = database->backend()->addObserver(std::move(observer), transaction->id());
+    m_observerIds.insert(observerId);
+    observerPtr->setId(observerId);
+}
+
+void IDBObserver::unobserve(IDBDatabase* database, ExceptionState& exceptionState)
+{
+    if (!database->backend()) {
+        exceptionState.throwDOMException(InvalidStateError, IDBDatabase::databaseClosedErrorMessage);
+        return;
+    }
+    std::set<int32_t>::iterator it = m_observerIds.begin();

[cmumford, 2016/07/06 22:57:56]

Nit: You could make this "auto it = ..." if you want, but is OK as is.

[palakj1, 2016/07/07 00:19:53]

Done.

+    std::vector<int32_t> observerIdsToRemove;
+    while (it != m_observerIds.end()) {
+        if (database->backend()->containsObserverId(*it)) {
+            observerIdsToRemove.push_back(*it);
+            it = m_observerIds.erase(it);
+        } else {
+            it++;
+        }
+    }
+    if (!observerIdsToRemove.empty())
+        database->backend()->removeObservers(observerIdsToRemove);
+}
+
+void IDBObserver::removeObserver(int32_t id)
+{
+    m_observerIds.erase(id);
 }
 
 DEFINE_TRACE(IDBObserver)
 {
     visitor->trace(m_callback);
 }
 
 } // namespace blink