29333 - illegal use of nsString and performace problem in nsString::ToNewUTF8String

Status: VERIFIED FIXED

(Core :: XPCOM, defect, P3)

Description

[Frank Tang]

I found that I make a mistake when I implmeent the nsString::ToNewUTF8String
I copy some code from nsString::ToNewCString. Howerver, this slow down the
implementation. Also, it cause the assertion filed in 28424.
Here is the patch which fix it. It is simply, remove the unnecessary copy of
data in the nsCString constructor.

retrieving revision 1.69
diff -c -r1.69 nsString2.cpp
*** nsString2.cpp       2000/02/24 08:19:57     1.69
--- nsString2.cpp       2000/02/26 01:31:21
***************
*** 682,688 ****
   * http://www.cis.ohio-state.edu/htbin/rfc/rfc2279.html
   */
  char* nsString::ToNewUTF8String() const {
!   nsCString temp(*this);
    temp.SetCapacity(8); //ensure that we get an allocated buffer instead of the
 common empty one.

    // Caculate how many bytes we need
--- 682,688 ----
   * http://www.cis.ohio-state.edu/htbin/rfc/rfc2279.html
   */
  char* nsString::ToNewUTF8String() const {
!   nsCString temp("");
    temp.SetCapacity(8); //ensure that we get an allocated buffer instead of the
 common empty one.

    // Caculate how many bytes we need

Rickg- Please review.
This fix will increase our performance when we convert nsString to UTF8

Comment 1

[bobj]

Safe 1 line performance fix.
Nominating for beta1.

Comment 2

[leger]

Putting on PDT+ radar for beta1.

Comment 3

[bobj]

     Subject: Re: code review, please
       Date: Mon, 28 Feb 2000 12:44:36 -0800
       From: [e-mail] (Rick Gessner)
Organization: Netscape Communications
         To: [e-mail]
        CC: [e-mail]
 References: <[e-mail]>

Frank --

You're on the right track, but you should call SetCapacity() with a larger
buffer than 8 bytes.
Rick

Comment 4

[Frank Tang]

fixed and check in. rickg- I do SetCapacity later.

Comment 5

[leger]

ftang, was this in the Feb29 build ok?  Not sure to how to verify.  Can you 
verify?

Comment 6

[leger]

Updating QA Contact to teruko to check on how to verify.  Or ftang need to mark 
this verified if code ok with latest build.

Comment 7

[Frank Tang]

verified