Bug 57507

Summary:

Clean up IndexedDB WebKit API

Product:

WebKit

Reporter:

Jeremy Orlow <jorlow>

Component:

New Bugs

Assignee:

Hans Wennborg <hans>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dglazkov, dgrogan, dgrogan, eric, hans, japhet, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

OS X 10.5

Bug Depends on:

57684

Bug Blocks:

Attachments:

Description	Flags
Patch	none
fixed patch	none
manual upload	abarth: review+

Jeremy Orlow

Reported 2011-03-30 14:55:30 PDT

Clean up IndexedDB WebKit API

Attachments
Patch (68.74 KB, patch) 2011-03-30 14:58 PDT, Jeremy Orlow	no flags	Details Formatted Diff Diff
fixed patch (68.69 KB, patch) 2011-04-04 05:59 PDT, Hans Wennborg	no flags	Details Formatted Diff Diff
manual upload (96.71 KB, patch) 2011-04-04 06:14 PDT, Hans Wennborg	abarth: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Jeremy Orlow

Comment 1 2011-03-30 14:58:00 PDT

Created attachment 87623 [details] Patch

Jeremy Orlow

Comment 2 2011-03-30 14:58:36 PDT

100% mechanical cleanup...no logic changes

David Grogan

Comment 3 2011-03-30 15:11:29 PDT

LGTM

WebKit Review Bot

Comment 4 2011-03-30 15:32:55 PDT

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

WebKit Review Bot

Comment 5 2011-03-30 23:45:05 PDT

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

Jeremy Orlow

Comment 6 2011-04-01 16:12:24 PDT

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

Adam Barth

Comment 7 2011-04-01 17:05:58 PDT

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

Hans Wennborg

Comment 8 2011-04-04 05:55:20 PDT

Re-opening since this got reverted.

Hans Wennborg

Comment 9 2011-04-04 05:59:54 PDT

Created attachment 88053 [details] fixed patch Uploading patch again, with fix the should make all tests pass this time.

WebKit Review Bot

Comment 10 2011-04-04 06:05:16 PDT

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

Hans Wennborg

Comment 11 2011-04-04 06:14:23 PDT

Created attachment 88054 [details] manual upload webkit-patch upload fail; doing it manually.

WebKit Review Bot

Comment 12 2011-04-04 06:19:40 PDT

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

Adam Barth

Comment 13 2011-04-04 13:19:28 PDT

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

Comment 14 2011-04-05 02:32:56 PDT

(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

Comment 15 2011-04-05 02:38:31 PDT

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

WebKit Review Bot

Comment 16 2011-04-05 03:50:50 PDT

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

Jeremy Orlow

Comment 17 2011-04-06 16:34:33 PDT

Thanks Hans! Glad we had tests. :-)

Note You need to log in before you can comment on or make changes to this bug.