Indicate if the connection is using a sharedCache or not

VERIFIED WONTFIX

Status

()

Toolkit
Storage
VERIFIED WONTFIX
10 years ago
10 years ago

People

(Reporter: sdwilsh, Assigned: sdwilsh)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
It'd be potentially useful for consumers to be able to know if a given database is using a shared cache or not.  They may be using a database that was not opened by them, and therefore have no idea what it was opened with.

Also, we'll need this information for bug 429986.
(Assignee)

Comment 1

10 years ago
Created attachment 321770 [details] [diff] [review]
v1.0

API change, so need sr as well.
Assignee: nobody → sdwilsh
Status: NEW → ASSIGNED
Attachment #321770 - Flags: superreview?(shaver)
Attachment #321770 - Flags: review?(vladimir)
(Assignee)

Updated

10 years ago
Whiteboard: [has patch][needs review vlad][needs sr shaver]
(Assignee)

Comment 2

10 years ago
alternatively, we could just make this public on mozStorageConnection, and make this bug dependent on the change in bug 434796.  No API change would be needed then.
(Assignee)

Comment 3

10 years ago
Not needed by bug 434796 anymore - it turns out we can actually use connection objects across multiple threads so I don't need to clone...
No longer blocks: 429986
Comment on attachment 321770 [details] [diff] [review]
v1.0

Looks ok, but add the attribute to the end of the interface?  Not sure what the compat implications here are (or whether we care -- js callers will be fine).
Attachment #321770 - Flags: review?(vladimir) → review+
(Assignee)

Comment 5

10 years ago
We don't need this for anything in 1.9.1 yet, so I'm inclined to leave it where it is and if compat is an issue, we'll just wait for 2.0.  I'm presently disinclined to change our nice organization of our idl files if at all possible.
Whiteboard: [has patch][needs review vlad][needs sr shaver] → [has patch][has review][needs sr shaver]
Comment on attachment 321770 [details] [diff] [review]
v1.0

What do we need it for in 2.0 that we don't need it for in 1.9.1?  I'm not sure what's motivating the change, but if we want people to be able to use sharedCache stuff in 1.9.1 (and I thought we did, which is why we did the work to enable much of it in 1.9.0 -- at non-zero cost) then this addition should go to the end to ease multi-version compatibility.
Attachment #321770 - Flags: superreview?(shaver) → superreview-
(Assignee)

Comment 7

10 years ago
(In reply to comment #6)
> (From update of attachment 321770 [details] [diff] [review])
> What do we need it for in 2.0 that we don't need it for in 1.9.1?  I'm not sure
> what's motivating the change, but if we want people to be able to use
> sharedCache stuff in 1.9.1 (and I thought we did, which is why we did the work
> to enable much of it in 1.9.0 -- at non-zero cost) then this addition should go
> to the end to ease multi-version compatibility.
Internally I don't think we need it (I initially needed it with my async work, but ended up not needing it).  With that said, I can see consumers finding it useful when using internal databases (like getting nsIDownloadManager::dbConnection) and trying to decide if it'll support virtual tables or not.

Additionally, bsmedberg said either on irc or in one of the newgroups that you must change the iid if you change the interface, and that will break binary compatibility anyway, so it doesn't matter where it goes in the interface (AFAIK).
Changing the IID breaks binary compatibility, but if the additional member is added at the end, then client code that knows it's not going to use it (and as you say, use of this is still uncertain -- couldn't they just try to create a virtual table and see if it failed for any reason, including lack of shared cache?) can just downcast to the shorter version of the interface and not have to bifurcate all of their code that deals with connections.
(Assignee)

Comment 9

10 years ago
Yes, they could just try and create a virtual table and see if it fails.  I'm OK with wontfixing this bug if others don't think it's needed at this point.
Let's do that, and REOPEN with this patch in play if the need arises.  Otherwise we might as well not add more API to support and bug habitat.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WONTFIX
(Assignee)

Updated

10 years ago
Status: RESOLVED → VERIFIED
(Assignee)

Updated

10 years ago
Whiteboard: [has patch][has review][needs sr shaver]
You need to log in before you can comment on or make changes to this bug.