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: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | OS X 10.5 | ||||||||||
Bug Depends on: | 57684 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Jeremy Orlow
2011-03-30 14:55:30 PDT
Created attachment 87623 [details]
Patch
100% mechanical cleanup...no logic changes LGTM Attachment 87623 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8309019 Attachment 87623 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8154154 Committed r82738: <http://trac.webkit.org/changeset/82738> This appears to have broken Broke storage/indexeddb/objectstore-autoincrement.html and InjectIDBKey. Re-opening since this got reverted. Created attachment 88053 [details]
fixed patch
Uploading patch again, with fix the should make all tests pass this time.
Attachment 88053 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8247991 Created attachment 88054 [details]
manual upload
webkit-patch upload fail; doing it manually.
Attachment 88053 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8185934 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. (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. Committed r82917: <http://trac.webkit.org/changeset/82917> http://trac.webkit.org/changeset/82917 might have broken GTK Linux 32-bit Debug Thanks Hans! Glad we had tests. :-) |