Make VolatileBuffer not use MOZ_DECLARE_REFCOUNTED_TYPENAME

RESOLVED FIXED in mozilla33

Status

()

Core
MFBT
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks: 1 bug)

unspecified
mozilla33
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
The issue here is that mozalloc is not part of libxul, so
MOZ_DECLARE_REFCOUNTED_TYPENAME expands to nothing there.  MSVC seems
to just happily inline these function calls even though they are on a
dllimport class.
(Assignee)

Comment 1

4 years ago
Created attachment 8441631 [details] [diff] [review]
Make VolatileBuffer not use MOZ_DECLARE_REFCOUNTED_TYPENAME
(Assignee)

Updated

4 years ago
Assignee: nobody → ehsan
Blocks: 752004
(Assignee)

Updated

4 years ago
Attachment #8441631 - Flags: review?(mh+mozilla)
Comment on attachment 8441631 [details] [diff] [review]
Make VolatileBuffer not use MOZ_DECLARE_REFCOUNTED_TYPENAME

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

I'd rather change MOZ_DECLARE_REFCOUNTED_TYPENAME to expand independently of MOZILLA_INTERNAL_API.
Attachment #8441631 - Flags: review?(mh+mozilla) → review-
(Assignee)

Comment 3

4 years ago
Created attachment 8442053 [details] [diff] [review]
Expand MOZ_DECLARE_REFCOUNTED_TYPENAME unconditioanlly in order to make it available outside of libxul as well; r=glandium
(Assignee)

Updated

4 years ago
Attachment #8442053 - Flags: review?(mh+mozilla)
Comment on attachment 8442053 [details] [diff] [review]
Expand MOZ_DECLARE_REFCOUNTED_TYPENAME unconditioanlly in order to make it available outside of libxul as well; r=glandium

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

::: mfbt/RefPtr.h
@@ +166,5 @@
> +// two small inline functions which will hopefully get eliminated by the linker
> +// in non-leak-checking builds.
> +#define MOZ_DECLARE_REFCOUNTED_TYPENAME(T) \
> +  const char* typeName() const { return #T; } \
> +  size_t typeSize() const { return sizeof(*this); }

I think you can reasonably put that under #if defined(DEBUG) || defined(FORCE_BUILD_REFCNT_LOGGING)

That said, I'm not a MFBT peer.
Attachment #8442053 - Flags: review?(mh+mozilla) → review?(jwalden+bmo)
(Assignee)

Comment 5

4 years ago
(In reply to Mike Hommey [:glandium] from comment #4)
> Comment on attachment 8442053 [details] [diff] [review]
> Expand MOZ_DECLARE_REFCOUNTED_TYPENAME unconditioanlly in order to make it
> available outside of libxul as well; r=glandium
> 
> Review of attachment 8442053 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mfbt/RefPtr.h
> @@ +166,5 @@
> > +// two small inline functions which will hopefully get eliminated by the linker
> > +// in non-leak-checking builds.
> > +#define MOZ_DECLARE_REFCOUNTED_TYPENAME(T) \
> > +  const char* typeName() const { return #T; } \
> > +  size_t typeSize() const { return sizeof(*this); }
> 
> I think you can reasonably put that under #if defined(DEBUG) ||
> defined(FORCE_BUILD_REFCNT_LOGGING)

Sure, I could do that.  Shouldn't make a difference either way...
Component: General → MFBT

Comment 6

4 years ago
Comment on attachment 8442053 [details] [diff] [review]
Expand MOZ_DECLARE_REFCOUNTED_TYPENAME unconditioanlly in order to make it available outside of libxul as well; r=glandium

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

Actually you are, per very ancient discussion from ~2012ish (?) that only made its way into the module page list just recently.  :-)
Attachment #8442053 - Flags: review?(jwalden+bmo) → review+

Comment 8

4 years ago
https://hg.mozilla.org/mozilla-central/rev/8eeb287c59fd
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.