Last Comment Bug 656878 - nsStringArraySH::GetProperty copies string to js, but it could probably share it
: nsStringArraySH::GetProperty copies string to js, but it could probably share it
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 All
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-13 04:16 PDT by Olli Pettay [:smaug]
Modified: 2011-05-13 10:02 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (478 bytes, text/html)
2011-05-13 04:17 PDT, Olli Pettay [:smaug]
no flags Details
wip (2.13 KB, patch)
2011-05-13 06:03 PDT, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
without QI change (1.27 KB, patch)
2011-05-13 07:05 PDT, Olli Pettay [:smaug]
bzbarsky: review+
Details | Diff | Splinter Review

Description Olli Pettay [:smaug] 2011-05-13 04:16:24 PDT
.
Comment 1 Olli Pettay [:smaug] 2011-05-13 04:17:56 PDT
Created attachment 532169 [details]
testcase
Comment 2 Olli Pettay [:smaug] 2011-05-13 06:03:46 PDT
Created attachment 532185 [details] [diff] [review]
wip

Uploaded to tryserver.

While I was looking at this code, I removed one QI in nsDOMTokenListSH::GetStringAt.

We may want to add some helper method which does all the ForgetSharedBuffer
thing. Or perhaps the last parameter of ReadableToJSVal should be
boolean to indicate whether it is ok to call ForgetSharedBuffer.
Comment 3 Olli Pettay [:smaug] 2011-05-13 07:05:41 PDT
Created attachment 532206 [details] [diff] [review]
without QI change

The QI removal doesn't apparently work, so this is just about the
string handling optimization.
Comment 4 Olli Pettay [:smaug] 2011-05-13 07:08:58 PDT
(The QI optimization fails because I forgot the code is used also for nsDOMSettableTokenList)
Comment 5 Boris Zbarsky [:bz] 2011-05-13 08:58:32 PDT
Comment on attachment 532206 [details] [diff] [review]
without QI change

r=me
Comment 6 Olli Pettay [:smaug] 2011-05-13 10:02:25 PDT
http://hg.mozilla.org/mozilla-central/rev/5d5f2f15037b

Note You need to log in before you can comment on or make changes to this bug.