Closed Bug 387044 Opened 15 years ago Closed 15 years ago

Cleanup string usage in nsCSSParser

Categories

(Core :: CSS Parsing and Computation, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: sharparrow1, Unassigned)

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
Some small stuff.
Attachment #271136 - Flags: review?(bzbarsky)
So how do .get() and BeginReading() actually differ?  I assume the big win here is the lack of need to call PromiseFlatString() and the lack of a copy in ParseColorString()?
(In reply to comment #1)
> So how do .get() and BeginReading() actually differ?
The primary difference is that you can only call get() on a string with guaranteed null-terminated storage, i.e. an ns[C]String. They return exactly the same pointer.  The minor difference is that BeginReading returns a const char*.  

> I assume the big win here
> is the lack of need to call PromiseFlatString() and the lack of a copy in
> ParseColorString()?

That's right. PromiseFlatString is probably cheap in this case (just a small stack allocation plus a function call) because we're unlikely to be passing in non-null-terminated strings; however, we don't need it.
Wait, actually BeginReading involves a function call.  But it's functionally the same for internal strings.
(In reply to comment #3)
> Wait, actually BeginReading involves a function call.  But it's functionally
> the same for internal strings.

Wait, no, I was right the first time... I got confused by some code that's ifdef'ed out.
Comment on attachment 271136 [details] [diff] [review]
Patch

Oh, BeginReading() is inlined too.  Great.

r+sr=bzbarsky
Attachment #271136 - Flags: superreview+
Attachment #271136 - Flags: review?(bzbarsky)
Attachment #271136 - Flags: review+
Checked in.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.