Closed Bug 1283085 Opened 3 years ago Closed 3 years ago

Add new ALPN enum to switch to avoid Gecko bustage with NSS Uplift

Categories

(Core :: WebRTC: Networking, defect, P1)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: ekr, Assigned: ekr)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch x.patch (obsolete) — Splinter Review
No description provided.
Attachment #8766294 - Flags: review?(adam)
Tim, this needs to land at the same time as NSS Uplift to avoid bustage.
Flags: needinfo?(ttaubert)
Rank: 10
Priority: -- → P1
Comment on attachment 8766294 [details] [diff] [review]
x.patch

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

r+ with nits

::: media/mtransport/transportlayerdtls.cpp
@@ +928,5 @@
>        MOZ_MTLOG(ML_ERROR, LAYER_INFO << "error in ALPN selection callback");
>        return false;
> +
> +    case SSL_NEXT_PROTO_EARLY_VALUE:
> +      // 0-RTT not yet wired up.

Add a bug number?

@@ +929,5 @@
>        return false;
> +
> +    case SSL_NEXT_PROTO_EARLY_VALUE:
> +      // 0-RTT not yet wired up.
> +      MOZ_MTLOG(ML_ERROR, LAYER_INFO << "error in ALPN selection callback");

This is going to be difficult to distinguish in logs from the previous case -- suggest some variation on the message.
Attachment #8766294 - Flags: review?(adam) → review+
Actually, we should just have a can't happen assert, because we don't expect to turn on 0-RTT for WebRTC any time soon
Attached patch x.patch (obsolete) — Splinter Review
Attached patch x.patch (obsolete) — Splinter Review
Attachment #8766294 - Attachment is obsolete: true
Attachment #8766437 - Attachment is obsolete: true
Attached patch x.patchSplinter Review
Attachment #8766438 - Attachment is obsolete: true
Comment on attachment 8766440 [details] [diff] [review]
x.patch

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

Adam, let's try again
Attachment #8766440 - Flags: review?(adam)
Comment on attachment 8766440 [details] [diff] [review]
x.patch

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

LGTM
Attachment #8766440 - Flags: review?(adam) → review+
Assignee: nobody → ekr
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(ttaubert)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.