Closed Bug 894448 Opened 7 years ago Closed 7 years ago

Remove nativeOwnership = 'nsisupports'

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: peterv, Assigned: peterv)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed, Whiteboard: [qa-])

Attachments

(2 files)

No description provided.
Attached patch v1Splinter Review
More template tricks :-/. This makes all refcounted objects (nsISupports-based or not) us nativeOwnership = 'refcounted'. Note how a number of objects were actually mismarked (for example AudioContext), and so we weren't supporting everything correctly for them.
Attachment #777649 - Flags: review?
Attachment #777649 - Flags: review? → review?(bzbarsky)
Comment on attachment 777649 [details] [diff] [review]
v1

>+template <class T, bool isISupports=IsISupports<T>::Value>

We should get rid of isISupports and just use IsBaseOf here and in other places where we use isISupports, no?

r=me with that.
Attachment #777649 - Flags: review?(bzbarsky) → review+
I pushed a version of the patch with the review comments addressed to try: https://tbpl.mozilla.org/?tree=Try&rev=152c70dae89d

I need to figure out what's going on with those test timeouts.  :(
(In reply to Boris Zbarsky (:bz) from comment #3)
> I pushed a version of the patch with the review comments addressed to try:
> https://tbpl.mozilla.org/?tree=Try&rev=152c70dae89d
> 
> I need to figure out what's going on with those test timeouts.  :(

No need; those are normal for OSX m-bc runs lately.
So the bc timeout there is a known randomorange.  See bug 895426.  It just becomes permaorange with this patch...  and is apparently bad enough even without that the test is disabled.  Going to rebase and try this again.
And now someone rejiggered includes somewhere so using nsINode::IsChromeOrXBL in BindingUtils.h no longer compiles...  We should move that function somewhere else; the question is where would be a reasonable place that would not pull in too many headers.
Flags: needinfo?(bobbyholley+bmo)
(In reply to Boris Zbarsky (:bz) from comment #6)
> And now someone rejiggered includes somewhere so using
> nsINode::IsChromeOrXBL in BindingUtils.h no longer compiles...  We should
> move that function somewhere else; the question is where would be a
> reasonable place that would not pull in too many headers.

What about xpcpublic.h?
Flags: needinfo?(bobbyholley+bmo)
Assignee: peterv → bzbarsky
Attachment #781092 - Flags: review?(bobbyholley+bmo) → review+
This has seriously regressed the number of static initializers, from 232 to 530.
(In reply to Mike Hommey [:glandium] from comment #11)
> This has seriously regressed the number of static initializers, from 232 to
> 530.

This apparently comes from the fact that "participant" is not a nullptr in many cases anymore because of
https://hg.mozilla.org/integration/mozilla-inbound/rev/9308a970daee#l3.12
It's still a nullptr, but apparently the compiler is failing to statically determine that and just output a 0; it outputs a static initializer that writes 0.  :(
Depends on: 899368
I had to backport part 1 to esr24 to backport bug 912322.

remote:   https://hg.mozilla.org/releases/mozilla-esr24/rev/db03ff59d7ee
Whiteboard: [qa-]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.