Closed
Bug 1004564
Opened 10 years ago
Closed 10 years ago
Move AtomicRefCounted to mozilla::external and outlaw it in Gecko code
Categories
(Core :: MFBT, defect)
Core
MFBT
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file)
4.35 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8415971 -
Flags: review?(nfroyd)
Comment 2•10 years ago
|
||
Comment on attachment 8415971 [details] [diff] [review] Move AtomicRefCounted to mozilla::external and outlaw it in Gecko code Review of attachment 8415971 [details] [diff] [review]: ----------------------------------------------------------------- Um, sure, I guess. WDYT about making mozilla::external something scarier, like mozilla::do_not_use_in_gecko, or mozilla::external_use_only, so uses stand out a little more? "external" sounds very generic to me, I don't know that I'd look it up if I was doing a review.
Attachment #8415971 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 3•10 years ago
|
||
We discussed the naming a bit on IRC, it sounds like we don't have much better ideas than "external".
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cb70de5fd6a
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4cb70de5fd6a
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment 6•9 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #2) > Comment on attachment 8415971 [details] [diff] [review] > Move AtomicRefCounted to mozilla::external and outlaw it in Gecko code > > Review of attachment 8415971 [details] [diff] [review]: > ----------------------------------------------------------------- > > Um, sure, I guess. WDYT about making mozilla::external something scarier, > like mozilla::do_not_use_in_gecko, or mozilla::external_use_only, so uses > stand out a little more? "external" sounds very generic to me, I don't know > that I'd look it up if I was doing a review. Not to mention there's places where we don't want dependencies on nsISupportsImpl. So either we need an alternative that we can use that supports whatever you want in MFBT, or we're using this inside Moz2D.
Flags: needinfo?(nfroyd)
Comment 7•9 years ago
|
||
(In reply to Bas Schouten (:bas.schouten) from comment #6) > Not to mention there's places where we don't want dependencies on > nsISupportsImpl. So either we need an alternative that we can use that > supports whatever you want in MFBT, or we're using this inside Moz2D. I don't really understand the question here. This code has been in the tree for 18+ months, and gfx/ is obviously using it...even if nobody but graphics folks really understand why Moz2D insists on this "can't depend on xpcom" mantra.
Flags: needinfo?(nfroyd)
Comment 8•9 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #7) > (In reply to Bas Schouten (:bas.schouten) from comment #6) > > Not to mention there's places where we don't want dependencies on > > nsISupportsImpl. So either we need an alternative that we can use that > > supports whatever you want in MFBT, or we're using this inside Moz2D. > > I don't really understand the question here. This code has been in the tree > for 18+ months, and gfx/ is obviously using it...even if nobody but graphics > folks really understand why Moz2D insists on this "can't depend on xpcom" > mantra. As long as we're good on that so am I, I only was pointed at the comment yesterday.
You need to log in
before you can comment on or make changes to this bug.
Description
•