[Webkit-unassigned] [Bug 36830] IndexedDB: Finish hooking up bindings for IndexedDatabaseRequest

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Mads Ager <ager at chromium.org>  2010-04-06 06:03:31 PST ---
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 52044 [details] [details])
> > r+ with a nit.
> > 
> > > +private:
> > > +    V8CustomIDBCallbacks(v8::Local<v8::Value> onSuccess, v8::Local<v8::Value> onError, Frame* frame)
> > > +        : m_onSuccessNull(!onSuccess->IsObject())
> > > +        , m_onSuccess(v8::Persistent<v8::Object>::New(onSuccess->ToObject()))
> > > +        , m_onErrorNull(!onError->IsObject())
> > > +        , m_onError(v8::Persistent<v8::Object>::New(onError->ToObject()))
> > > +        , m_frame(frame)
> > > +    {
> > > +    }
> > > +
> > > +    // FIXME: Should these be v8::Functions?  For some reason, VoidCallback (which this copied) uses v8::Objects.
> > > +    bool m_onSuccessNull;
> > > +    v8::Persistent<v8::Object> m_onSuccess;
> > > +    bool m_onErrorNull;
> > 
> > Why do we need separate bools here?  It seems like unnecessary duplication of
> > information to me.
> 
> How can I tell when a v8 object is valid or not?  I'm sure there is some way.
> 
> Going to commit this and continue working, but I am interested in making this
> cleaner.
> 
> + mads since he might know.

Sorry for the late reply, was on vacation.

You could leave the two persistent handles m_onSuccess and m_onError empty if
the arguments are not objects. Disposing empty handles is valid and your check
would be m_onSuccess.IsEmpty() instead of testing the boolean flag. Also, if
the arguments are objects, you do not need to call ToObject() you can use
handle cast instead (but it doesn't really matter since the ToObject call will
essentially do that in this case).

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