[Webkit-unassigned] [Bug 78167] New: [Chromium] IndexedDB: IDBVersionChangeRequest V8 wrapper not generated as ActiveDOMObject

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=78167

           Summary: [Chromium] IndexedDB: IDBVersionChangeRequest V8
                    wrapper not generated as ActiveDOMObject
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: Unspecified
        OS/Version: Unspecified
            Status: ASSIGNED
          Severity: Normal
          Priority: P2
         Component: WebKit Misc.
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: jsbell at chromium.org
                CC: abarth at webkit.org, jochen at chromium.org,
                    dgrogan at chromium.org


Created an attachment (id=126167)
 --> (https://bugs.webkit.org/attachment.cgi?id=126167&action=review)
Repro case

The IndexedDB implementation relies on the ActiveDOMObject concept to keep certain objects alive when they have pending activity. IDBRequest and its subclass IDBVersionChangeRequest need to be kept alive if they are expecting events to be raised, even if all of the references from script have been dropped.

However, this isn't working for IDBVersionChangeRequest, meaning that sometimes the events from IDBFactory.deleteDatabase() and IDBDatabase.setVersion() never fire. See attached repro; if run in DRT or Chrome with --js-flags='--expose-gc' to expose window.gc() the onsuccess event will never fire.

It appears that the IDBVersionChangeRequest.idl's derivation from IDBRequest is insufficient; the generated V8IDBVersionChangeRequest is missing the logic to ensure that the V8 wrapper is treated as an ActiveDOMObject. With this change:

--- a/Source/WebCore/storage/IDBVersionChangeRequest.idl
+++ b/Source/WebCore/storage/IDBVersionChangeRequest.idl
@@ -27,6 +27,7 @@ module storage {

     interface [
         Conditional=INDEXED_DATABASE,
+        ActiveDOMObject,
         EventTarget
     ] IDBVersionChangeRequest : IDBRequest {
         attribute EventListener onblocked;

The following is diff is produced in the generated V8IDBVersionChangeRequest.h/.cpp:

diff -uw 1/V8IDBVersionChangeRequest.h 2/V8IDBVersionChangeRequest.h
--- 1/V8IDBVersionChangeRequest.h    2012-02-08 15:25:48.927992208 -0800
+++ 2/V8IDBVersionChangeRequest.h    2012-02-08 15:26:26.368725887 -0800
@@ -25,7 +25,6 @@

 #include "IDBVersionChangeRequest.h"
 #include "V8DOMWrapper.h"
-#include "V8IDBRequest.h"
 #include "WrapperTypeInfo.h"
 #include <v8.h>
 #include <wtf/HashMap.h>
@@ -35,7 +34,7 @@

 class V8IDBVersionChangeRequest {
 public:
-    static const bool hasDependentLifetime = V8IDBRequest::hasDependentLifetime;
+    static const bool hasDependentLifetime = true;
     static bool HasInstance(v8::Handle<v8::Value>);
     static v8::Persistent<v8::FunctionTemplate> GetRawTemplate();
     static v8::Persistent<v8::FunctionTemplate> GetTemplate();
@@ -46,6 +45,7 @@
     inline static v8::Handle<v8::Object> wrap(IDBVersionChangeRequest*);
     static void derefObject(void*);
     static WrapperTypeInfo info;
+    static ActiveDOMObject* toActiveDOMObject(v8::Handle<v8::Object>);
     static const int eventListenerCacheIndex = v8DefaultWrapperInternalFieldCount + 0;
     static const int internalFieldCount = v8DefaultWrapperInternalFieldCount + 1;
     static v8::Handle<v8::Object> existingWrapper(IDBVersionChangeRequest*);
@@ -56,7 +56,7 @@

 ALWAYS_INLINE v8::Handle<v8::Object> V8IDBVersionChangeRequest::existingWrapper(IDBVersionChangeRequest* impl)
 {
-    return getDOMObjectMap().get(impl);
+    return getActiveDOMObjectMap().get(impl);
 }

 v8::Handle<v8::Object> V8IDBVersionChangeRequest::wrap(IDBVersionChangeRequest* impl)

diff -uw 1/V8IDBVersionChangeRequest.cpp 2/V8IDBVersionChangeRequest.cpp
--- 1/V8IDBVersionChangeRequest.cpp    2012-02-08 15:25:48.927992208 -0800
+++ 2/V8IDBVersionChangeRequest.cpp    2012-02-08 15:26:26.368725887 -0800
@@ -37,7 +37,7 @@

 namespace WebCore {

-WrapperTypeInfo V8IDBVersionChangeRequest::info = { V8IDBVersionChangeRequest::GetTemplate, V8IDBVersionChangeRequest::derefObject, 0, &V8IDBRequest::info };
+WrapperTypeInfo V8IDBVersionChangeRequest::info = { V8IDBVersionChangeRequest::GetTemplate, V8IDBVersionChangeRequest::derefObject, V8IDBVersionChangeRequest::toActiveDOMObject, &V8IDBRequest::info };

 namespace IDBVersionChangeRequestInternal {

@@ -114,6 +114,10 @@
     return GetRawTemplate()->HasInstance(value);
 }

+ActiveDOMObject* V8IDBVersionChangeRequest::toActiveDOMObject(v8::Handle<v8::Object> object)
+{
+    return toNative(object);
+}      

 v8::Handle<v8::Object> V8IDBVersionChangeRequest::wrapSlow(IDBVersionChangeRequest* impl)
 {
@@ -128,7 +132,7 @@

     if (!hasDependentLifetime)
         wrapperHandle.MarkIndependent();
-    getDOMObjectMap().set(impl, wrapperHandle);
+    getActiveDOMObjectMap().set(impl, wrapperHandle);
     return wrapper;
 }

It seems like that should happen "automagically" in CodeGeneratorV8.pm, but perhaps adding the ActiveDOMObject annotation to the derived class is the right fix?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.