Bug 101415 – IndexedDB: Combine IDBBackingStore and IDBLevelDBBackingStore

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

[Alec Flett]

IndexedDB: Combine IDBBackingStore and IDBLevelDBBackingStore

[Alec Flett]

Created attachment 172687 [details]
Patch

[Alec Flett]

jsbell/dgrogan - this is a decent sized refactor, but do you mind taking a look? It's mechanical, but check the changelog for the exact details. Beyond the reduction in code complexity, this also removes roughly 200 lines of code.

[Alec Flett]

oh, and jsbell this will most certainly conflict with bug 100700 - I'm happy to wait for that to land and deal with the fallout.

[WebKit Review Bot]

Comment on attachment 172687 [details]
Patch

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

[Peter Beverloo (cr-android ews)]

Comment on attachment 172687 [details]
Patch

Attachment 172687 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14732979

[David Grogan]

Comment on attachment 172687 [details]
Patch

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

Just a cursory look

> Source/WebCore/ChangeLog:23
> +        owner, IDBCursorBackendImpl.

I think you mean IDBTransactionBackendImpl?

> Source/WebCore/ChangeLog:25
> +        4) Shortened the name of a few classes/methods (e.g.

Could you do 4, 5, and 6 in separate patches?  Maybe 3 too?

> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:-408
> -        backingStoreCursor->close();

This line seems scary, why don't we have to close the backingStoreCursor anymore?

[Build Bot]

Comment on attachment 172687 [details]
Patch

Attachment 172687 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14760198

[EFL EWS Bot]

Comment on attachment 172687 [details]
Patch

Attachment 172687 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14731952

[Alec Flett]

Created attachment 172856 [details]
Patch

[Alec Flett]

ok, I've broken this up into 3 patches. The first 2 are reasonably simple, and this bug has the "final" bit that couldn't be split up any further... but as you'll see it is way easier to read now.

Regarding closing the cursor - so IDBLevelDBBackingStore::Cursor::close() was a no-op - so I nuked it - if anything it demonstrates why having so many layers of abstraction and interfaces obfuscate and clutter up other code - it's hard for us to figure out what parts of what interfaces are actually dead or not!

[Alec Flett]

Comment on attachment 172856 [details]
Patch

jsbell/dgrogan - care to take a look?

[Alec Flett]

(FYI, this patch won't apply cleanly to chromium until the next webkit roll)

[Alec Flett]

Created attachment 173096 [details]
Patch

[Alec Flett]

Created attachment 173101 [details]
Patch

[Alec Flett]

my changelogs went haywire in the last patch. I also fixed the histogram enums back the way they were. Patch is otherwise the same.

[Alec Flett]

Created attachment 173153 [details]
Patch

Update after bug 100700 landed

[David Grogan]

LGTM

[Alec Flett]

tony@ - another refactor for ya. r?

[Tony Chang]

Comment on attachment 173153 [details]
Patch

rs=me

[WebKit Review Bot]

Comment on attachment 173153 [details]
Patch

Clearing flags on attachment: 173153

Committed r134101: <http://trac.webkit.org/changeset/134101>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by bug 101790

[Alec Flett]

Created attachment 173357 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 173357 [details]
Patch for landing

Rejecting attachment 173357 [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/14680479

[Alec Flett]

Created attachment 173362 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 173362 [details]
Patch for landing

Rejecting attachment 173362 [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:
nt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...
Updating OpenSource
From http://git.chromium.org/external/Webkit
   955bc47..a388f5d  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 134117 = 5609c0bf5278e86109c3d178cfb12786bf7f48d5
last_rev is higher!: 134117 >= 134113 at /usr/lib/git-core/git-svn line 1523

Died at Tools/Scripts/update-webkit line 154.

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

[Alec Flett]

Comment on attachment 173362 [details]
Patch for landing

trying again - I think the git mirror got wedged or something

[WebKit Review Bot]

Comment on attachment 173362 [details]
Patch for landing

Clearing flags on attachment: 173362

Committed r134129: <http://trac.webkit.org/changeset/134129>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.