Last Comment Bug 1315812 - Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated
: Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated
Status: RESOLVED FIXED
: addon-compat
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla52
Assigned To: Eric Rahm [:erahm]
:
: Nathan Froyd [:froydnj]
Mentors:
Depends on: 1317040
Blocks: nuke-nsSupportsArray
  Show dependency treegraph
 
Reported: 2016-11-07 15:04 PST by Eric Rahm [:erahm]
Modified: 2016-11-14 13:47 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated (5.20 KB, patch)
2016-11-07 15:04 PST, Eric Rahm [:erahm]
nfroyd: review+
Details | Diff | Splinter Review
Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated (12.50 KB, patch)
2016-11-09 17:25 PST, Eric Rahm [:erahm]
nfroyd: review+
Details | Diff | Splinter Review

Description User image Eric Rahm [:erahm] 2016-11-07 15:04:53 PST
This marks the idl classes as deprecated, removes an unnecessary include that
was triggering deprecation warnings and wraps a necessary include in
XPCOMInit.cpp that is used for registering the component in deprecation
disabling pragmas.

MozReview-Commit-ID: BbNU5q8O4Q4
Comment 1 User image Eric Rahm [:erahm] 2016-11-07 15:04:56 PST
Created attachment 8808395 [details] [diff] [review]
Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated
Comment 2 User image Nathan Froyd [:froydnj] 2016-11-08 07:46:47 PST
Comment on attachment 8808395 [details] [diff] [review]
Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated

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

r=me, I guess, though I'd like to understand why we only need the single deprecation location.

::: xpcom/build/XPCOMInit.cpp
@@ +44,5 @@
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> +#elif defined(_MSC_VER)
> +#pragma warning (push)
> +#pragma warning (disiable : 4996)

"disiable"? :)

@@ +51,1 @@
>  #include "nsSupportsArray.h"

How is this the only place that requires the deprecation suppression?  Surely the implementation code somewhere includes the deprecated .h files...
Comment 3 User image Eric Rahm [:erahm] 2016-11-09 17:25:19 PST
Created attachment 8809233 [details] [diff] [review]
Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated

Now with more pragmas
Comment 4 User image Eric Rahm [:erahm] 2016-11-10 13:15:39 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/f424e29dc77ed2790f3ab6e05940a31c4a85316a
Bug 1315812 - Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated. r=froydnj
Comment 5 User image Wes Kocher (:KWierso) 2016-11-11 14:00:34 PST
https://hg.mozilla.org/mozilla-central/rev/f424e29dc77e
Comment 6 User image :aceman 2016-11-12 04:40:18 PST
It could be better for all external users if this went into 53. Forcing changing of interfaces (due to compiler warnings-as-errors) so close before branching of 52 ESR is not nice.
Comment 7 User image Nathan Froyd [:froydnj] 2016-11-12 06:47:06 PST
(In reply to :aceman from comment #6)
> It could be better for all external users if this went into 53. Forcing
> changing of interfaces (due to compiler warnings-as-errors) so close before
> branching of 52 ESR is not nice.

You can apply the workarounds for C++ from this very bug; you don't have to change any interfaces.
Comment 8 User image Jorg K (GMT+1) 2016-11-13 08:59:37 PST
We did ;-)

Nathan, when will M-C remove nsISupportsArray completely? We still use it extensively in Thunderbird, despite our best ongoing efforts to remove it. It's mainly used in the search component.
Comment 9 User image Nathan Froyd [:froydnj] 2016-11-14 13:41:53 PST
(In reply to Jorg K (GMT+1) from comment #8)
> We did ;-)
> 
> Nathan, when will M-C remove nsISupportsArray completely? We still use it
> extensively in Thunderbird, despite our best ongoing efforts to remove it.
> It's mainly used in the search component.

I believe Eric was planning on removing it after a single release advertising its deprecation, so in 53.  Is that correct, Eric?  I suppose we could consider other options, especially since it just got more inconvenient to use from C++...
Comment 10 User image Eric Rahm [:erahm] 2016-11-14 13:47:56 PST
(In reply to Nathan Froyd [:froydnj] from comment #9)
> (In reply to Jorg K (GMT+1) from comment #8)
> > We did ;-)
> > 
> > Nathan, when will M-C remove nsISupportsArray completely? We still use it
> > extensively in Thunderbird, despite our best ongoing efforts to remove it.
> > It's mainly used in the search component.
> 
> I believe Eric was planning on removing it after a single release
> advertising its deprecation, so in 53.  Is that correct, Eric?  I suppose we
> could consider other options, especially since it just got more inconvenient
> to use from C++...

I think kmag and I settled on a full release cycle so that addon devs can put out one release that works with all of ours, so remove when 52 (or is it 53?) hits release. I'd be happy to remove earlier of course!

If we can't remove the dependencies in TB by that time then another option is to move the interface into c-c at that point.

Note You need to log in before you can comment on or make changes to this bug.