| Summary: | [Cocoa] Merge and simplify the nsColor family of functions | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||
| Component: | Platform | Assignee: | 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
Darin Adler
2021-10-19 15:27:50 PDT
Created attachment 441886 [details]
Patch
Created attachment 441897 [details]
Patch
Created attachment 441906 [details]
Patch
Passing all the EWS tests now so a good time for someone to review. Style feedback welcome. 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? 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. Committed r284630 (243351@main): <https://commits.webkit.org/243351@main> |