[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





--- Comment #18 from Jeremy Orlow <jorlow at chromium.org>  2010-09-21 02:16:41 PST ---
(In reply to comment #14)
> All fixed, except for the below:
> 
> (In reply to comment #12)
> > > WebCore/storage/IDBObjectStore.cpp:70
> > >      return request;
> > 
> > .release()
> >
> 
> Why release? The object store is associated with this transaction and other operations may follow.

Release just releases one ref as a pass ref pointer.  If your last use of a RefPtr is use variable in something that takes a passRefPtr, you should always call .release() so it doesn't have to ref it only to have the RefPtr get rid of its ref a moment later because it's gone out of scope.

> > > WebCore/storage/IDBObjectStoreBackendImpl.cpp:84
> > > +    if (transaction) {
> > 
> > Would this ever not be true?
> > 
> 
> 
> Yes, we use this to differentiate between the method being called by the object store or by the transaction.

Then it's a compat hack?  If so, add a fixme.

> > > WebCore/storage/IDBTransaction.cpp:70
> > > +        throwError(NOT_SUPPORTED_ERR);
> > 
> > Do you need to add the string for this error somewhere?
> >
> 
> I don't think so, it's a standard exception.

Wait...why didn't you use an IndexedDB exception code?  This one's probably not valid.


(In reply to comment #16)
> (In reply to comment #13)
> > 
> > > LayoutTests/storage/indexeddb/transaction-get.html:18
> > > +        debug("Accessing a committed transaction should throw");
> > 
> > This should be a failure, not a debug
> > 
> > It's ok to check in a failing result if that's what it should be.
> > 
> > > LayoutTests/storage/indexeddb/transaction-get.html:22
> > > +        shouldBe('exc.code', '9');
> > 
> > use the constant
> > 
> > > LayoutTests/storage/indexeddb/transaction-get.html:27
> > > +function nonExistingKey() {
> > 
> > all functions should put { on the next line
> > 
> > > LayoutTests/storage/indexeddb/transaction-get.html:41
> > > +    transaction.onabort = unexpectedErrorCallback;
> > 
> > newline after this maybe?
> > 
> > > LayoutTests/storage/indexeddb/transaction-get.html:44
> > > +    var result = store.get('myKey');
> > 
> > put in evalAndLog
> > 
> > > LayoutTests/storage/indexeddb/transaction-get.html:49
> > > +    var emptyResult = store.get('nonExistingKey');
> > 
> > put at the end of gotValue (yes, this is supposed to work, but that'll make it easier to read)
> > 
> > > LayoutTests/storage/indexeddb/transaction-get.html:80
> > > +function test() {
> > 
> > From now on, please write new tests from top down in terms of execution and function names saying what just happened, please.
> > 
> 
> These were added in the previous patch. Will look and fix tomorrow.

sounds good

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