The default bug view has changed. See this FAQ.

Move Base64 code in XPConnect away from nsXPConnect

RESOLVED FIXED in mozilla12

Status

()

Core
XPConnect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

Trunk
mozilla12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 584339 [details] [diff] [review]
Patch v1

This code is rather self-contained, and I don't see how static methods on nsXPConnect is a good place for them.
Attachment #584339 - Flags: review?(bobbyholley+bmo)
It seems to me that the only thing that should really live in XPConnect is the jsval translation stuff, and everything else should live in xpcom/string, or maybe some other general utility drawer.

Kyle, what do you think?
Comment on attachment 584339 [details] [diff] [review]
Patch v1

Lets put this stuff in xpcom/io/Base64.[cpp|h]?
Attachment #584339 - Flags: review?(bobbyholley+bmo) → review-
(Assignee)

Comment 3

5 years ago
Created attachment 584462 [details] [diff] [review]
Patch v2

Mmk.
Attachment #584339 - Attachment is obsolete: true
Attachment #584462 - Flags: review?(khuey)
Attachment #584462 - Flags: review?(bobbyholley+bmo)
Comment on attachment 584462 [details] [diff] [review]
Patch v2

As mentioned on IRC, please put the public declarations of the XPConnect-y versions in xpcpublic, and leave the implementations in nsXPConnect.
Attachment #584462 - Flags: review?(bobbyholley+bmo) → review-
(Assignee)

Comment 5

5 years ago
Created attachment 584483 [details] [diff] [review]
Patch v3
Attachment #584462 - Attachment is obsolete: true
Attachment #584462 - Flags: review?(khuey)
Attachment #584483 - Flags: review?(khuey)
Attachment #584483 - Flags: review?(bobbyholley+bmo)
Comment on attachment 584483 [details] [diff] [review]
Patch v3

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

I assumed you just copied and pasted the code and didn't look too closely.

::: js/xpconnect/src/nsXPConnect.cpp
@@ +2807,5 @@
>  {
> +    MOZ_ASSERT(cx);
> +    MOZ_ASSERT(out);
> +
> +    JS::Value root = val;

Is this necessary? val is already on the stack ...

@@ +2833,5 @@
>  {
> +    MOZ_ASSERT(cx);
> +    MOZ_ASSERT(out);
> +
> +    JS::Value root = val;

Same question.
Attachment #584483 - Flags: review?(khuey) → review+
Comment on attachment 584483 [details] [diff] [review]
Patch v3

r=bholley on the xpconnect bits.
Attachment #584483 - Flags: review?(bobbyholley+bmo) → review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/d144d8a5af9e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.