713550 - Move Base64 code in XPConnect away from nsXPConnect

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

This code is rather self-contained, and I don't see how static methods on nsXPConnect is a good place for them.

Comment 1

[Bobby Holley (:bholley)]

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 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 584339 [details] [diff] [review]
Patch v1

Lets put this stuff in xpcom/io/Base64.[cpp|h]?

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v2

Mmk.

Comment 4

[Bobby Holley (:bholley)]

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.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v3

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 7

[Bobby Holley (:bholley)]

Comment on attachment 584483 [details] [diff] [review]
Patch v3

r=bholley on the xpconnect bits.

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/d144d8a5af9e