- From: Jonas Sicking <jonas@sicking.cc>
- Date: Sun, 27 Jun 2010 00:27:25 -0700
- To: Jeremy Orlow <jorlow@chromium.org>
- Cc: Webapps WG <public-webapps@w3.org>
On Sat, Jun 26, 2010 at 11:44 AM, Jeremy Orlow <jorlow@chromium.org> wrote: > On Sat, Jun 26, 2010 at 10:09 AM, Jonas Sicking <jonas@sicking.cc> wrote: >> >> On Fri, Jun 25, 2010 at 2:43 AM, Jeremy Orlow <jorlow@chromium.org> wrote: >> >> > I'm OK with making createObjectStore/createIndex synchronous. It >> >> > would >> >> > definitely make such code cleaner and I don't see a major downside, >> >> > but >> >> > at >> >> > the same time I feel like this API is starting to get kind of ad-hoc >> >> > and >> >> > unintuitive to a new user. Having the create and the remove >> >> > functions >> >> > completely different and in different places seems weird. >> >> > So I agree with either A or leaving things as originally proposed by >> >> > Jonas >> >> > in "[IndexDB] Proposal for async API changes". >> >> >> >> Well, there's no reason not to make the same changes to >> >> removeObjectStore/removeIndex. Though the more I think with this >> >> design createObjectStore and removeObjectStore can stay on IDBDatabase >> >> and simply throw if called outside the appropriate transaction >> >> callback. >> >> >> >> Here is the revised proposal: >> >> >> >> interface IDBDatabase { >> >> ... >> >> IDBObjectStore createObjectStore (in DOMString name, in optional >> >> DOMString keyPath, in optional boolean autoIncrement); >> >> void removeObjectStore (in DOMString storeName); >> >> ... >> >> }; >> >> >> >> interface IDBObjectStore { >> >> ... >> >> IDBIndex createIndex (in DOMString name, in DOMString keyPath, in >> >> optional boolean unique); >> >> IDBRequest removeIndex (in DOMString indexName); >> >> ... >> >> }; >> >> >> >> Where createObjecStore/createIndex throws if a objectStore or index of >> >> the given name already exists, if the keyPath has an invalid syntax, >> >> or if the function is called when not inside a version-change >> >> transaction callback. And removeObjectStore/removeIndex throws if the >> >> objectStore or index doesn't exist or if the function is called when >> >> not inside a version-change transaction callback. >> >> >> >> Throwing if not inside the right type of callback isn't a super clean >> >> solution, but is really not that different from how add/put throws if >> >> called when not inside a READ_WRITE transaction. >> > >> > Just to be clear, no async function (one that returns an IDBRequest) >> > should >> > _ever_ throw. (They should only call onerror.) I assume you didn't >> > mean >> > "add/put throws" literally? >> >> Well, there's no reason we couldn't make add/put throw if called from >> inside the wrong type of transaction. I guess I don't feel strongly >> about it, but it seems to me that calling put from inside a READ_ONLY >> transaction is a much bigger mistake than writing a duplicate key or >> running out of disc space. Additionally it's a situation where we >> synchronously know that there is a bug in the program. > > When this came up before, the thought was that it'd be confusing to web > developers which errors would raise immediately and which would call the > error callback. My feeling is that consistency is more useful than raising > right away for errors like this. (Plus, if we find it hard to get web > developers to do any error checking, it seems even harder to get them to do > it in 2 ways. :-) Well, I wouldn't expect them to check for put-in-wrong transaction, and thus have that error bubble up and become a top level error, which is handled through the usual error reporting mechanisms which should alert the developer of a bug in their code. So it's actually intentional that this error is handled different from other errors. But if everyone else feels otherwise I'll shut up and accept that strategy. >> >> > But, I feel pretty strongly that a setVersion/schema change >> >> > transaction >> >> > should not simply kill off anything else currently running. The >> >> > reason >> >> > is >> >> > that it's not hard for apps to recover from a connection failing, but >> >> > it >> >> > is >> >> > hard to handle a transaction failing in an unpredictable way. >> >> > Especially >> >> > static transactions (which should rarely fail committing since >> >> > serialization >> >> > can be guaranteed before the transaction starts). >> >> >> >> That might be a good idea for v1. I was planning on doing a separate >> >> thread for setVersion, but maybe it's tied enough to the topic of >> >> schema changes that it makes sense to bring up here. >> >> >> >> What I suggest is that when setVersion is called, we fire >> >> 'versionchange' event on all other open IDBDatabase objects. This >> >> event contains information of what the desired new version number is. >> >> If no other IDBDatabase objects are open for the specific database, no >> >> 'versionchange' events are fired. This allows pages using the old >> >> schema version to automatically save any pending data (for example any >> >> draft emails) and display UI to the user suggesting that the tab be >> >> closed. If possible without dataloss, the tab could even reload itself >> >> to automatically load an updated version of the page which uses the >> >> new schema version. >> >> >> >> The 'versionchange' event would use an interface like >> >> >> >> interface IDBVersionEvent : IDBEvent { >> >> readonly attribute string version; >> >> }; >> > >> > First of all, what I was originally advocating (sorry for not being >> > clear) >> > is that we should kill the database connection but not until all active >> > transactions are complete. Though we should probably block new >> > transactions >> > from starting once setVersion is called. >> > But I really like your versionchange event idea regardless. I agree >> > that >> > letting the app sync any data that might be in memory (for example, a >> > draft >> > email) is important. And the idea that the web app could refresh itself >> > (or >> > download new application code or something) seems pretty cool and >> > useful. >> > I'm fine with it firing on all frames except the one that initiated >> > (like >> > storage events). If we go with the "kill the connection once all active >> > transactions are done and block new ones from starting", we'd want to >> > start >> > the blocking only after all versionchange events have finished. >> > The main reason that I like the idea of not stating the version change >> > until >> > all active connections have closed is that not all apps will handle >> > versionchange. My original idea was that we should just break such web >> > apps >> > and let the user refresh, but now that you've pointed out the potential >> > for >> > data loss I'm not sure that's an option. Savvy web apps can kill all >> > existing database connections when they get the versionchange and thus >> > avoid >> > stalling things. >> > >> >> Additionally, if there are open IDBDatabase objects, we fire a >> >> 'blocked' event at the IDBRequest object returned from the setVersion >> >> call. This allows the page to display UI to the user asking the user >> >> to close all other relevant tabs. >> >> >> >> Once all other IDBDatabase objects are closed, we create a transaction >> >> and fire the normal 'success' event. >> >> >> >> While there are pending version change requests, no success events are >> >> fired for calls to IDBFactory.open for the relevant database. >> >> >> >> We might want to support forcefully killing off open IDBDatabase >> >> objects in the future, but I think that can wait until after v1. >> > >> > Really? I can't see an app like gmail ever asking users to close tabs. >> > I >> > bet they'd sooner run all the application logic in an iframe and >> > navigate it >> > away when doing a schema change. >> > And I don't see many people correctly implementing a blocked event >> > handler. >> > If anything, it should be an error code. >> > It doesn't seem that hard to have an explicit way to tell the database >> > explicitly "OK, I'm done". Or, at very least, we could make it so that >> > when >> > there's an existing setVersion happening, all new connection requests >> > stall. >> > That way all pages can reload themselves but they won't connect to the >> > database until the upgrade is complete. >> > But really...all of this is really hacky. I'm starting to wonder if we >> > should just kill the database connections on a setVersion as I >> > originally >> > tried to suggest. >> >> I'm pretty concerned though that sites will need to take asynchronous >> actions in order to save all required data. While gmail happily >> automatically saves every few minutes, and presumably could >> immediately do so upon a 'versionchange' event, I don't think all >> editors are willing t. For example many editors ask the user if they >> want to save the current changes when they are closed, in order to not >> overwrite correct data. >> >> Additionally, there is always the risk that developers will forget to >> use a versionchange event handler to protect their data. I think a >> good design principal is that if sites do the minimal amount of work, >> that should default to safe behavior. >> >> I do realize that not all applications are willing to do the "please >> close all other tabs" UI thing. But for those we would provide enough >> tools do something better. If we add a IDBDatabase.close() function >> then applications that can easily emulate the "force-close open >> connections" using that and the versionchange event. And they could >> even use that to implement asynchronous data saving for when that is >> required. > > Yeah. I think giving developers a .close() (or maybe even a > .closeOthers()?) should be enough. It still feels clunky to me, but I also > think it'd be dangerous to over-design this without some feedback from > several developers using this in the real world...which won't happen until > we ship something (even if its basic). Ok, I'll file a bug. / Jonas
Received on Sunday, 27 June 2010 07:34:48 UTC