Closed Bug 1255438 Opened 4 years ago Closed 4 years ago

create nsI{Mutable,}Array directly in security/manager/ssl/

Categories

(Core :: Security: PSM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(1 file)

No description provided.
The less involvement of the component manager, the better.
Attachment #8729018 - Flags: review?(dkeeler)
Comment on attachment 8729018 [details] [diff] [review]
create nsI{Mutable,}Array directly in security/manager/ssl/

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

LGTM with comments addressed.

::: security/manager/pki/nsNSSDialogs.cpp
@@ +32,5 @@
>  #include "nsIX509CertValidity.h"
>  
>  #include "nsEmbedCID.h"
>  #include "nsIPromptService.h"
> +#include "nsArray.h"

If you would, please sort all of the #includes (we've been updating them to be in line with the style guidelines as we add/remove #includes in security/manager).

::: security/manager/ssl/nsCertTree.cpp
@@ +14,5 @@
>  #include "nsReadableUtils.h"
>  #include "nsUnicharUtils.h"
>  #include "nsNSSCertificate.h"
>  #include "nsNSSCertHelper.h"
> +#include "nsArray.h"

Same here.

::: security/manager/ssl/nsNSSASN1Object.cpp
@@ +4,5 @@
>  #include "nsNSSASN1Object.h"
>  #include "nsIComponentManager.h"
>  #include "secasn1.h"
>  #include "nsReadableUtils.h"
> +#include "nsArray.h"

... etc.

::: security/manager/ssl/nsNSSCertificate.cpp
@@ +892,1 @@
>    if (NS_FAILED(rv)) {

Looks like this should be:

if (!array) {
  return NS_ERROR_FAILURE;
}

(also, looks like the done label can be removed)

::: security/manager/ssl/nsNSSCertificateDB.cpp
@@ +445,5 @@
>    if (!certCollection) {
>      PORT_FreeArena(arena, false);
>      return NS_ERROR_FAILURE;
>    }
> +  nsCOMPtr<nsIMutableArray> array = nsArrayBase::Create();

Note these changes may conflict with the patch in bug 1250258.
Attachment #8729018 - Flags: review?(dkeeler) → review+
Comment on attachment 8729018 [details] [diff] [review]
create nsI{Mutable,}Array directly in security/manager/ssl/

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

FWIW, looks like there some unconverted lines such as https://hg.mozilla.org/mozilla-central/file/tip/security/manager/ssl/nsPK11TokenDB.cpp#l454.
Not sure if they were just missed, or were omitted on purpose.
(In reply to David Keeler [:keeler] (use needinfo?) from comment #2)
> Note these changes may conflict with the patch in bug 1250258.

Yup.

froydnj: Go ahead and land the changes here first if you like, the Bug 1250258 changes can wait.
https://hg.mozilla.org/mozilla-central/rev/b125b74ee12b
https://hg.mozilla.org/mozilla-central/rev/6dbd1b04414d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.