Merge nsColorNames into nsColor to have fewer public functions and less code

VERIFIED FIXED in mozilla1.9.1b2

Status

()

VERIFIED FIXED
11 years ago
2 months ago

People

(Reporter: alfredkayser, Assigned: alfredkayser)

Tracking

(Blocks: 1 bug, {memory-footprint, verified1.9.1})

Trunk
mozilla1.9.1b2
memory-footprint, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
Created attachment 309098 [details] [diff] [review]
V1: Merge nsColorNames into nsColor

merge nsColor and nsColorName
    nsColor is only user of nsLookupName of nsColorName
    GetStringValue is nowhere used.
    kColors public member of nsColorName is only used in nsColor (and TestColorNames...)

The only real function of nsColorName is used by NS_ColorNameToRGB from nsColor, so instead of exposing nsColorName as a separate class, embed it into nsColor. 

This results in less code, saving about 2K from gkgfx.lib (and about 22K objectsize).
Attachment #309098 - Flags: review?(vladimir)
Version: unspecified → Trunk
(Assignee)

Updated

10 years ago
Blocks: 430829
(Assignee)

Comment 1

10 years ago
Created attachment 337057 [details] [diff] [review]
V2: Updated to current trunk
Attachment #309098 - Attachment is obsolete: true
Attachment #309098 - Flags: review?(vladimir)
(Assignee)

Updated

10 years ago
Attachment #337057 - Flags: review?(vladimir)
(Assignee)

Comment 2

10 years ago
Comment on attachment 337057 [details] [diff] [review]
V2: Updated to current trunk

Changed the patch so that no change to layout/build is needed anymore, but still with all the code removal in place.

Less=More!
(Assignee)

Updated

10 years ago
Attachment #337057 - Flags: superreview?(roc)
(Assignee)

Comment 3

10 years ago
Requesting 'Wanted1.9.1' for a simple and reviewed code saving patch.
Flags: wanted1.9.1?
Please don't abuse the blocking/wanted process to get people to do reviews.
(Assignee)

Comment 5

10 years ago
That was not the intention. I thought the wanted flag is needed for checkin into FF3.1?
Attachment #337057 - Flags: superreview?(roc) → superreview+
(In reply to comment #5)
> That was not the intention. I thought the wanted flag is needed for checkin
> into FF3.1?

It isn't (at least not yet).
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/003a5a6570af
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
This caused bustage, but it was easy to fix.

http://hg.mozilla.org/mozilla-central/rev/580e62ad39ae
Summary: Merge nsColorNames into nsColor to have less public functions and less code → Merge nsColorNames into nsColor to have fewer public functions and less code
Flags: wanted1.9.1? → wanted1.9.1+
(Assignee)

Updated

10 years ago
Status: RESOLVED → VERIFIED
Seeing as there hasn't been any discussions about this bug for 5 months and it's not really a testable bug, I'm assuming there aren't any residual issues. I'm moving this to verified as a result. If anyone has any qualms, feel free to bring them up.
Keywords: fixed1.9.1 → verified1.9.1
Depends on: 551775

Updated

2 months ago
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.