Status

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: jorgk, Assigned: jorgk)

Tracking

Trunk
Thunderbird 60.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

Last year
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
Assignee

Comment 1

Last year
Posted patch 1421084-nsNSSShutDown.patch (obsolete) — Splinter Review
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.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8947664 - Flags: review?(ttaubert)
Assignee

Comment 2

Last year
Killed one more, sorry about the noise.
Attachment #8947664 - Attachment is obsolete: true
Attachment #8947664 - Flags: review?(ttaubert)
Attachment #8947666 - Flags: review?(ttaubert)

Comment 3

Last year
Pushed by mozilla@jorgk.com:
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
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Assignee

Comment 4

Last year
(In reply to Jorg K (GMT+1) from comment #1)
> I will add some questions in the next comment.
I answered my own question ;-)
Target Milestone: --- → Thunderbird 60.0
Version: 52 → Trunk
The bug is fixed but there's a pending review. Do you want me to still review?
Flags: needinfo?(jorgk)
Assignee

Comment 6

Last year
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.
Flags: needinfo?(jorgk)
(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.
Attachment #8947666 - Flags: review?(ttaubert) → review+
Assignee

Comment 8

Last year
(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!
You need to log in before you can comment on or make changes to this bug.