The implementation will be done as a diploma thesis, here's the goal of the thesis: The goal of this work is to implement the IndexedDB synchronous API in Firefox browser and in the new mobile platform Firefox OS. These platforms already support the asynchronous API of the database, but the missing implementation of the synchronous API, which is currently unsupported in the other known web browsers too, partially prevents broader deployment and increase of popularity of IndexedDB among developers of web applications. 1. Describe HTML5 applications in general and local storage of data (especially IndexedDB database) 2. Implementation of the synchronous IndexedDB API in Firefox / Firefox OS 3. Describe the implementation in terms of architecture, description of objects, interactions between them as well as description of communication between threads. 4. Evaluation of results

New attachment adds WebIDL files to dom/webidl and *.h *.cpp files to dom/workers with stub methods for sync API of IDB. DOMStringList attributes and parameters are commented out because they are not yet available (see bug 803106 ) and the same with callbacks (see bug 812783 ).

The actual state of work. In the case of testing this patch need to be applied on top of the ipcthreadbase patch from bug 809888 Methods(m)/properties(p) which mostly work: IDBDatabaseSync p: name,version,objectStoreNames m: transaction IDBFactorySync m: open, cmp IDBTransactionSync p: mode, db, m: objectStore IDBObjectStoreSync p: transaction, m: get, delete, clear, index, count IDBIndexSync p: objectStore m: openKeyCursor, get, getKey, count IDBCursorSync p: source, direction, key, primaryKey m: continue

Vendo is going to submit a new patch soon. The implementation is almost feature complete. Stuff that needs to be implemented: IDBFactory::DeleteDatabase() IDBTransactionSync::Error() IDBTransactionSync::Abort() onversionchange handling

For more information about current state of implementation visit: https://etherpad.mozilla.org/sync-idb

This version of the synchronous IDB API patch was officially attached to the diploma thesis (final version for university).

Hi, vendo. It would be great to have this feature. Could you please divide the patch into smaller chunks, so as to simplify the work of reviewers?

I just finished work on bringing the sync IDB patch up to date and I also rebased it on top of the temp storage, bug 785884. Sync IDB is a Q3 goal and we will be talking about it during the work week in Toronto. I've started working on an initial review and Ben Turner will do a super review after the first one is addressed. We will split the patch into two parts at least, the implementation and the tests. There's actually a standalone patch already, bug 809888

(In reply to David Rajchenbach Teller [:Yoric] <on workweek, will not follow bugzilla until July 1st> from comment #9) > Hi, vendo. > It would be great to have this feature. Could you please divide the patch > into smaller chunks, so as to simplify the work of reviewers? I forgot to ask, what is your use case of sync IDB ?

> I just finished work on bringing the sync IDB patch up to date and I also rebased it on top of the temp storage, bug 785884. Ah, great. I was a bit worried because I saw a huge patch without a review flag, so I was afraid it would stay here and bitrot. > I forgot to ask, what is your use case of sync IDB ? I have no immediate use case, but I'm tech leading the Async project [1] and I'd like to evaluate sync IDB to determine whether I should suggest this as a storage solution for add-on authors. [1] https://bugzilla.mozilla.org/buglist.cgi?cmdtype=runnamed&namedcmd=Async

I just found this: "Rationale: Workers are coming along great, but of course it would be great to have more: IndexedDB access, mozTCPSocket access, better debug and profile support for them. Rationale for Gaia: Email app uses a worker with moxTCPSocket and IndexedDB. Right now it proxies back to the main thread to do IO with those capabilities. It complicates the code. It is also difficult to debug issues in the workers as well as get accurate info on profiling what happens in them." The current implementation of sync IDB API is single process only, we will need to add support for child processes too.

(In reply to Jan Varga [:janv] from comment #13) > Rationale for Gaia: Email app uses a worker with moxTCPSocket and ... > The current implementation of sync IDB API is single process only, we will > need to add support for child processes too. For Gaia, I think it's always expected that each app (with its own origin) will only ever operate within a single process, so that might eliminate that concern. For the Gaia e-mail app (in regards to that blurb), we'd really prefer the async API because of our architecture. But we can use the sync API in a sub-worker so that we're remoting to another background thread rather than the main thread.

(In reply to Andrew Sutherland (:asuth) from comment #14) > (In reply to Jan Varga [:janv] from comment #13) > > Rationale for Gaia: Email app uses a worker with moxTCPSocket and > ... > > The current implementation of sync IDB API is single process only, we will > > need to add support for child processes too. > > For Gaia, I think it's always expected that each app (with its own origin) > will only ever operate within a single process, so that might eliminate that > concern. I actually meant that the current implementation works in Firefox Desktop only (one single process) and that will need to add support for workers created from child processes. > > For the Gaia e-mail app (in regards to that blurb), we'd really prefer the > async API because of our architecture. But we can use the sync API in a > sub-worker so that we're remoting to another background thread rather than > the main thread. ok, thanks for this info

(In reply to Andrew Sutherland (:asuth) from comment #14) > For the Gaia e-mail app (in regards to that blurb), we'd really prefer the > async API because of our architecture. But we can use the sync API in a > sub-worker so that we're remoting to another background thread rather than > the main thread. This wording worries me a bit. "But we can use the sync API in a sub-worker" suggests that you could also decide otherwise (that is using the sync API in the main thread). Last I discussed this with Jonas Sicking [1] on this topic, it was clear that this cannot be an option. With the current implementation (or at least intent on this bug), will it be possible to use the sync API on the main thread? [1] http://lists.w3.org/Archives/Public/public-webapps/2012JulSep/0648.html and following messages

(In reply to David Bruant from comment #16) > This wording worries me a bit. "But we can use the sync API in a sub-worker" > suggests that you could also decide otherwise (that is using the sync API in > the main thread). Last I discussed this with Jonas Sicking [1] on this > topic, it was clear that this cannot be an option. It is indeed my understanding that the sync API would never be exposed to the main thread under any circumstances. What I was trying to express is that for the e-mail app, what we really want is async access to IndexedDB on a worker thread. We currently use only one worker and having it block on I/O would be very bad. But it seems like we could cobble together an async IndexedDB impementation out of a sub-worker and a synchronous-on-workers-only IndexedDB implementation if that is all we have. Currently, we see a non-trivial amount of main-thread jank (on a single-core non-beefy Firefox OS device) just from the structured clone operations bouncing our calls to the async IndexedDB off the main thread.

(In reply to David Bruant from comment #16) > With the current implementation (or at least intent on this bug), will it be > possible to use the sync API on the main thread? of course not :) using the sync API in a subworker should be ok

A rollup patch for those who want to try it with the current m-c. https://tbpl.mozilla.org/?tree=Try&rev=b0246a1406d2

This one actually builds on windows: https://tbpl.mozilla.org/?tree=Try&rev=a932c801c1d8

Additional development like review fixes, support for stored files, access from workers in child processes, etc. will be done on a disposable project branch. https://tbpl.mozilla.org/?tree=Jamun

Comment on attachment 746580 [details] [diff] [review] Sync IDB API (diploma thesis version) Review of attachment 746580 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/DatabaseInfoSync.h @@ +12,5 @@ > +#include "mozilla/dom/indexedDB/DatabaseInfo.h" > + > +BEGIN_WORKERS_NAMESPACE > + > +struct DatabaseInfoSync : public indexedDB::DatabaseInfoBase This should be called DatabaseInfoMT and the hashtable should be protected by a mutex. It would be also nice to have a test that creates multiple workers which access the same database.

Comment on attachment 746580 [details] [diff] [review] Sync IDB API (diploma thesis version) Review of attachment 746580 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/webidl/IDBCursorSync.webidl @@ +6,5 @@ > + * The origin of this IDL file is http://www.w3.org/TR/IndexedDB/ > + */ > + > +interface IDBCursorSync { > + readonly attribute object source; We decided to follow the style used in the specification [1], bug 888598. Please adjust all the webidl files to match the spec. [1] http://www.w3.org/TR/2012/WD-IndexedDB-20120524/ ::: dom/webidl/IDBDatabaseSync.webidl @@ +36,5 @@ > + optional unsigned long timeout); > + > + [Throws] > + /* FileHandleSync */ any > + mozCreateFileHandle(DOMString name, This is a mozilla specific method, thus it should be move to "partial interface IDBDatabaseSync" (see how it is done elsewhere). Also, you can remove the "moz" prefix in the implementation, take a look at 'binaryNames' for IDBIndex in Bindings.conf I think you can do that for IDBDatabase interface too. .mozStorage in IDBDatabaseSync should be moved to a partial interface too and make it pref controlled please

Comment on attachment 792717 [details] [diff] [review] A rollup patch Review of attachment 792717 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/Codegen.py @@ +3397,5 @@ > + # We have a specialization of Optional that will use a > + # Rooted for the storage here. > + declType = CGGeneric("JS::Handle<JSObject*>") > + else: > + declType = CGGeneric("JS::Rooted<JSObject*>") Vendo, please file a new bug for this and cc bz. This is actually a followup for bug 868715 part 10, code for callbacks in workers hasn't been updated properly I think.

Comment on attachment 808274 [details] [diff] [review] diff between m-c and jamun Review of attachment 808274 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/DatabaseInfoMT.h @@ +29,5 @@ > + already_AddRefed<DatabaseInfoMT> Clone(); > + > + nsCString id; > + > + static StaticMutex sDatabaseInfoMutex; Vendo, the mutex only protects Get/Put/Remove at the moment, we have to protect other methods too. For example GetObjectStore(), take a look at this crash: https://tbpl.mozilla.org/php/getParsedLog.php?id=28204915&tree=Jamun&full=1#error1

Comment on attachment 808274 [details] [diff] [review] diff between m-c and jamun Review of attachment 808274 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/IDBDatabaseSync.cpp @@ +524,5 @@ > + > + if (mUpgradeNeeded) { > + if (aUpgradeCallback) { > + ErrorResult rv; > + aUpgradeCallback->Call(mFactory, *mTransaction, 99, rv); Vendo, we need to pass a real number here and it seems we don't have a test for it. Async test suite has it in test_setVersion_events.js

Vendo, another thing ... please convert all NS_ASSERTION that we added to MOZ_ASSERT

It would be also nice to have getAllKeys and openKeyCursor in IDBObjectStoreSync. These were added to async API in bug 920633 and bug 920800.

IDBKeyRange is now shared by the main thread and workers

Comment on attachment 808274 [details] [diff] [review] diff between m-c and jamun Review of attachment 808274 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/WorkerScope.cpp @@ +1180,3 @@ > !EventBinding::GetConstructorObject(aCx, global) || > !FileReaderSyncBinding_workers::GetConstructorObject(aCx, global) || > + !IDBCursorSyncBinding_workers::GetProtoObject(aCx, global) || Vendo, all these GetProtoObject() should be changed to GetConstructorObject() ::: dom/workers/test/indexedDB/objectCursors_worker.js @@ +57,5 @@ > + > + } > + } > + > + ok(true, "Test successfully completed"); This can be: info("Test successfully completed"), you will need to add info() the helpers.

Most of the reviewers comments has been already addressed on jamun. I had a wonderful time at the Brussels summit and fixed comment#31 only today. OpenKeyCursor function will be added soon to IDBObjectStoreSync.

OpenKeyCursor function has been added.

Just to bring some context, we're currently pondering the possibility to quickly rewrite the sync IDB impl using Ben's new event loop and direct IPC in workers once it's ready or just let Ben review the current patch. Vendo said he's available to help with the rewrite.

Update: The current plan is to land existing patch, pref it off, get feedback, enable it once some other vendor is willing to implement it

The synchronous API didn't happen and is definitely no longer going to happen.