160143 - Minor optimization to CSS color function

Status: NEW

(Core :: Layout, defect, P3)

Description

[Håkan Waara]

This patch slightly optimizes nsCSSRendering::TranformColor() and wipes away a 
redundany call to it from a rendering hot-spot in nsTextFrame...

The reason we don't need it in nsTextFrame, is this:

TransformColor() bails out if its second argument is false, and we call it like 
this:

         } else if (!isPaginated) {
            aRenderingContext.SetColor(currentFGColor);

Comment 1

[Håkan Waara]

Attachment: Patch

Also remove some ifdeffed-out code.

Comment 2

[Håkan Waara]

Bz, wanna check out the patch?

Comment 3

[Håkan Waara]

Attachment: Patch -u10

More context requested.

Comment 4

[Håkan Waara]

Attachment: Patch -u10, v2

More TransformColor() removal, and some review nits fixed.

Comment 5

[Håkan Waara]

Attachment: Patch -u10, v3

A real _patch_ this time. ;-)

Comment 6

[Håkan Waara]

Attachment: Final patch

Fixed some final nits from bz.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 93340 [details] [diff] [review]
Final patch

sr=bzbarsky if you indent that comment about new selection painting two more
spaces

Comment 8

[John Keiser (jkeiser)]

Comment on attachment 93340 [details] [diff] [review]
Final patch

r=jkeiser

Comment 9

[Boris Zbarsky [:bzbarsky]]

hwaara, are you still planning to check this in?  Or what?

Comment 10

[Anne (:annevk)]

Was this ever checked in. Any updates?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Anne, you can use lxr/bonsai to see whether it was checked in.

As for updates, if it hasn't been checked in and applies to trunk, let me know.

Comment 12

[Anne (:annevk)]

I downloaded the patch, updated the locations of the files and also got the
newest files from CVS. However, all "5 hunks" failed. (When comparing them
visually this seems quite logical.)

Comment 13

[Boris Zbarsky [:bzbarsky]]

Well, then if it's not in the trunk yet someone should merge it to trunk and
request reviews... ;)

Comment 14

[timeless]

Attachment: untested merge

anne: if this works, can you rerequest reviews from the original reviewers?

Comment 15

[Anne (:annevk)]

Comment on attachment 174465 [details] [diff] [review]
untested merge

It applies perfectly. Re-requesting review from original reviewers.

Comment 16

[Anne (:annevk)]

Mabus said he might have time to test it later today (tonight). I misunderstood
timeless request.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 174465 [details] [diff] [review]
untested merge

1) jkeiser is not allowed to look at bugzilla by the terms of his employment
contract.

2) Don't ever request review on a patch that hasn't been compiled and run
unless it's really absolutely necessary.

Comment 18

[Håkan Waara]

No idea if the patch still applies nor if this is valid. If someone's interested, feel free to check.   Reassigning to get out of my bug list.