Bug 187788

Summary: Let clients override _WKThumbnailView's background color
Product: WebKit Reporter: Ricky Mondello <rmondello>
Component: PlatformAssignee: Ricky Mondello <rmondello>
Status: RESOLVED FIXED    
Severity: Normal CC: anshu, commit-queue, rmondello, sam, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Attempt.
thorton: review+
Attempt 2, using setNeedsDisplay
none
Attempt 3, calling -setNeedsDisplay: on the view itself none

Ricky Mondello
Reported 2018-07-18 15:31:04 PDT
Let clients override _WKThumbnailView's background color
Attachments
Attempt. (3.71 KB, patch)
2018-07-18 16:57 PDT, Ricky Mondello 		thorton: review+
DetailsFormatted DiffDiff
Attempt 2, using setNeedsDisplay (3.72 KB, patch)
2018-07-18 17:10 PDT, Ricky Mondello 		no flags
DetailsFormatted DiffDiff
Attempt 3, calling -setNeedsDisplay: on the view itself (3.72 KB, patch)
2018-07-18 17:24 PDT, Ricky Mondello 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ricky Mondello
Comment 1 2018-07-18 16:57:11 PDT
Created attachment 345305 [details] Attempt.
Anshu Chimala
Comment 2 2018-07-18 17:05:57 PDT
Comment on attachment 345305 [details] Attempt. View in context: https://bugs.webkit.org/attachment.cgi?id=345305&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKThumbnailView.mm:159 > +- (void)setOverrideBackgroundColor:(NSColor *)overrideBackgroundColor > +{ > + if ([_overrideBackgroundColor isEqual:overrideBackgroundColor]) > + return; > + > + _overrideBackgroundColor = overrideBackgroundColor; > + [self updateLayer]; It seems strange to call -updateLayer directly. Could we instead -setNeedsDisplay on the layer so that -updateLayer gets called during the next display pass?
Tim Horton
Comment 3 2018-07-18 17:07:39 PDT
Comment on attachment 345305 [details] Attempt. View in context: https://bugs.webkit.org/attachment.cgi?id=345305&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKThumbnailView.mm:159 > + [self updateLayer]; Don't you usually setNeedsDisplay, not updateLayer directly?
Tim Horton
Comment 4 2018-07-18 17:08:07 PDT
lol, yes, that
Ricky Mondello
Comment 5 2018-07-18 17:10:24 PDT
Created attachment 345308 [details] Attempt 2, using setNeedsDisplay
Tim Horton
Comment 6 2018-07-18 17:14:25 PDT
Comment on attachment 345308 [details] Attempt 2, using setNeedsDisplay View in context: https://bugs.webkit.org/attachment.cgi?id=345308&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKThumbnailView.mm:159 > + [self.layer setNeedsDisplay]; setNeedsDisplay on the view, no? Isn't that how you get to updateLayer? This might work too, I'm not sure.
Ricky Mondello
Comment 7 2018-07-18 17:24:24 PDT
Created attachment 345310 [details] Attempt 3, calling -setNeedsDisplay: on the view itself
WebKit Commit Bot
Comment 8 2018-07-18 18:19:47 PDT
Comment on attachment 345310 [details] Attempt 3, calling -setNeedsDisplay: on the view itself Clearing flags on attachment: 345310 Committed r233945: <https://trac.webkit.org/changeset/233945>
WebKit Commit Bot
Comment 9 2018-07-18 18:19:48 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2018-07-18 18:20:25 PDT
<rdar://problem/42361917>
Sam Weinig
Comment 11 2018-07-19 13:29:53 PDT
Comment on attachment 345310 [details] Attempt 3, calling -setNeedsDisplay: on the view itself View in context: https://bugs.webkit.org/attachment.cgi?id=345310&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKThumbnailView.h:51 > +@property (strong, nonatomic) NSColor *overrideBackgroundColor; I believe this should probably be nullable.
Note You need to log in before you can comment on or make changes to this bug.