[IndexedDB] Propogating changes to observers : Renderer

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

Index: third_party/WebKit/Source/modules/indexeddb/IDBObserverChanges.cpp
diff --git a/third_party/WebKit/Source/modules/indexeddb/IDBObserverChanges.cpp b/third_party/WebKit/Source/modules/indexeddb/IDBObserverChanges.cpp
index abb4042ff9422d42bb4a623c04727b979e96e9ad..54a2c04312f0556b23ec9af6792770b20f6ffff7 100644
--- a/third_party/WebKit/Source/modules/indexeddb/IDBObserverChanges.cpp
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBObserverChanges.cpp
@@ -6,27 +6,43 @@
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ScriptState.h"
+#include "bindings/core/v8/V8Binding.h"
 #include "bindings/modules/v8/ToV8ForModules.h"
 #include "bindings/modules/v8/V8BindingForModules.h"
 #include "modules/indexeddb/IDBAny.h"
+#include "modules/indexeddb/IDBObservation.h"
+#include "public/platform/modules/indexeddb/WebIDBObservation.h"
 
 namespace blink {
 
 ScriptValue IDBObserverChanges::records(ScriptState* scriptState)
 {
-    return ScriptValue::from(scriptState, m_records);
+    v8::Local<v8::Context> context(scriptState->context());
+    v8::Isolate* isolate(scriptState->isolate());
+    v8::Local<v8::Map> map = v8::Map::New(isolate);
+    for (const auto& it : m_records) {
+        v8::Local<v8::String> key = v8String(isolate, m_database->getObjectStoreName(it.key));
+        v8::Local<v8::Value> value = toV8(it.value, context->Global(), isolate);
+        v8CallOrCrash(map->Set(context, key, value));

[haraken, 2016/07/20 16:22:12]

Use CreateDataProperty instead of Set. If the setter of Map is overridden,
map->Set() may execute the overridden setter and cause unexpected side-effects.

[palakj1, 2016/07/20 18:22:11]

Is using CreateDataProperty on map identical to Set.   
Doing something like 
map->CreateDataProperty(context, key, value) in IDBObserverChanges.cpp

and in javascript :
function callback(changes)
changes.records.get(key) 

returns NaN with CreateDataProperty while it works fine with Set.

Could you elaborate the usage

[jsbell, 2016/07/20 18:41:48]

CreateDataProperty is definitely not correct here; that's for e.g. x = {};
x['k'] = v. In this case, map is a Map and and desire is to populate it as if
calling m = new Map; m.set('k', v); - but yes, we want to run the "initial"
version of set(), not the current  value of Map.prototype.set

I can't follow the v8 code enough to know for certain if v8::Map::Set is
overridden by a change to Map.prototype.set; the impl (src/api.cc) looks like it
calls into the registered 'map_set' which maps to MapSet (src/js/collection.js);
this does not appear to go through properties on the instance itself, but the
indirection in src/contexts.h is not clear to me.

palakj: you could test by running on a page which does e.g. Map.prototype.set =
function(k, v) { console.log('overridden!', k, v); } and then ensure that
observation maps are populated (i.e. the overridden code did not execute)

[haraken, 2016/07/20 19:02:14]

Thanks for the clarification! Yeah, I realized that CreateDataProperty is
definitely not correct.

[adamk, 2016/07/20 19:04:04]

jsbell's reading of the code is correct: v8::Map::Set() always calls the
original Map.prototype.set function, no matter what the author code may have
done to it since the context was created. So calling Map::Set() is the right way
to fill a Map.

Apologies for the fact that Map::Set and Object::Set have the same name, I
didn't know what else to call the former.

+    }
+    return ScriptValue::from(scriptState, map);
 }
 
-IDBObserverChanges* IDBObserverChanges::create(IDBDatabase* database, IDBTransaction* transaction, IDBAny* records)
+IDBObserverChanges* IDBObserverChanges::create(IDBDatabase* database, const WebVector<WebIDBObservation>& observations, const WebVector<int32_t>& observationIndex)
 {
-    return new IDBObserverChanges(database, transaction, records);
+    return new IDBObserverChanges(database, observations, observationIndex);
 }
 
-IDBObserverChanges::IDBObserverChanges(IDBDatabase* database, IDBTransaction* transaction, IDBAny* records)
+IDBObserverChanges::IDBObserverChanges(IDBDatabase* database, const WebVector<WebIDBObservation>& observations, const WebVector<int32_t>& observationIndex)
     : m_database(database)
-    , m_transaction(transaction)
-    , m_records(records)
 {
+    extractChanges(observations, observationIndex);
+}
+
+void IDBObserverChanges::extractChanges(const WebVector<WebIDBObservation>& observations, const WebVector<int32_t>& observationIndex)
+{
+    for (const auto& idx : observationIndex)
+        m_records.add(observations[idx].objectStoreId, HeapVector<Member<IDBObservation>>()).storedValue->value.append(IDBObservation::create(observations[idx]));
 }
 
 DEFINE_TRACE(IDBObserverChanges)