Bug 158049

Summary: _WKThumbnailView should expose its snapshot size
Product: WebKit Reporter: Conrad Shultz <conrad_shultz>
Component: WebKit2Assignee: 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 Flags
Patch thorton: review+

Description Conrad Shultz 2016-05-24 17:34:15 PDT
_WKThumbnailView should expose its snapshot size and make it observable.
Comment 1 Conrad Shultz 2016-05-24 17:34:47 PDT
<rdar://problem/26458877>
Comment 2 Conrad Shultz 2016-05-24 17:36:45 PDT
Created attachment 279728 [details]
Patch
Comment 3 Conrad Shultz 2016-05-24 17:45:43 PDT
Committed http://svn.webkit.org/repository/webkit/trunk@201366
Comment 4 Sam Weinig 2016-05-24 22:02:33 PDT
Can you please add an API test for this?
Comment 5 Darin Adler 2016-05-25 03:35:34 PDT
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?
Comment 6 Conrad Shultz 2016-05-25 10:27:25 PDT
(In reply to comment #4)
> Can you please add an API test for this?

I'll look into it!
Comment 7 Conrad Shultz 2016-05-25 10:28:49 PDT
(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.