158049 2016-05-24 17:34:15 -0700 _WKThumbnailView should expose its snapshot size 2016-05-25 10:28:49 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Local Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 conrad_shultz conrad_shultz conrad_shultz sam oldest_to_newest 1196096 0 conrad_shultz 2016-05-24 17:34:15 -0700 _WKThumbnailView should expose its snapshot size and make it observable. 1196097 1 conrad_shultz 2016-05-24 17:34:47 -0700 <rdar://problem/26458877> 1196098 2 279728 conrad_shultz 2016-05-24 17:36:45 -0700 Created attachment 279728 Patch 1196103 3 conrad_shultz 2016-05-24 17:45:43 -0700 Committed http://svn.webkit.org/repository/webkit/trunk@201366 1196169 4 sam 2016-05-24 22:02:33 -0700 Can you please add an API test for this? 1196265 5 279728 darin 2016-05-25 03:35:34 -0700 Comment on attachment 279728 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? 1196370 6 conrad_shultz 2016-05-25 10:27:25 -0700 (In reply to comment #4) > Can you please add an API test for this? I'll look into it! 1196372 7 conrad_shultz 2016-05-25 10:28:49 -0700 (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. 279728 2016-05-24 17:36:45 -0700 2016-05-24 17:40:04 -0700 Patch bug-158049-20160524173752.patch text/plain 2652 conrad_shultz U3VidmVyc2lvbiBSZXZpc2lvbjogMjAxMzM3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggN2IzOWMxN2IxZDBjMDZk N2IxNmIyNGFmNDM2NzJhOTcyODliYTI1Yy4uNDhjOGMxODhhYmI2NTgwYTYwODY5ODE2ZWU3ZGEy ODMzMTcxMGE4OCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE2LTA1LTI0ICBDb25y YWQgU2h1bHR6ICA8Y29ucmFkX3NodWx0ekBhcHBsZS5jb20+CisKKyAgICAgICAgX1dLVGh1bWJu YWlsVmlldyBzaG91bGQgZXhwb3NlIGl0cyBzbmFwc2hvdCBzaXplCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTgwNDkKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvX1dL VGh1bWJuYWlsVmlldy5oOgorICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvX1dLVGh1bWJu YWlsVmlldy5tbToKKyAgICAgICAgKC1bX1dLVGh1bWJuYWlsVmlldyBfZGlkVGFrZVNuYXBzaG90 Ol0pOgorICAgICAgICBVcGRhdGUgdGhlIG5ldyBzbmFwc2hvdFNpemUgcHJvcGVydHkgaW4gYSBL Vk8tY29tcGxpYW50IG1hbm5lci4KKwogMjAxNi0wNS0yNCAgQ2hyaXMgRHVtZXogIDxjZHVtZXpA YXBwbGUuY29tPgogCiAgICAgICAgIFVzZSBhdXRvIGZvciBzb21lIG9mIG91ciBsYW1iZGEgZnVu Y3Rpb24gcGFyYW1ldGVycwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQ SS9Db2NvYS9fV0tUaHVtYm5haWxWaWV3LmggYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJ L0NvY29hL19XS1RodW1ibmFpbFZpZXcuaAppbmRleCA3MjQ5YWM0NmU5ZmFjN2I1ODVkOWJmNDM1 MTc0YTk2MjIxODQ1NGI2Li5hYzI2NjViODYzMmExYWZiNGYxZTIzM2RmMGQ2ODM4NDhhMThiM2Nh IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL0NvY29hL19XS1RodW1i bmFpbFZpZXcuaAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL0NvY29hL19XS1Ro dW1ibmFpbFZpZXcuaApAQCAtMzksNiArMzksNyBAQCBXS19DTEFTU19BVkFJTEFCTEUoMTBfMTAs IDhfMCkKIC0gKGluc3RhbmNldHlwZSlpbml0V2l0aEZyYW1lOihOU1JlY3QpZnJhbWUgZnJvbVdL VmlldzooV0tWaWV3ICopd2tWaWV3OwogCiBAcHJvcGVydHkgKG5vbmF0b21pYykgQ0dGbG9hdCBz Y2FsZTsKK0Bwcm9wZXJ0eSAobm9uYXRvbWljLCByZWFkb25seSkgQ0dTaXplIHNuYXBzaG90U2l6 ZTsKIEBwcm9wZXJ0eSAobm9uYXRvbWljKSBCT09MIGV4Y2x1c2l2ZWx5VXNlc1NuYXBzaG90Owog CiAvLyBUaGlzIHNob3VsZCBiZSByZW1vdmVkIHdoZW4gYWxsIGNsaWVudHMgZ28gYXdheTsgaXQg aXMgYWx3YXlzIFlFUyBub3cuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv QVBJL0NvY29hL19XS1RodW1ibmFpbFZpZXcubW0gYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv QVBJL0NvY29hL19XS1RodW1ibmFpbFZpZXcubW0KaW5kZXggZWM1YjIyMGVkYjkyNGM5OTRjYTBl Yzg2NDY3ZDQ1MmIwMWRjOWU0Yi4uNjE3YWI2MTFmMzY2YjZhZjk1ODZmYzA1MjM4MThkNTVjMTUw ODk3NSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9fV0tU aHVtYm5haWxWaWV3Lm1tCisrKyBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvQ29jb2Ev X1dLVGh1bWJuYWlsVmlldy5tbQpAQCAtNTYsNiArNTYsNyBAQCB1c2luZyBuYW1lc3BhY2UgV2Vi S2l0OwogICAgIGRvdWJsZSBfbGFzdFNuYXBzaG90U2NhbGU7CiB9CiAKK0BzeW50aGVzaXplIHNu YXBzaG90U2l6ZT1fc25hcHNob3RTaXplOwogQHN5bnRoZXNpemUgX3dhaXRpbmdGb3JTbmFwc2hv dD1fd2FpdGluZ0ZvclNuYXBzaG90OwogQHN5bnRoZXNpemUgZXhjbHVzaXZlbHlVc2VzU25hcHNo b3Q9X2V4Y2x1c2l2ZWx5VXNlc1NuYXBzaG90OwogCkBAIC0xMzgsNiArMTM5LDkgQEAgdXNpbmcg bmFtZXNwYWNlIFdlYktpdDsKIAogLSAodm9pZClfZGlkVGFrZVNuYXBzaG90OihDR0ltYWdlUmVm KWltYWdlCiB7CisgICAgW3NlbGYgd2lsbENoYW5nZVZhbHVlRm9yS2V5OkAic25hcHNob3RTaXpl Il07CisKKyAgICBfc25hcHNob3RTaXplID0gQ0dTaXplTWFrZShDR0ltYWdlR2V0V2lkdGgoaW1h Z2UpLCBDR0ltYWdlR2V0SGVpZ2h0KGltYWdlKSk7CiAgICAgX3dhaXRpbmdGb3JTbmFwc2hvdCA9 IE5POwogICAgIHNlbGYubGF5ZXIuc3VibGF5ZXJzID0gQFtdOwogICAgIHNlbGYubGF5ZXIuY29u dGVudHNHcmF2aXR5ID0ga0NBR3Jhdml0eVJlc2l6ZUFzcGVjdEZpbGw7CkBAIC0xNDgsNiArMTUy LDggQEAgdXNpbmcgbmFtZXNwYWNlIFdlYktpdDsKICAgICAgICAgX3NuYXBzaG90V2FzRGVmZXJy ZWQgPSBOTzsKICAgICAgICAgW3NlbGYgX3JlcXVlc3RTbmFwc2hvdElmTmVlZGVkXTsKICAgICB9 CisKKyAgICBbc2VsZiBkaWRDaGFuZ2VWYWx1ZUZvcktleTpAInNuYXBzaG90U2l6ZSJdOwogfQog CiAtICh2b2lkKXZpZXdEaWRNb3ZlVG9XaW5kb3cK