[Webkit-unassigned] [Bug 44700] IDBObjectStore::get should run in a transaction.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=44700


Jeremy Orlow <jorlow at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #66964|review?                     |review-
               Flag|                            |




--- Comment #7 from Jeremy Orlow <jorlow at chromium.org>  2010-09-09 06:57:40 PST ---
(From update of attachment 66964)
View in context: https://bugs.webkit.org/attachment.cgi?id=66964&action=prettypatch

> LayoutTests/storage/indexeddb/objectstore-basics-expected.txt:-142
> -
these lines should be here

> LayoutTests/storage/indexeddb/objectstore-removeobjectstore.html:78
> +    window.setTimeout('removeObjectStore()', 0);
Note that this will force the transaction to commit, but it won't necessarily have completed after this.  BUT you are guaranteed that it will before the removal happens.  Maybe just update the comment?

> LayoutTests/storage/indexeddb/objectstore-removeobjectstore.html:113
> +    shouldBe("event.code", "5");
Fix this fixme.

> LayoutTests/storage/indexeddb/script-tests/transaction-get.js:12
> +        shouldBe('exc.code', '9');
use the constant

> LayoutTests/storage/indexeddb/script-tests/transaction-get.js:34
> +    transaction.onabort = unexpectedErrorCallback;
Does the abort have an error code and message?  If not, you should make a new unexpectedAbortCallback method.

> LayoutTests/storage/indexeddb/transaction-get.html:11
> +<script src="script-tests/transaction-get.js"></script>
I'm about to submit a change that gets rid of script tests and converts them over to normal html pages.  Please convert your new one and merge your changes to the .js files to the associated html files?

> WebCore/bindings/v8/V8Proxy.cpp:655
> +    IDBPendingTransactionMonitor::abortPendingTransactions(page->group().idbFactory());
I'd rather this still call some blazingly fast method (completely inline) method to decide whether we need to call into abortPendingTransactions since this will be called _every_ single time we leave JavaScript.

> WebCore/storage/IDBDatabase.cpp:60
> +    // FIXME: remove this method.
Maybe describe why and move it to just above the method?  And capitalize R.

> WebCore/storage/IDBDatabase.cpp:63
> +    return IDBObjectStore::create(objectStore.release(), -1);
-1 should be some constant somewhere

> WebCore/storage/IDBDatabaseBackendImpl.cpp:153
> +    // FIXME: Do not expose this method via IDL.
This fixme seems unneeded.  Maybe add a fixme to remove it from the interface if appropriate tho?

> WebCore/storage/IDBDatabaseBackendImpl.cpp:157
> +        objectStore->setTransaction(transaction);
The backend object should not store the transaction since multiple transactions can be associated with one backend at once.  Only the frontend objhects should store it.

> WebCore/storage/IDBFactoryBackendImpl.cpp:150
> +    m_transactionCoordinator->didCompleteEventsForTransaction(transactionID);
Is there any reason why the coordinators should be shared between databases.

> WebCore/storage/IDBObjectStore.cpp:128
> +    if (!m_isTransactionPending)
Pending is so generic.

m_isTransactionScheduleForExecution?  :-)
m_isTransactionInPurgatory
m_inPurgatory
etc

btw, this really should be a per-transaction value, not a per object store.  Maybe we should pass around pointers rather than IDs.  Even though that means more wrapping/unwrapping.

> WebKit/chromium/src/IDBTransactionBackendProxy.cpp:81
> +    // FIXME: implement
use complete sentences


comments so far

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.