Bug 69264 - IndexedDB: Remove SQLite-LevelDB migration code
Summary: IndexedDB: Remove SQLite-LevelDB migration code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hans Wennborg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-03 09:22 PDT by Hans Wennborg
Modified: 2011-10-06 01:59 PDT (History)
4 users (show)

See Also:


Attachments
Patch (25.83 KB, patch)
2011-10-03 09:26 PDT, Hans Wennborg
tony: review+
hans: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hans Wennborg 2011-10-03 09:22:55 PDT
IndexedDB: Remove SQLite-LevelDB migration code
Comment 1 Hans Wennborg 2011-10-03 09:26:22 PDT
Created attachment 109491 [details]
Patch
Comment 2 David Grogan 2011-10-03 10:54:20 PDT
Comment on attachment 109491 [details]
Patch

LGTM

Is the next step to get rid of USE(LEVELDB)?
Comment 3 Hans Wennborg 2011-10-04 09:49:47 PDT
(In reply to comment #2)
> (From update of attachment 109491 [details])
> LGTM
> 
> Is the next step to get rid of USE(LEVELDB)?

Yes, ENABLE(INDEXEDDB) would imply using LevelDB, because that'll be the only back-end.

Tony: would you like to take an official-reviewer look?
Comment 4 Tony Chang 2011-10-04 12:17:51 PDT
Comment on attachment 109491 [details]
Patch

Are there uses with indexdb data still in sqlite format or am I misunderstanding what this code is for?
Comment 5 David Grogan 2011-10-04 12:22:51 PDT
(In reply to comment #4)
> (From update of attachment 109491 [details])
> Are there uses with indexdb data still in sqlite format or am I misunderstanding what this code is for?

We think that there are no remaining users with IDB data stored in sqlite.
Comment 6 Tony Chang 2011-10-04 13:31:09 PDT
Comment on attachment 109491 [details]
Patch

I'm skeptical, but if you say so.  What if there's a user who hasn't run chromium for the past 6 months?

Historically, we keep migration code forever.  For example, the history database format has changed many times, but we still have all the migration code in handle formats that are 3+ years old.

Is there a large benefit to removing the migration code?  Certainly code for writing to sqlite can be removed, but that seems orthogonal to the migration code.
Comment 7 David Grogan 2011-10-04 13:48:10 PDT
(In reply to comment #6)
> (From update of attachment 109491 [details])
> I'm skeptical, but if you say so.  What if there's a user who hasn't run chromium for the past 6 months?

You're right, "no remaining users with sqlite data" is a bit strong.  I suppose "anyone that still has sqlite data doesn't care about it" would be more accurate.

We've talked about it a bunch and decided that removing the sqlite code is worth losing any residual sqlite stores.  Between the fact that IDB isn't yet widely adopted (let alone 6 months ago) and that most apps just it as a cache or temporary storage, we're not really worried about it.  (We considered not writing any migration code at all.)

> Is there a large benefit to removing the migration code?  Certainly code for writing to sqlite can be removed, but that seems orthogonal to the migration code.

I could see the merit in that; I'll let Hans respond.
Comment 8 Hans Wennborg 2011-10-05 04:18:04 PDT
(In reply to comment #7)
> > Is there a large benefit to removing the migration code?  Certainly code for writing to sqlite can be removed, but that seems orthogonal to the migration code.
> 

The benefit is that it cleans up our existing code for opening a database, and it reduces the amount of code we need to maintain.

The plan is to remove all of the IndexedDB SQLite code.
Comment 9 Tony Chang 2011-10-05 10:34:11 PDT
It looks like I was wrong about the Chromium history database.  According to the comments, it supports versions that any shipping product supports.  If we want to use the same rule here, we should keep the migration code until we stop shipping Chromium that would create a sqlite backed indexdb.

Is it possible for Chromium 14 to use the sqlite backend?
Comment 10 Hans Wennborg 2011-10-05 11:36:39 PDT
(In reply to comment #9)
> It looks like I was wrong about the Chromium history database.  According to the comments, it supports versions that any shipping product supports.  If we want to use the same rule here, we should keep the migration code until we stop shipping Chromium that would create a sqlite backed indexdb.
> 
> Is it possible for Chromium 14 to use the sqlite backend?

No version of Chrome from 14 and onwards will create a SQLite database for IndexedDB. Anyone who has had Chrome 14 or 15 will have been migrated. So we feel it's safe to remove it now.

I'll land this tomorrow unless someone raises objections.
Comment 11 Hans Wennborg 2011-10-06 01:59:29 PDT
Committed r96793: <http://trac.webkit.org/changeset/96793>