Bug 102018 – IndexedDB: simplify RecordIdentifier

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

[Alec Flett]

IndexedDB: simplify RecordIdentifier

[Alec Flett]

Created attachment 173771 [details]
Patch

[Alec Flett]

jsbell/dgrogan - quick once-over?

tony - r?

[WebKit Review Bot]

Comment on attachment 173771 [details]
Patch

Attachment 173771 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14813822

New failing tests:
storage/indexeddb/mozilla/object-store-remove-values.html
storage/indexeddb/index-unique.html
storage/indexeddb/cursor-skip-deleted.html
storage/indexeddb/mozilla/cursor-mutation.html
storage/indexeddb/cursor-delete.html
storage/indexeddb/cursor-index-delete.html
storage/indexeddb/mozilla/cursors.html
storage/indexeddb/delete-range.html

[Joshua Bell]

Sorry, just getting a chance to look at this now. The source of the test failures isn't obvious - were you able to track it down?

Overall the change looks good, although you may want to mark the constructors explicit.

[Alec Flett]

Created attachment 174566 [details]
Patch

[Alec Flett]

thanks for the pointer on explicit-ness. I ended up spinning that out into bug 102450 since the actual classes missing explicitness weren't from this patch.

[WebKit Review Bot]

Comment on attachment 174566 [details]
Patch

Attachment 174566 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14873080

New failing tests:
storage/indexeddb/mozilla/object-store-remove-values.html
storage/indexeddb/index-unique.html
storage/indexeddb/cursor-skip-deleted.html
storage/indexeddb/mozilla/cursor-mutation.html
storage/indexeddb/cursor-delete.html
storage/indexeddb/cursor-index-delete.html
storage/indexeddb/mozilla/cursors.html
storage/indexeddb/delete-range.html

[Alec Flett]

Created attachment 174770 [details]
Patch

[Alec Flett]

*** Bug 101668 has been marked as a duplicate of this bug. ***

[Alec Flett]

ok, the breakage was a stupid typo on my part (declared a subclass with a variable name that was identical to the parent class, etc)

[Alec Flett]

Created attachment 174771 [details]
Patch

One other minor optimization to avoid a Vector<char> copy

[Tony Chang]

Comment on attachment 174771 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBBackingStore.h:119
> +        virtual const RecordIdentifier& recordIdentifier() const  { return m_recordIdentifier; }

Nit: extra space between const and {

[Alec Flett]

Created attachment 174773 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 174773 [details]
Patch for landing

Rejecting attachment 174773 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/14860431

[Alec Flett]

Created attachment 174788 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 174788 [details]
Patch for landing

Rejecting attachment 174788 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
mit-queue/Source/WebKit/chromium/third_party/skia/src --revision 6424 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
48>At revision 6424.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/14880036

[Alec Flett]

Created attachment 175005 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 175005 [details]
Patch for landing

Clearing flags on attachment: 175005

Committed r135177: <http://trac.webkit.org/changeset/135177>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

Comment on attachment 175005 [details]
Patch for landing

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

> Source/WebCore/Modules/indexeddb/IDBBackingStore.h:76
> +        DISALLOW_COPY_AND_ASSIGN(RecordIdentifier);

Isn't this a Chromium macro rather than a WebKit macro?  /me is getting compile failures on this line.