Closed Bug 492139 Opened 11 years ago Closed 11 years ago

Expose the Variant publicly so consumers can also use it

Categories

(Toolkit :: Storage, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: sdwilsh, Assigned: sdwilsh)

References

Details

Attachments

(2 files, 2 obsolete files)

This will be helpful for mozIStorageFunction implementors so they do not have to use nsIWritableVariant.

Also going to make a generic Storage header to include here so folks who want to use storage just #include "mozilla/storage/Storage.h"
Blocks: 455555
Attached patch v1.0 (obsolete) — Splinter Review
I'm basically copying what cjones uses for his new synchronization primaitves to make this work.  See http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/Makefile.in#135 for details.
Attachment #376509 - Flags: review?(benjamin)
Whiteboard: [needs review bsmedberg]
Also had to add #include "mozStorageCID.h" which is in the build/ folder.  That's in my patch queue this way, but I won't upload that change.
Summary: Expose the Variant publicaly so consumers can also use it → Expose the Variant publicly so consumers can also use it
Attached patch v1.1 (obsolete) — Splinter Review
I just bitrotted this, so here's an update that actually applies.
Attachment #376509 - Attachment is obsolete: true
Attachment #376519 - Flags: review?(benjamin)
Attachment #376509 - Flags: review?(benjamin)
Comment on attachment 376519 [details] [diff] [review]
v1.1

>diff --git a/storage/public/storage.h b/storage/public/storage.h

>+#ifndef _storage_h_
>+#define _storage_h_

Identifiers that start with underscores are reserved for the compiler/OS. I recommend mozilla_storage_h

r=me with that change
Attachment #376519 - Flags: review?(benjamin) → review+
(In reply to comment #4)
> Identifiers that start with underscores are reserved for the compiler/OS. I
> recommend mozilla_storage_h
That's a bit scary since I think we use that all over.  I'll be filing a bug to fix that in storage now.
Whiteboard: [needs review bsmedberg] → [needs new patch]
Attached patch v1.2Splinter Review
Addresses review comments for checkin.
Attachment #376519 - Attachment is obsolete: true
Whiteboard: [needs new patch] → [can land]
http://hg.mozilla.org/mozilla-central/rev/9afc23efcbba
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
Target Milestone: --- → mozilla1.9.2a1
This doesn't quite work...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch v2.0Splinter Review
Alright - this seems to do the trick.
Attachment #378452 - Flags: review?(benjamin)
Whiteboard: [needs review bsmedberg]
Comment on attachment 378452 [details] [diff] [review]
v2.0

Assuming this passes tryserver, r=me

note that in NS_IMPL_THREADSAFE_QUERY_INTERFACE1 the _THREADSAFE is completely unnecessary.
Attachment #378452 - Flags: review?(benjamin) → review+
(In reply to comment #10)
> note that in NS_IMPL_THREADSAFE_QUERY_INTERFACE1 the _THREADSAFE is completely
> unnecessary.
Yeah, but I figured for consistency's sake I'd leave it in.
Whiteboard: [needs review bsmedberg] → [can land]
http://hg.mozilla.org/mozilla-central/rev/3d731633296e
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
You need to log in before you can comment on or make changes to this bug.