Summary: | _WKThumbnailView should expose its snapshot size | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Conrad Shultz <conrad_shultz> | ||||
Component: | WebKit2 | Assignee: | Conrad Shultz <conrad_shultz> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | conrad_shultz, sam | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Local Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Conrad Shultz
2016-05-24 17:34:15 PDT
Created attachment 279728 [details]
Patch
Can you please add an API test for this? Comment on attachment 279728 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=279728&action=review > Source/WebKit2/UIProcess/API/Cocoa/_WKThumbnailView.mm:144 > + [self willChangeValueForKey:@"snapshotSize"]; > + > + _snapshotSize = CGSizeMake(CGImageGetWidth(image), CGImageGetHeight(image)); Should this optimize the case where the value of _snapshotSize is not changing to not call willChangeValueForKey/didChangeValueForKey in that case? Maybe it’s common to update the snapshot without the image size changing? (In reply to comment #4) > Can you please add an API test for this? I'll look into it! (In reply to comment #5) > Comment on attachment 279728 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=279728&action=review > > > Source/WebKit2/UIProcess/API/Cocoa/_WKThumbnailView.mm:144 > > + [self willChangeValueForKey:@"snapshotSize"]; > > + > > + _snapshotSize = CGSizeMake(CGImageGetWidth(image), CGImageGetHeight(image)); > > Should this optimize the case where the value of _snapshotSize is not > changing to not call willChangeValueForKey/didChangeValueForKey in that > case? Maybe it’s common to update the snapshot without the image size > changing? Hopefully updates overall aren't particularly common, and I generally expect any observers to check the value to see whether the change was of interest, but there's probably no harm in further optimizing here. |