Bug 231992

Summary: [Cocoa] Merge and simplify the nsColor family of functions
Product: WebKit Reporter: Darin Adler <darin>
Component: PlatformAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andersca, andresg_22, apinheiro, cdumez, cfleizach, changseok, dmazzoni, esprehn+autocc, ews-watchlist, glenn, jcraig, jdiggs, kondapallykalyan, mifenton, pdr, samuel_white, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch andersca: review+

Darin Adler
Reported 2021-10-19 15:27:50 PDT
Merge and simplify the nsColor family of functions
Attachments
Patch (76.15 KB, patch)
2021-10-20 09:04 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
Patch (76.13 KB, patch)
2021-10-20 10:52 PDT, Darin Adler 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (76.13 KB, patch)
2021-10-20 11:24 PDT, Darin Adler 		andersca: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2021-10-20 09:04:52 PDT Comment hidden (obsolete)
Darin Adler
Comment 2 2021-10-20 10:52:46 PDT Comment hidden (obsolete)
Darin Adler
Comment 3 2021-10-20 11:24:44 PDT
Created attachment 441906 [details] Patch
Darin Adler
Comment 4 2021-10-20 14:52:47 PDT
Passing all the EWS tests now so a good time for someone to review. Style feedback welcome.
Anders Carlsson
Comment 5 2021-10-21 08:12:38 PDT
Comment on attachment 441906 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441906&action=review > Source/WebCore/platform/graphics/cocoa/ColorCocoa.mm:38 > +RetainPtr<UIColor> cocoaColor(const Color& color) Can/should this assert if the passed in color is invalid?
Darin Adler
Comment 6 2021-10-21 11:08:01 PDT
Comment on attachment 441906 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441906&action=review >> Source/WebCore/platform/graphics/cocoa/ColorCocoa.mm:38 >> +RetainPtr<UIColor> cocoaColor(const Color& color) > > Can/should this assert if the passed in color is invalid? Currently the behavior is to treat invalid colors as transparent. Removing this feature and instead requiring that no one pass any invalid colors would probably be OK, but I’d want to review that change one call site at a time. If I was *sure* that no one needed the "treat invalid as transparent" behavior, I would be tempted to just merge the cocoaColorOrNil behavior in to the cocoaColor function and not have two. This makes me re-discover why I like std::optional better than types like RefPtr, WTF::String, and WTF::Color that have the null value built-in. It’s not easy to see if nulls are expected or an error.
Darin Adler
Comment 7 2021-10-21 11:25:16 PDT
Committed r284630 (243351@main): <https://commits.webkit.org/243351@main>
Radar WebKit Bug Importer
Comment 8 2021-10-21 11:26:17 PDT
<rdar://problem/84516075>
Note You need to log in before you can comment on or make changes to this bug.