Benoit Jacob [:bjacob] (mostly away) Assignee
	Description • 10 years ago

This is a follow-up of bug 1028588, which added such detection to much of nsISupportsImpl.h but left out _INHERITED macros.

	alexander :surkov (:asurkov)
	Comment 10 • 10 years ago

Comment on attachment 8451846 [details] [diff] [review]
fix accessible

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

rs=me

::: accessible/xul/XULListboxAccessible.cpp
@@ +742,5 @@
>  }
>  
> +XULListCellAccessible::~XULListCellAccessible()
> +{
> +}

any benefits of moving it into cpp file?

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 27 • 10 years ago

Comment on attachment 8451849 [details] [diff] [review]
fix content/html

r+ except struct HasDangerousPublicDestructor<dom::HTMLFormElement> 
needs an explanation or a fix.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use)
	Comment 28 • 10 years ago

Comment on attachment 8451848 [details] [diff] [review]
fix content/base

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

::: content/base/src/DOMQuad.cpp
@@ +137,5 @@
>      *aY2 = y2;
>    }
>  
>  protected:
> +  ~QuadBounds() {}

virtual please

::: content/base/src/nsObjectLoadingContent.cpp
@@ +390,5 @@
>    // nsITimerCallback
>    NS_IMETHOD Notify(nsITimer *timer);
>  
> +protected:
> +  ~nsStopPluginRunnable() {}

virtual please

::: content/base/src/nsXMLHttpRequest.h
@@ +172,5 @@
>      return mListenerManager && mListenerManager->HasListeners();
>    }
> +
> +private:
> +  ~nsXMLHttpRequestUpload() {}

virtual please

	Benoit Jacob [:bjacob] (mostly away) Assignee
	Comment 29 • 10 years ago

(In reply to Olli Pettay [:smaug] from comment #27)
> Comment on attachment 8451849 [details] [diff] [review]
> fix content/html
> 
> r+ except struct HasDangerousPublicDestructor<dom::HTMLFormElement> 
> needs an explanation or a fix.

So if I make that destructor protected, I get:

 0:09.45 /home/bjacob/hack/mozilla-central/content/html/content/src/HTMLFormElement.cpp:75:12: error: calling a protected destructor of class 'mozilla::dom::HTMLFormElement'
 0:09.46     delete it;
 0:09.46            ^
 
The code in question is

http://dxr.mozilla.org/mozilla-central/source/content/html/content/src/HTMLFormElement.cpp#75

It looks very easy to fix. I'll file the followup bug as I land.

	Benoit Jacob [:bjacob] (mostly away) Assignee
	Comment 30 • 10 years ago

(In reply to alexander :surkov from comment #10)
> Comment on attachment 8451846 [details] [diff] [review]
> fix accessible
> 
> Review of attachment 8451846 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> rs=me
> 
> ::: accessible/xul/XULListboxAccessible.cpp
> @@ +742,5 @@
> >  }
> >  
> > +XULListCellAccessible::~XULListCellAccessible()
> > +{
> > +}
> 
> any benefits of moving it into cpp file?

None. That was just my default action to be able to process all the failures quickly. Indeed, that's the safe choice as it doesn't rely on all the definitions to be #included in the header file. But I tried, and in the present case, it works, even in non-unified build. So I'll do that.

	Benoit Jacob [:bjacob] (mostly away) Assignee
	Comment 31 • 10 years ago

Landed everything:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=558231b7c91c

Green try push (aside from Git server error):
https://tbpl.mozilla.org/?tree=Try&rev=f53b52969d45

	Benoit Jacob [:bjacob] (mostly away) Assignee
	Comment 32 • 10 years ago

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #28)
> virtual please

Done (x3).

	Benoit Jacob [:bjacob] (mostly away) Assignee
	Comment 33 • 10 years ago

Follow-up bugs to fix whitelisted dangerous destructors have been file; there were only 3. The tracking bug is bug 1028132.

	Carsten Book [:Tomcat]
	Comment 34 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/959a72081da6
https://hg.mozilla.org/mozilla-central/rev/d44b04df56ae
https://hg.mozilla.org/mozilla-central/rev/f25ec3d47ca0
https://hg.mozilla.org/mozilla-central/rev/67b022015900
https://hg.mozilla.org/mozilla-central/rev/6af74fb23859
https://hg.mozilla.org/mozilla-central/rev/fcc1ebdf82c0
https://hg.mozilla.org/mozilla-central/rev/3f0f389821f7
https://hg.mozilla.org/mozilla-central/rev/821dd7192068
https://hg.mozilla.org/mozilla-central/rev/18dd3958f2f6
https://hg.mozilla.org/mozilla-central/rev/15ea98d307c5
https://hg.mozilla.org/mozilla-central/rev/e5f8bd650ef1
https://hg.mozilla.org/mozilla-central/rev/ad2e6df50240
https://hg.mozilla.org/mozilla-central/rev/cddfa382a2ba
https://hg.mozilla.org/mozilla-central/rev/f6fd4bafabdf
https://hg.mozilla.org/mozilla-central/rev/53bc1bd562ac
https://hg.mozilla.org/mozilla-central/rev/6e71cf4db1ad
https://hg.mozilla.org/mozilla-central/rev/dcd4f09949ce
https://hg.mozilla.org/mozilla-central/rev/74685fc519f4
https://hg.mozilla.org/mozilla-central/rev/bb3aaa6cdaa7
https://hg.mozilla.org/mozilla-central/rev/d070ae6e63cb
https://hg.mozilla.org/mozilla-central/rev/5821b55a574c
https://hg.mozilla.org/mozilla-central/rev/2f5b2fbae4fb
https://hg.mozilla.org/mozilla-central/rev/cd28b3d3a4bd
https://hg.mozilla.org/mozilla-central/rev/683e40882500
https://hg.mozilla.org/mozilla-central/rev/558231b7c91c

	Michael Wu [:mwu]
	Comment 35 • 10 years ago

Accidentally used this bug number in https://hg.mozilla.org/integration/b2g-inbound/rev/86702af059f4 . The intended bug number was bug 1036394.

	Nigel Babu [:nigelb]
	Comment 36 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/86702af059f4