Closed Bug 836513 Opened 7 years ago Closed 7 years ago

IndexedDBHelper should pass store names, not a database name to IDBDatabase.transaction()

Categories

(Core :: DOM: Device Interfaces, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21
blocking-b2g tef+
Tracking Status
firefox20 --- wontfix
firefox21 --- fixed
b2g18 + fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: reuben, Assigned: reuben)

References

Details

Attachments

(2 files, 2 obsolete files)

The only reason it works right now is because all the users use the same database and store name, and only access a single store.
Attachment #708336 - Flags: review?(anygregor)
Blocks: 836519
I forgot to include ContactDB.jsm on the first patch.
Attachment #708336 - Attachment is obsolete: true
Attachment #708336 - Flags: review?(anygregor)
Attachment #708342 - Flags: review?(anygregor)
…and that one was missing a crucial change to ContactDB.jsm, using the STORE_NAME when creating the object store. Sorry for the spam.
Attachment #708342 - Attachment is obsolete: true
Attachment #708342 - Flags: review?(anygregor)
Attachment #708403 - Flags: review?(anygregor)
Comment on attachment 708403 [details] [diff] [review]
Support multiple object stores in IndexedDBHelper

Review of attachment 708403 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #708403 - Flags: review?(anygregor) → review+
https://hg.mozilla.org/mozilla-central/rev/66068b936790
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Bug Summary
> IndexedDBHelper should pass an array of store names, not a single dbname to
> IDBDatabase.transaction()

(In reply to Reuben Morais [:reuben] from comment #0)
> The only reason it works right now is because all the users use the same
> database and store name, and only access a single store.

Those two are completely different things. It is perfectly valid to pass just 1 store name to IDBDatabase.transaction() in the form of a string. And I think it's not a terrible assumption for IndexedDBHelper to assume that consumers only work with 1 store, because that has been our use case so far. What's not ok is to assume that store name == db name.
Duplicate of this bug: 837337
(In reply to Philipp von Weitershausen [:philikon] from comment #7)
> Those two are completely different things. It is perfectly valid to pass
> just 1 store name to IDBDatabase.transaction() in the form of a string. And
> I think it's not a terrible assumption for IndexedDBHelper to assume that
> consumers only work with 1 store, because that has been our use case so far.
> What's not ok is to assume that store name == db name.

The bug summary describes both problems, and my patch supports both use cases: the transaction is created with all store names passed to init — which allows you to call txn.objectStore(other_store) inside the callback if you want to —, but only one object store is passed to the callback, which is the common case in our code base. Is this clearer?
Summary: IndexedDBHelper should pass an array of store names, not a single dbname to IDBDatabase.transaction() → IndexedDBHelper should pass store names, not a database name to IDBDatabase.transaction()
(In reply to Philipp von Weitershausen [:philikon] from comment #7)
> Bug Summary
> > IndexedDBHelper should pass an array of store names, not a single dbname to
> > IDBDatabase.transaction()
> 
> (In reply to Reuben Morais [:reuben] from comment #0)
> > The only reason it works right now is because all the users use the same
> > database and store name, and only access a single store.
> 
> Those two are completely different things. It is perfectly valid to pass
> just 1 store name to IDBDatabase.transaction() in the form of a string. And
> I think it's not a terrible assumption for IndexedDBHelper to assume that
> consumers only work with 1 store, because that has been our use case so far.
> What's not ok is to assume that store name == db name.

We need it for bug 836519 where we have to deal with 2 object stores.
blocking-b2g: --- → tef?
(In reply to Gregor Wagner [:gwagner] from comment #10)
> (In reply to Philipp von Weitershausen [:philikon] from comment #7)
> > Bug Summary
> > > IndexedDBHelper should pass an array of store names, not a single dbname to
> > > IDBDatabase.transaction()
> > 
> > (In reply to Reuben Morais [:reuben] from comment #0)
> > > The only reason it works right now is because all the users use the same
> > > database and store name, and only access a single store.
> > 
> > Those two are completely different things. It is perfectly valid to pass
> > just 1 store name to IDBDatabase.transaction() in the form of a string. And
> > I think it's not a terrible assumption for IndexedDBHelper to assume that
> > consumers only work with 1 store, because that has been our use case so far.
> > What's not ok is to assume that store name == db name.
> 
> We need it for bug 836519 where we have to deal with 2 object stores.

https://bugzilla.mozilla.org/show_bug.cgi?id=836519#c5 let's get this landed on mozilla-central before considering for v1.0.0/v1.0.1
We will consider a low risk uplift nomination this week to get this in v1.0.1 but this isn't blocking any blockers and we're being very discriminating about what is a blocker at this point.
blocking-b2g: tef? → -
tracking-b2g18: --- → +
Batch edit: Bugs still affected on b2g18 after 2/13 merge to v1.0.1 branch are affected on v1.0.1 branch.
This blocks 836519 which blocks leo. Please land attachment 715651 [details] [diff] [review] in mozilla-b2g18.
Keywords: checkin-needed
blocking-b2g: - → leo?
blocking-b2g: leo? → leo+
blocking-b2g: leo+ → tef+
Blocking tef+, therefore this is tef+.
You need to log in before you can comment on or make changes to this bug.