1435093 - Port bug 1421084 to mailnews.

Status: RESOLVED FIXED

(MailNews Core :: Security, enhancement)

Description

[Jorg K (CEST = GMT+2)]

From bug 1421084:
part 1/4 - remove now-unnecessary nsNSSShutDownPreventionLock r=mt,ttaubert
https://hg.mozilla.org/integration/autoland/rev/ecb9941ee034
part 2/4 - remove nsNSSShutDownObject::isAlreadyShutDown() r=mt,ttaubert
https://hg.mozilla.org/integration/autoland/rev/0d42218045d9
part 3/4 - remove nsNSSShutDownObject::shutdown and virtualDestroyNSSReference r=mt,ttaubert
https://hg.mozilla.org/integration/autoland/rev/e21956fd51a3
part 4/4 - remove nsNSSShutDown.h and (hopefully) all references to it r=mt,ttaubert

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1421084-nsNSSShutDown.patch

Tim, I followed your changes in bug 1421084 as well as I could, but knowing nothing about this, I'm not sure I got it right.

Since we're busted, I have to land this now, but please look over the changes. I will add some questions in the next comment.

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1421084-nsNSSShutDown.patch (v1b)

Killed one more, sorry about the noise.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/113fa182b10b
Port bug 1421084 to mailnews: remove nsNSSShutDownPreventionLock, isAlreadyShutDown, shutdown, virtualDestroyNSSReference, nsNSSShutDown.h from mailnews. rs=bustage-fix

Comment 4

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #1)
> I will add some questions in the next comment.
I answered my own question ;-)

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

The bug is fixed but there's a pending review. Do you want me to still review?

Comment 6

[Jorg K (CEST = GMT+2)]

Yes please, as per comment #1. I'm always in the situation having to react to bustage, so in some cases there needs to be a "post landing review" and a follow-up in case there is an issue. I removed everything the compiler complained about which is basically following what you've done in the four changesets quoted in comment #0. I think a quick glance of the the changes, maybe even via https://hg.mozilla.org/comm-central/rev/113fa182b10b would ensure that I got it right.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Jorg K (GMT+1) from comment #6)
> I think a quick glance of the the changes,
> maybe even via https://hg.mozilla.org/comm-central/rev/113fa182b10b would
> ensure that I got it right.

The only things I spotted were:

* nsCMSMessage::destructorSafeDestroyNSSReference() could probably be inlined in the destructor.
* Same for nsZeroTerminatedCertArray::destructorSafeDestroyNSSReference()
* And nsCMSDecoder::destructorSafeDestroyNSSReference()
* And nsCMSEncoder::destructorSafeDestroyNSSReference()

Up to you. These aren't correctness fixes, of course. Everything else looks good.

Comment 8

[Jorg K (CEST = GMT+2)]

(In reply to Tim Taubert [:ttaubert] from comment #7)
> * nsCMSMessage::destructorSafeDestroyNSSReference() could probably be
> inlined in the destructor.
That's for the quick review. Yes, I saw this in your changes, but wanted to keep our changes to straight deletions to avoid error. I also noticed that you moved empty destructors to the .h file in at least one case.

I think I'll leave it "as is" now. Thanks again!