Bug 210658 - REGRESSION (r234105): [iOS] WKColorButton leaks a UIColor
Summary: REGRESSION (r234105): [iOS] WKColorButton leaks a UIColor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on: 187871
Blocks:
  Show dependency treegraph
 
Reported: 2020-04-17 09:26 PDT by David Kilzer (:ddkilzer)
Modified: 2020-04-17 15:08 PDT (History)
9 users (show)

See Also:


Attachments
Patch v1 (3.25 KB, patch)
2020-04-17 09:31 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch for landing (3.27 KB, patch)
2020-04-17 14:42 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2020-04-17 09:26:21 PDT
WKColorButton leaks a UIColor.

Found by clang static analyzer:

'WKColorButton' lacks a 'dealloc' instance method but must release '_color'

--

#pragma mark - WKColorButton

@interface WKColorButton : UIButton
@property (nonatomic, strong) UIColor *color;

+ (instancetype)colorButtonWithColor:(UIColor *)color;
@end

@implementation WKColorButton

+ (instancetype)colorButtonWithColor:(UIColor *)color
{
    WKColorButton *colorButton = [WKColorButton buttonWithType:UIButtonTypeCustom];
    colorButton.color = color;
    colorButton.backgroundColor = color;
    return colorButton;
}

@end
Comment 1 Radar WebKit Bug Importer 2020-04-17 09:27:32 PDT
<rdar://problem/61938137>
Comment 2 David Kilzer (:ddkilzer) 2020-04-17 09:31:30 PDT
Created attachment 396770 [details]
Patch v1
Comment 3 Darin Adler 2020-04-17 10:15:28 PDT
Comment on attachment 396770 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=396770&action=review

> Source/WebCore/page/Chrome.cpp:428
>  #if PLATFORM(IOS_FAMILY)
>      return nullptr;
> -#endif
> +#else
>      notifyPopupOpeningObservers();
>      return m_client.createColorChooser(client, initialColor);
> +#endif

We need UNUSED_PARAM here.

I also think the "always-null color chooser" design strategy is a little messy, but that goes beyond this function
Comment 4 David Kilzer (:ddkilzer) 2020-04-17 14:42:16 PDT
Created attachment 396803 [details]
Patch for landing
Comment 5 EWS 2020-04-17 15:08:36 PDT
Committed r260290: <https://trac.webkit.org/changeset/260290>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396803 [details].