1490265 - Port bug 1468222: Consolidate nsISSLStatus into nsITransportSecurityInfo (cannot find type `nsISSLStatus` in this scope)

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Jorg K (CEST = GMT+2)]

M-C last good: fea371cafd2c73be689184ca6670e33c3f
M-C first bad: 423bdf7a802b0d302244492b423609187d

Range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fea371cafd2c73be689184ca6670e33c3f&tochange=423bdf7a802b0d302244492b423609187d

0:58.41 dom/media/platforms/ffmpeg/ffvpx
0:59.74 dom/media/systemservices
1:00.05 cannot find type `nsISSLStatus` in this scope
1:00.05 c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/xpcrs/rt/imIAccount.rs:773:101
1:00.05     pub GetSslStatus: unsafe extern "system" fn (this: *const prplIAccount, aSslStatus: *mut *const nsISSLStatus) -> nsresult,
1:00.05  cannot find type `nsISSLStatus` in this scope
1:00.05 c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/xpcrs/rt/imIAccount.rs:1053:63
1:00.05     pub unsafe fn GetSslStatus(&self, aSslStatus: *mut *const nsISSLStatus) -> nsresult {
1:00.05                                       ^^^^^^^^^^^^not found in this scope
1:02.94 dom/media/wave

Comment 1

[Jorg K (CEST = GMT+2)]

That comes from chat's imIAccount.idl which is somehow processed into Rust and there's something wrong.

We have:
  interface nsISSLStatus;
and
  readonly attribute nsISSLStatus sslStatus;

I see it now:
5cfda4227c6a Dipen Patel - Bug 1468222 Consolidate nsISSLStatus info nsITransportSecurityInfo r=Gijs,snorp,jcj,mcmanus,sfraser,keeler,baku,ato

Comment 2

[Jorg K (CEST = GMT+2)]

Looks like we need to look at the call sites providing SSL status:
https://searchfox.org/comm-central/search?q=sslStatus&case=false&regexp=false&path=

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1490265-remove-nsISSLStatus.patch

This lets us compile again.

Comment 4

[Jorg K (CEST = GMT+2)]

Magnus, would you like to be the doctor here? The paramedics morning shift is over and the patient is no longer bleeding.

Comment 5

[Jorg K (CEST = GMT+2)]

Make that genitive: The paramedic's ...

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/11615766cf8a
Port bug 1468222: remove use of nsISSLStatus. rs=bustage-fix

Comment 7

[Jorg K (CEST = GMT+2)]

Hi Dipen, anything in particular we need to look for when porting this? Just removing the interface (see comment #6) hasn't caused any test failures, but of course not everything is covered by tests.

Comment 8

[Magnus Melin [:mkmelin]]

Can take a look in the evening, if nobody else wants to do it.

Comment 9

[Jorg K (CEST = GMT+2)]

OK, DXR is back :-)
https://dxr.mozilla.org/comm-central/search?q=sslStatus+path%3Acomm%2F&redirect=false

https://dxr.mozilla.org/comm-central/search?q=nsISSLStatus+path%3Acomm%2F&redirect=false
Mostly comments and the stuff I've already removed in https://hg.mozilla.org/comm-central/rev/11615766cf8a, but
comm/mail/components/accountcreation/content/guessConfig.js
859 let cert = status.QueryInterface(Ci.nsISSLStatus).serverCert;
surely won't work now.

Replacement is here:
https://hg.mozilla.org/mozilla-central/rev/5cfda4227c6a#l10.12

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: 1490265-nsISSLStatus.patch

I've made a start gleaning bits from
https://hg.mozilla.org/mozilla-central/rev/5cfda4227c6a

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: 1490265-nsISSLStatus.patch (v2)

That should cover everything except chat/modules/jsProtoHelper.jsm.

Your turn, Magnus?

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: 1490265-nsISSLStatus.patch (v3)

Ah well, this should do it.

Comment 13

[Jorg K (CEST = GMT+2)]

Comment on attachment 9008156 [details] [diff] [review]
1490265-nsISSLStatus.patch (v3)

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

::: chat/modules/jsProtoHelper.jsm
@@ +59,5 @@
>  
>      if (aIsSslError)
>        return Ci.prplIAccount.ERROR_ENCRYPTION_ERROR;
>  
> +    let secInfo = this._secInfo = aSocket.secInfo;

Actually, I didn't see an example of this in the M-C patch.

Comment 14

[Magnus Melin [:mkmelin]]

Comment on attachment 9008156 [details] [diff] [review]
1490265-nsISSLStatus.patch (v3)

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

Looks ok to me. It would be a bit complicated to test though. r=mkmelin

::: chat/components/public/imIAccount.idl
@@ +149,5 @@
>    /* When a certificate error occurs, the host/port that caused a
>     * SSL/certificate error when connecting to it. This is only valid when
>     * connectionErrorReason is one of ERROR_CERT_*. */
>    readonly attribute AUTF8String connectionTarget;
> +  /* When a certificate error occurs, the nsITransportSecurityInfo error of the socket. This

comment style is wrong in this file

/** is documentation style comments. /* is just start of a multiline comment

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/37bdf33fc10c
Port bug 1468222: Replace nsISSLStatus with nsITransportSecurityInfo. r=mkmelin

Comment 16

[Dipen Patel [:dipen]]

Comment on attachment 9008156 [details] [diff] [review]
1490265-nsISSLStatus.patch (v3)

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

A little late but looks fine.

Comment 17

[Jorg K (CEST = GMT+2)]

Better late than never, as they say ;-) - Thanks!