894448 - Remove nativeOwnership = 'nsisupports'

Status: RESOLVED FIXED

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

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: v1

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.

Comment 2

[Boris Zbarsky [: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.

Comment 3

[Boris Zbarsky [:bzbarsky]]

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.  :(

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(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.

Comment 5

[Boris Zbarsky [:bzbarsky]]

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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

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.

Comment 7

[Bobby Holley (:bholley)]

(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?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Move IsChromeOrXBL to xpcpublic.h.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Try run with all those bits: https://tbpl.mozilla.org/?tree=Try&rev=844d3b3220a2

Comment 10

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb2a64270e6a
https://hg.mozilla.org/integration/mozilla-inbound/rev/9308a970daee

Comment 11

[Mike Hommey [:glandium]]

This has seriously regressed the number of static initializers, from 232 to 530.

Comment 12

[Mike Hommey [:glandium]]

(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

Comment 13

[Boris Zbarsky [:bzbarsky]]

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.  :(

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bb2a64270e6a
https://hg.mozilla.org/mozilla-central/rev/9308a970daee

Comment 15

[Bobby Holley (:bholley)]

I had to backport part 1 to esr24 to backport bug 912322.

remote:   https://hg.mozilla.org/releases/mozilla-esr24/rev/db03ff59d7ee