[Webkit-unassigned] [Bug 103484] IndexedDB: Implement IndexedDB bindings for JSC

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Michael Pruett <michael at 68k.org>  2012-12-04 18:14:48 PST ---
(In reply to comment #6)
> > Source/WebCore/bindings/js/IDBBindingUtilities.cpp:130
> > +        if (currentValue.isString() && keyPathElements[i] == "length")
> > +            return jsNumber(currentValue.toString(exec)->length());
> > +        if (!currentValue.isObject())
> > +            return jsUndefined();
> 
> This behavior looks different from V8's one.

Thanks, I neglected to remove this unnecessary code when I cleaned up this method.

> > Source/WebCore/bindings/js/IDBBindingUtilities.cpp:148
> > +    if (jsValue.isUndefined())
> 
> This should be jsValue.isEmpty()?

I've used jsUndefined() as the error return value in ensureNthValueOnKeyPath() and getNthValueOnKeyPath(), so I believe the appropriate test is isUndefined() rather than isEmpty().

> > Source/WebCore/bindings/js/IDBBindingUtilities.cpp:228
> > +    ExecState* exec = requestState->exec();
> > +    APIEntryShim entryShim(exec);
> 
> You can use DOMRequestState::Scope.
> 
> BTW, V8 binding isn't creating a scope of DOMRequestState::Scope, which looks wrong.

I've removed all APIEntryShim scopes from IDBBindingUtilities.cpp. The V8 implementation of IDBBindingUtilities.cpp assumes that the proper DOMRequestState scope has already been established in the core IDB code, and the same now holds true for the JSC implementation.

> > Source/WebCore/bindings/js/JSIDBAnyCustom.cpp:88
> > +    case IDBAny::StringType:
> > +        return jsStringWithCache(exec, idbAny->string());
> 
> Just to confirm: Is there any reason why you moved this code from above to here?

Sorry, I've undone this unnecessary change.

> > Source/WebCore/bindings/js/JSIDBOpenDBRequestCustom.cpp:48
> > +void JSIDBOpenDBRequest::visitChildren(JSCell* cell, JSC::SlotVisitor& visitor)
> > +{
> > +    JSIDBOpenDBRequest* jsRequest = jsCast<JSIDBOpenDBRequest*>(cell);
> > +    ASSERT_GC_OBJECT_INHERITS(jsRequest, &s_info);
> > +    COMPILE_ASSERT(StructureFlags & OverridesVisitChildren, OverridesVisitChildrenWithoutSettingFlag);
> > +    ASSERT(jsRequest->structure()->typeInfo().overridesVisitChildren());
> > +    Base::visitChildren(jsRequest, visitor);
> > +    static_cast<IDBOpenDBRequest*>(jsRequest->impl())->visitJSEventListeners(visitor);
> > +}
> 
> Why do you need this? Event listeners are already cared by JS bindings, aren't they?

With the binding generator change in bug 103908, the visitChildren() methods of IDBOpenDBRequest and IDBVersionChangeRequest are now automatically generated, and these custom visitChildren() methods are unnecessary.

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