Remove nsICertTree.isHostPortOverride()

RESOLVED FIXED in Firefox 48

Status

()

Core
Security: PSM
--
minor
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Cykesiopka, Assigned: Cykesiopka)

Tracking

({addon-compat})

unspecified
mozilla48
addon-compat
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
The last use of the method was removed in the changes for Bug 825583.

As such, the IDL definition and the implementation at https://hg.mozilla.org/mozilla-central/annotate/5e5b76d86634/security/manager/ssl/nsCertTree.cpp#l863 can now be removed.
Whiteboard: [psm-cleanup]
(Assignee)

Comment 1

2 years ago
I forgot that there is a possibility that some add-on is using isHostPortOverride() for whatever reason, but I don't have access to the addons MXR so I can't say for for sure that it is unused.
Still, the method is IMO of low value and should be removed regardless.

This pseudocode might work as a workaround:
> function isHostPortOverride(index) {
>   let cert = nsICertTree.getCert(index);
>   return nsICertOverrideService.isCertUsedForOverrides(cert, ...) > 0;
> }
Assignee: nobody → cykesiopka.bmo
Status: NEW → ASSIGNED
Whiteboard: [psm-cleanup] → [psm-assigned]
(Assignee)

Comment 2

2 years ago
Created attachment 8738547 [details]
MozReview Request: Bug 1252384 - Remove nsICertTree.isHostPortOverride().

It is unused since the changes in Bug 825583 landed.

Review commit: https://reviewboard.mozilla.org/r/44573/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44573/
Attachment #8738547 - Flags: review?(dkeeler)
Attachment #8738547 - Flags: review?(dkeeler) → review+
Comment on attachment 8738547 [details]
MozReview Request: Bug 1252384 - Remove nsICertTree.isHostPortOverride().

https://reviewboard.mozilla.org/r/44573/#review41445

Looks like only 2 addons use it: the Firefox OS simulator (which I think only does so because it packages up everything in each desktop platform's build for some reason) and an addon called "Key Manager" for which the most recent version of Firefox that works is 24, so I think we're safe in removing this.
cc mkaply so he's aware we're removing this.
(Assignee)

Comment 5

2 years ago
Thanks for the review!

(In reply to David Keeler [:keeler] (use needinfo?) from comment #3)
> Looks like only 2 addons use it: the Firefox OS simulator (which I think
> only does so because it packages up everything in each desktop platform's
> build for some reason) and an addon called "Key Manager" for which the most
> recent version of Firefox that works is 24, so I think we're safe in
> removing this.
Cool, thanks for checking.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c630efea2a3f4f2d6052e0d10590440b5b62d1cb
Keywords: checkin-needed

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0ed5e2650d6a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(Assignee)

Updated

2 years ago
Keywords: addon-compat
Whiteboard: [psm-assigned]
You need to log in before you can comment on or make changes to this bug.