Bug 47511 – [Chromium] Add plumbing for synchronous indexedDB exceptions

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jeremy Orlow]

[Chromium] Add plumbing for synchronous indexedDB exceptions

[Jeremy Orlow]

Nate/Darin can one of you guys review today?  Thanks!

[Jeremy Orlow]

Created attachment 70487 [details]
Patch

[Nate Chapin]

Comment on attachment 70487 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=70487&action=review

r+ provided my comments/guesses are correct :)

> WebKit/chromium/public/WebExceptionCode.h:36
> +// This should match how exception code is defined in WebCore.

Nit: replace 'exception code' with 'ExceptionCode' so it's clear what this is referring to?

> WebKit/chromium/public/WebIDBCursor.h:65
> +    virtual void update(const WebSerializedScriptValue& value, WebIDBCallbacks* callbacks, WebExceptionCode&)
> +    {
> +        update(value, callbacks);
> +    }
> +    virtual void update(const WebSerializedScriptValue& value, WebIDBCallbacks* callbacks)
> +    {
> +        WebExceptionCode ec = 0;
> +        update(value, callbacks, ec);
> +    }

It appears that a bunch of these implementations are circular.  I'm assuming that this code is unused and these are just to ensure the compiler doesn't give unused variable warnings?

> WebKit/chromium/public/WebIDBCursor.h:89
> +    /*
> +    virtual void update(const WebSerializedScriptValue&, WebIDBCallbacks*, WebExceptionCode&) { WEBKIT_ASSERT_NOT_REACHED(); }
> +    virtual void continueFunction(const WebIDBKey&, WebIDBCallbacks*, WebExceptionCode&) { WEBKIT_ASSERT_NOT_REACHED(); }
> +    virtual void remove(WebIDBCallbacks*, WebExceptionCode&) { WEBKIT_ASSERT_NOT_REACHED(); }
> +    */

Here and below:  Why are these commented out?  Are they going to replace the implementations above when the FIXMEs are resolved?

[Jeremy Orlow]

(In reply to comment #3)
> (From update of attachment 70487 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=70487&action=review
> 
> r+ provided my comments/guesses are correct :)
> 
> > WebKit/chromium/public/WebExceptionCode.h:36
> > +// This should match how exception code is defined in WebCore.
> 
> Nit: replace 'exception code' with 'ExceptionCode' so it's clear what this is referring to?

done
 
> > WebKit/chromium/public/WebIDBCursor.h:65
> > +    virtual void update(const WebSerializedScriptValue& value, WebIDBCallbacks* callbacks, WebExceptionCode&)
> > +    {
> > +        update(value, callbacks);
> > +    }
> > +    virtual void update(const WebSerializedScriptValue& value, WebIDBCallbacks* callbacks)
> > +    {
> > +        WebExceptionCode ec = 0;
> > +        update(value, callbacks, ec);
> > +    }
> 
> It appears that a bunch of these implementations are circular.  I'm assuming that this code is unused and these are just to ensure the compiler doesn't give unused variable warnings?

They're circular because the old implementation needs to point to the new _and_ the new needs to point to the old since Chrome and WebKit both implement and consume the API.

> > WebKit/chromium/public/WebIDBCursor.h:89
> > +    /*
> > +    virtual void update(const WebSerializedScriptValue&, WebIDBCallbacks*, WebExceptionCode&) { WEBKIT_ASSERT_NOT_REACHED(); }
> > +    virtual void continueFunction(const WebIDBKey&, WebIDBCallbacks*, WebExceptionCode&) { WEBKIT_ASSERT_NOT_REACHED(); }
> > +    virtual void remove(WebIDBCallbacks*, WebExceptionCode&) { WEBKIT_ASSERT_NOT_REACHED(); }
> > +    */
> 
> Here and below:  Why are these commented out?  Are they going to replace the implementations above when the FIXMEs are resolved?

Yup....just wanted to save myself some effort.  They'll be uncommented once WebKit rolls.

[Nate Chapin]

Ok, thanks.

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 70487 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=70487&action=review
> > 
> > r+ provided my comments/guesses are correct :)
> > 
> > > WebKit/chromium/public/WebExceptionCode.h:36
> > > +// This should match how exception code is defined in WebCore.
> > 
> > Nit: replace 'exception code' with 'ExceptionCode' so it's clear what this is referring to?
> 
> done
> 
> > > WebKit/chromium/public/WebIDBCursor.h:65
> > > +    virtual void update(const WebSerializedScriptValue& value, WebIDBCallbacks* callbacks, WebExceptionCode&)
> > > +    {
> > > +        update(value, callbacks);
> > > +    }
> > > +    virtual void update(const WebSerializedScriptValue& value, WebIDBCallbacks* callbacks)
> > > +    {
> > > +        WebExceptionCode ec = 0;
> > > +        update(value, callbacks, ec);
> > > +    }
> > 
> > It appears that a bunch of these implementations are circular.  I'm assuming that this code is unused and these are just to ensure the compiler doesn't give unused variable warnings?
> 
> They're circular because the old implementation needs to point to the new _and_ the new needs to point to the old since Chrome and WebKit both implement and consume the API.
> 
> > > WebKit/chromium/public/WebIDBCursor.h:89
> > > +    /*
> > > +    virtual void update(const WebSerializedScriptValue&, WebIDBCallbacks*, WebExceptionCode&) { WEBKIT_ASSERT_NOT_REACHED(); }
> > > +    virtual void continueFunction(const WebIDBKey&, WebIDBCallbacks*, WebExceptionCode&) { WEBKIT_ASSERT_NOT_REACHED(); }
> > > +    virtual void remove(WebIDBCallbacks*, WebExceptionCode&) { WEBKIT_ASSERT_NOT_REACHED(); }
> > > +    */
> > 
> > Here and below:  Why are these commented out?  Are they going to replace the implementations above when the FIXMEs are resolved?
> 
> Yup....just wanted to save myself some effort.  They'll be uncommented once WebKit rolls.

[Jeremy Orlow]

Committed r69541: <http://trac.webkit.org/changeset/69541>