Bug 57507 – Clean up IndexedDB WebKit API

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

[Jeremy Orlow]

Clean up IndexedDB WebKit API

[Jeremy Orlow]

Created attachment 87623 [details]
Patch

[Jeremy Orlow]

100% mechanical cleanup...no logic changes

[David Grogan]

LGTM

[WebKit Review Bot]

Attachment 87623 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8309019

[WebKit Review Bot]

Attachment 87623 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8154154

[Jeremy Orlow]

Committed r82738: <http://trac.webkit.org/changeset/82738>

[Adam Barth]

This appears to have broken Broke storage/indexeddb/objectstore-autoincrement.html and InjectIDBKey.

[Hans Wennborg]

Re-opening since this got reverted.

[Hans Wennborg]

Created attachment 88053 [details]
fixed patch

Uploading patch again, with fix the should make all tests pass this time.

[WebKit Review Bot]

Attachment 88053 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8247991

[Hans Wennborg]

Created attachment 88054 [details]
manual upload

webkit-patch upload fail; doing it manually.

[WebKit Review Bot]

Attachment 88053 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8185934

[Adam Barth]

Comment on attachment 88054 [details]
manual upload

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

Thanks for picking up this change.  What was the bug with the old patch?

> Source/WebKit/chromium/ChangeLog:14
> +        Second try: added WebCore:: qualifier to
> +        injectIDBKeyIntoSerializedValue in WebIDBKey.cpp.

Usually we don't put this kind of information in ChangeLogs.  We usually put it in bug comments because it's information about how the patch was produced, not why the change is being made.

> Source/WebKit/chromium/src/WebIDBTransactionImpl.cpp:2
> - * Copyright (C) 2010 Google Inc. All rights reserved.
> + * Copyright (C) 2011 Google Inc. All rights reserved.

We probably shouldn't change just the copyright in this file.

> Source/WebKit/chromium/src/WebIDBTransactionImpl.h:2
> - * Copyright (C) 2010 Google Inc. All rights reserved.
> + * Copyright (C) 2011 Google Inc. All rights reserved.

Or this one.

[Hans Wennborg]

(In reply to comment #13)
> (From update of attachment 88054 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88054&action=review
> 
> Thanks for picking up this change.  What was the bug with the old patch?

Jeremy added a "using namespace WebCore;" directive to WebIDBkey when he moved it into the WebKit namespace, and removed the WebCore:: qualifier from the call to injectIDBKeyIntoSerializedValue. This made it call the wrong thing, so I put the WebCore qualifier back on that call.

> 
> > Source/WebKit/chromium/ChangeLog:14
> > +        Second try: added WebCore:: qualifier to
> > +        injectIDBKeyIntoSerializedValue in WebIDBKey.cpp.
> 
> Usually we don't put this kind of information in ChangeLogs.  We usually put it in bug comments because it's information about how the patch was produced, not why the change is being made.
Ok, removed.

> 
> > Source/WebKit/chromium/src/WebIDBTransactionImpl.cpp:2
> > - * Copyright (C) 2010 Google Inc. All rights reserved.
> > + * Copyright (C) 2011 Google Inc. All rights reserved.
> 
> We probably shouldn't change just the copyright in this file.
Fixed.

> 
> > Source/WebKit/chromium/src/WebIDBTransactionImpl.h:2
> > - * Copyright (C) 2010 Google Inc. All rights reserved.
> > + * Copyright (C) 2011 Google Inc. All rights reserved.
> 
> Or this one.
Fixed.

[Hans Wennborg]

Committed r82917: <http://trac.webkit.org/changeset/82917>

[WebKit Review Bot]

http://trac.webkit.org/changeset/82917 might have broken GTK Linux 32-bit Debug

[Jeremy Orlow]

Thanks Hans!  Glad we had tests.  :-)