146358 2015-06-26 12:36:37 -0700 ScriptMessageHandlerDelegate::didPostMessage() should reuse its JSContext instance 2015-06-26 16:47:47 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam mark.lam andersca fpizlo msaboff sam webkit-bug-importer oldest_to_newest 1105025 0 mark.lam 2015-06-26 12:36:37 -0700 Currently, ScriptMessageHandlerDelegate::didPostMessage() creates a new JSContext on every invocation. It would be more efficient to just create it once and reuse it for all subsequent calls. 1105027 1 mark.lam 2015-06-26 12:37:09 -0700 <rdar://problem/21349359> 1105028 2 255662 mark.lam 2015-06-26 13:07:06 -0700 Created attachment 255662 the patch. 1105029 3 255662 mark.lam 2015-06-26 13:08:42 -0700 Comment on attachment 255662 the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=255662&action=review > Source/WebKit2/ChangeLog:9 > + time it is called. This is JSContext is used only once to deserialized a JSON object typo: extra "is" before JSContext. Will fix this before landing. 1105058 4 255665 mark.lam 2015-06-26 14:22:34 -0700 Created attachment 255665 patch 2: use a static JSContext. Sam pointed out offline that didPostMessage() is always called from the UI thread, and we only need a static instance if the JSContext. Updated the patch to reflect this. 1105067 5 255665 andersca 2015-06-26 14:47:48 -0700 Comment on attachment 255665 patch 2: use a static JSContext. View in context: https://bugs.webkit.org/attachment.cgi?id=255665&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKUserContentController.mm:97 > + static JSContext* context = nil; > + if (!context) > + context = [[JSContext alloc] init]; You can just assign to the variable directly. > Source/WebKit2/UIProcess/API/Cocoa/WKUserContentController.mm:101 > + id body = [value toObject]; value.toObject? > Source/WebKit2/UIProcess/API/Cocoa/WKUserContentController.mm:103 > + > + RetainPtr<WKScriptMessage> message = adoptNS([[WKScriptMessage alloc] _initWithBody:body webView:fromWebPageProxy(page) frameInfo:frameInfo.get() name:m_name.get()]); I'd use auto here. 1105073 6 mark.lam 2015-06-26 15:17:21 -0700 Thanks for the reviews. Ander's feedback as been applied. Landed in r186010: <http://trac.webkit.org/r186010>. 1105088 7 mark.lam 2015-06-26 16:16:41 -0700 Some build bots were not liking the #import <WTF/MainThread.h>. I removed it for now in r186014: <http://trac.webkit.org/r186014> to green the bots while I investigate. 1105091 8 mark.lam 2015-06-26 16:47:47 -0700 #inport and assertion re-added in r186017: <http://trac.webkit.org/r186017>. 255662 2015-06-26 13:07:06 -0700 2015-06-26 14:22:34 -0700 the patch. bug-146358.patch text/plain 3536 mark.lam SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE4NjAwNikKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIxIEBACisyMDE1LTA2LTI2ICBNYXJrIExh bSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBTY3JpcHRNZXNzYWdlSGFuZGxlckRl bGVnYXRlOjpkaWRQb3N0TWVzc2FnZSgpIHNob3VsZCByZXVzZSBpdHMgSlNDb250ZXh0IGluc3Rh bmNlLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ2 MzU4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ3Vy cmVudGx5LCBTY3JpcHRNZXNzYWdlSGFuZGxlckRlbGVnYXRlOjpkaWRQb3N0TWVzc2FnZSgpIGNy ZWF0ZXMgYSBuZXcgSlNDb250ZXh0IGVhY2gKKyAgICAgICAgdGltZSBpdCBpcyBjYWxsZWQuICBU aGlzIGlzIEpTQ29udGV4dCBpcyB1c2VkIG9ubHkgb25jZSB0byBkZXNlcmlhbGl6ZWQgYSBKU09O IG9iamVjdAorICAgICAgICBhbmQgdGhlbiBkZXN0cm95ZWQuICBXZSB3aWxsIGNoYW5nZSBTY3Jp cHRNZXNzYWdlSGFuZGxlckRlbGVnYXRlIHRvIGNhY2hlIHRoZSBKU0NvbnRleHQKKyAgICAgICAg YW5kIHJldXNlIGl0IGluIGFsbCBzdWJzZXF1ZW50IGNhbGxzIHRvIGRpZFBvc3RNZXNzYWdlKCku CisKKyAgICAgICAgQWxzbyBhZGRlZCBhIEBhdXRvcmVsZWFzZXBvb2wgc2NvcGUgaW4gZGlkUG9z dE1lc3NhZ2UoKSBzbyB0aGF0IHRyYW5zaWVudCBPYmpDIG9iamVjdHMKKyAgICAgICAgd2lsbCBi ZSByZWxlYXNlIHNvb25lci4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvV0tVc2Vy Q29udGVudENvbnRyb2xsZXIubW06CisgICAgICAgICgtW1dLVXNlckNvbnRlbnRDb250cm9sbGVy IGFkZFNjcmlwdE1lc3NhZ2VIYW5kbGVyOm5hbWU6XSk6CisKIDIwMTUtMDYtMjYgIE1hdHQgUmFq Y2EgIDxtcmFqY2FAYXBwbGUuY29tPgogCiAgICAgICAgIFdpcmVsZXNzIFBsYXliYWNrIFRhcmdl dC1yZWxhdGVkICNpbXBvcnQgc2hvdWxkIGJlIGEgI2luY2x1ZGUKSW5kZXg6IFNvdXJjZS9XZWJL aXQyL1VJUHJvY2Vzcy9BUEkvQ29jb2EvV0tVc2VyQ29udGVudENvbnRyb2xsZXIubW0KPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1VzZXJDb250ZW50 Q29udHJvbGxlci5tbQkocmV2aXNpb24gMTg2MDA2KQorKysgU291cmNlL1dlYktpdDIvVUlQcm9j ZXNzL0FQSS9Db2NvYS9XS1VzZXJDb250ZW50Q29udHJvbGxlci5tbQkod29ya2luZyBjb3B5KQpA QCAtODcsMjIgKzg3LDI2IEBAIHB1YmxpYzoKICAgICAKICAgICB2aXJ0dWFsIHZvaWQgZGlkUG9z dE1lc3NhZ2UoV2ViS2l0OjpXZWJQYWdlUHJveHkmIHBhZ2UsIFdlYktpdDo6V2ViRnJhbWVQcm94 eSYgZnJhbWUsIGNvbnN0IFdlYktpdDo6U2VjdXJpdHlPcmlnaW5EYXRhJiBzZWN1cml0eU9yaWdp bkRhdGEsIFdlYkNvcmU6OlNlcmlhbGl6ZWRTY3JpcHRWYWx1ZSYgc2VyaWFsaXplZFNjcmlwdFZh bHVlKQogICAgIHsKLSAgICAgICAgUmV0YWluUHRyPFdLRnJhbWVJbmZvPiBmcmFtZUluZm8gPSB3 cmFwcGVyKEFQSTo6RnJhbWVJbmZvOjpjcmVhdGUoZnJhbWUsIHNlY3VyaXR5T3JpZ2luRGF0YS5z ZWN1cml0eU9yaWdpbigpKSk7CisgICAgICAgIEBhdXRvcmVsZWFzZXBvb2wgeworICAgICAgICAg ICAgUmV0YWluUHRyPFdLRnJhbWVJbmZvPiBmcmFtZUluZm8gPSB3cmFwcGVyKEFQSTo6RnJhbWVJ bmZvOjpjcmVhdGUoZnJhbWUsIHNlY3VyaXR5T3JpZ2luRGF0YS5zZWN1cml0eU9yaWdpbigpKSk7 CiAKLSAgICAgICAgUmV0YWluUHRyPEpTQ29udGV4dD4gY29udGV4dCA9IGFkb3B0TlMoW1tKU0Nv bnRleHQgYWxsb2NdIGluaXRdKTsKLSAgICAgICAgSlNWYWx1ZVJlZiB2YWx1ZVJlZiA9IHNlcmlh bGl6ZWRTY3JpcHRWYWx1ZS5kZXNlcmlhbGl6ZShbY29udGV4dCBKU0dsb2JhbENvbnRleHRSZWZd LCAwKTsKLSAgICAgICAgSlNWYWx1ZSAqdmFsdWUgPSBbSlNWYWx1ZSB2YWx1ZVdpdGhKU1ZhbHVl UmVmOnZhbHVlUmVmIGluQ29udGV4dDpjb250ZXh0LmdldCgpXTsKLSAgICAgICAgaWQgYm9keSA9 IFt2YWx1ZSB0b09iamVjdF07Ci0KLSAgICAgICAgUmV0YWluUHRyPFdLU2NyaXB0TWVzc2FnZT4g bWVzc2FnZSA9IGFkb3B0TlMoW1tXS1NjcmlwdE1lc3NhZ2UgYWxsb2NdIF9pbml0V2l0aEJvZHk6 Ym9keSB3ZWJWaWV3OmZyb21XZWJQYWdlUHJveHkocGFnZSkgZnJhbWVJbmZvOmZyYW1lSW5mby5n ZXQoKSBuYW1lOm1fbmFtZS5nZXQoKV0pOwotICAgIAotICAgICAgICBbbV9oYW5kbGVyIHVzZXJD b250ZW50Q29udHJvbGxlcjptX2NvbnRyb2xsZXIuZ2V0KCkgZGlkUmVjZWl2ZVNjcmlwdE1lc3Nh Z2U6bWVzc2FnZS5nZXQoKV07CisgICAgICAgICAgICBpZiAoIW1fY29udGV4dCkKKyAgICAgICAg ICAgICAgICBtX2NvbnRleHQgPSBhZG9wdE5TKFtbSlNDb250ZXh0IGFsbG9jXSBpbml0XSk7Cisg ICAgICAgICAgICBKU1ZhbHVlUmVmIHZhbHVlUmVmID0gc2VyaWFsaXplZFNjcmlwdFZhbHVlLmRl c2VyaWFsaXplKFttX2NvbnRleHQgSlNHbG9iYWxDb250ZXh0UmVmXSwgMCk7CisgICAgICAgICAg ICBKU1ZhbHVlICp2YWx1ZSA9IFtKU1ZhbHVlIHZhbHVlV2l0aEpTVmFsdWVSZWY6dmFsdWVSZWYg aW5Db250ZXh0Om1fY29udGV4dC5nZXQoKV07CisgICAgICAgICAgICBpZCBib2R5ID0gW3ZhbHVl IHRvT2JqZWN0XTsKKworICAgICAgICAgICAgUmV0YWluUHRyPFdLU2NyaXB0TWVzc2FnZT4gbWVz c2FnZSA9IGFkb3B0TlMoW1tXS1NjcmlwdE1lc3NhZ2UgYWxsb2NdIF9pbml0V2l0aEJvZHk6Ym9k eSB3ZWJWaWV3OmZyb21XZWJQYWdlUHJveHkocGFnZSkgZnJhbWVJbmZvOmZyYW1lSW5mby5nZXQo KSBuYW1lOm1fbmFtZS5nZXQoKV0pOworICAgICAgICAKKyAgICAgICAgICAgIFttX2hhbmRsZXIg dXNlckNvbnRlbnRDb250cm9sbGVyOm1fY29udHJvbGxlci5nZXQoKSBkaWRSZWNlaXZlU2NyaXB0 TWVzc2FnZTptZXNzYWdlLmdldCgpXTsKKyAgICAgICAgfQogICAgIH0KIAogcHJpdmF0ZToKICAg ICBSZXRhaW5QdHI8V0tVc2VyQ29udGVudENvbnRyb2xsZXI+IG1fY29udHJvbGxlcjsKICAgICBS ZXRhaW5QdHI8aWQgPFdLU2NyaXB0TWVzc2FnZUhhbmRsZXI+PiBtX2hhbmRsZXI7CiAgICAgUmV0 YWluUHRyPE5TU3RyaW5nPiBtX25hbWU7CisgICAgUmV0YWluUHRyPEpTQ29udGV4dD4gbV9jb250 ZXh0OwogfTsKIAogLSAodm9pZClhZGRTY3JpcHRNZXNzYWdlSGFuZGxlcjooaWQgPFdLU2NyaXB0 TWVzc2FnZUhhbmRsZXI+KXNjcmlwdE1lc3NhZ2VIYW5kbGVyIG5hbWU6KE5TU3RyaW5nICopbmFt ZQo= 255665 2015-06-26 14:22:34 -0700 2015-06-26 14:47:48 -0700 patch 2: use a static JSContext. bug-146358.patch text/plain 3482 mark.lam SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE4NjAwNikKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDE1LTA2LTI2ICBNYXJrIExh bSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBTY3JpcHRNZXNzYWdlSGFuZGxlckRl bGVnYXRlOjpkaWRQb3N0TWVzc2FnZSgpIHNob3VsZCByZXVzZSBpdHMgSlNDb250ZXh0IGluc3Rh bmNlLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ2 MzU4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ3Vy cmVudGx5LCBTY3JpcHRNZXNzYWdlSGFuZGxlckRlbGVnYXRlOjpkaWRQb3N0TWVzc2FnZSgpIGNy ZWF0ZXMgYSBuZXcgSlNDb250ZXh0IGVhY2gKKyAgICAgICAgdGltZSBpdCBpcyBjYWxsZWQuICBU aGlzIEpTQ29udGV4dCBpcyB1c2VkIG9ubHkgb25jZSB0byBkZXNlcmlhbGl6ZWQgYSBKU09OIG9i amVjdAorICAgICAgICBhbmQgdGhlbiBkZXN0cm95ZWQuICBXZSB3aWxsIGNoYW5nZSBTY3JpcHRN ZXNzYWdlSGFuZGxlckRlbGVnYXRlIHRvIGNhY2hlIHRoZSBKU0NvbnRleHQKKyAgICAgICAgYW5k IHJldXNlIGl0IGluIGFsbCBzdWJzZXF1ZW50IGNhbGxzIHRvIGRpZFBvc3RNZXNzYWdlKCkuCisK KyAgICAgICAgQWxzbyBhZGRlZCBhIEBhdXRvcmVsZWFzZXBvb2wgc2NvcGUgaW4gZGlkUG9zdE1l c3NhZ2UoKSBzbyB0aGF0IHRyYW5zaWVudCBPYmpDIG9iamVjdHMKKyAgICAgICAgd2lsbCBiZSBy ZWxlYXNlIHNvb25lci4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvV0tVc2VyQ29u dGVudENvbnRyb2xsZXIubW06CisKIDIwMTUtMDYtMjYgIE1hdHQgUmFqY2EgIDxtcmFqY2FAYXBw bGUuY29tPgogCiAgICAgICAgIFdpcmVsZXNzIFBsYXliYWNrIFRhcmdldC1yZWxhdGVkICNpbXBv cnQgc2hvdWxkIGJlIGEgI2luY2x1ZGUKSW5kZXg6IFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9B UEkvQ29jb2EvV0tVc2VyQ29udGVudENvbnRyb2xsZXIubW0KPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNl L1dlYktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1VzZXJDb250ZW50Q29udHJvbGxlci5tbQko cmV2aXNpb24gMTg2MDA2KQorKysgU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9X S1VzZXJDb250ZW50Q29udHJvbGxlci5tbQkod29ya2luZyBjb3B5KQpAQCAtNDAsNiArNDAsNyBA QAogI2ltcG9ydCAiX1dLVXNlckNvbnRlbnRGaWx0ZXJJbnRlcm5hbC5oIgogI2ltcG9ydCA8SmF2 YVNjcmlwdENvcmUvSlNDb250ZXh0Lmg+CiAjaW1wb3J0IDxKYXZhU2NyaXB0Q29yZS9KU1ZhbHVl Lmg+CisjaW1wb3J0IDxXVEYvTWFpblRocmVhZC5oPgogI2ltcG9ydCA8V2ViQ29yZS9TZXJpYWxp emVkU2NyaXB0VmFsdWUuaD4KIAogQGltcGxlbWVudGF0aW9uIFdLVXNlckNvbnRlbnRDb250cm9s bGVyCkBAIC04NywxNiArODgsMjIgQEAgcHVibGljOgogICAgIAogICAgIHZpcnR1YWwgdm9pZCBk aWRQb3N0TWVzc2FnZShXZWJLaXQ6OldlYlBhZ2VQcm94eSYgcGFnZSwgV2ViS2l0OjpXZWJGcmFt ZVByb3h5JiBmcmFtZSwgY29uc3QgV2ViS2l0OjpTZWN1cml0eU9yaWdpbkRhdGEmIHNlY3VyaXR5 T3JpZ2luRGF0YSwgV2ViQ29yZTo6U2VyaWFsaXplZFNjcmlwdFZhbHVlJiBzZXJpYWxpemVkU2Ny aXB0VmFsdWUpCiAgICAgewotICAgICAgICBSZXRhaW5QdHI8V0tGcmFtZUluZm8+IGZyYW1lSW5m byA9IHdyYXBwZXIoQVBJOjpGcmFtZUluZm86OmNyZWF0ZShmcmFtZSwgc2VjdXJpdHlPcmlnaW5E YXRhLnNlY3VyaXR5T3JpZ2luKCkpKTsKKyAgICAgICAgQGF1dG9yZWxlYXNlcG9vbCB7CisgICAg ICAgICAgICBSZXRhaW5QdHI8V0tGcmFtZUluZm8+IGZyYW1lSW5mbyA9IHdyYXBwZXIoQVBJOjpG cmFtZUluZm86OmNyZWF0ZShmcmFtZSwgc2VjdXJpdHlPcmlnaW5EYXRhLnNlY3VyaXR5T3JpZ2lu KCkpKTsKIAotICAgICAgICBSZXRhaW5QdHI8SlNDb250ZXh0PiBjb250ZXh0ID0gYWRvcHROUyhb W0pTQ29udGV4dCBhbGxvY10gaW5pdF0pOwotICAgICAgICBKU1ZhbHVlUmVmIHZhbHVlUmVmID0g c2VyaWFsaXplZFNjcmlwdFZhbHVlLmRlc2VyaWFsaXplKFtjb250ZXh0IEpTR2xvYmFsQ29udGV4 dFJlZl0sIDApOwotICAgICAgICBKU1ZhbHVlICp2YWx1ZSA9IFtKU1ZhbHVlIHZhbHVlV2l0aEpT VmFsdWVSZWY6dmFsdWVSZWYgaW5Db250ZXh0OmNvbnRleHQuZ2V0KCldOwotICAgICAgICBpZCBi b2R5ID0gW3ZhbHVlIHRvT2JqZWN0XTsKLQotICAgICAgICBSZXRhaW5QdHI8V0tTY3JpcHRNZXNz YWdlPiBtZXNzYWdlID0gYWRvcHROUyhbW1dLU2NyaXB0TWVzc2FnZSBhbGxvY10gX2luaXRXaXRo Qm9keTpib2R5IHdlYlZpZXc6ZnJvbVdlYlBhZ2VQcm94eShwYWdlKSBmcmFtZUluZm86ZnJhbWVJ bmZvLmdldCgpIG5hbWU6bV9uYW1lLmdldCgpXSk7Ci0gICAgCi0gICAgICAgIFttX2hhbmRsZXIg dXNlckNvbnRlbnRDb250cm9sbGVyOm1fY29udHJvbGxlci5nZXQoKSBkaWRSZWNlaXZlU2NyaXB0 TWVzc2FnZTptZXNzYWdlLmdldCgpXTsKKyAgICAgICAgICAgIEFTU0VSVChpc1VJVGhyZWFkKCkp OworICAgICAgICAgICAgc3RhdGljIEpTQ29udGV4dCogY29udGV4dCA9IG5pbDsKKyAgICAgICAg ICAgIGlmICghY29udGV4dCkKKyAgICAgICAgICAgICAgICBjb250ZXh0ID0gW1tKU0NvbnRleHQg YWxsb2NdIGluaXRdOworCisgICAgICAgICAgICBKU1ZhbHVlUmVmIHZhbHVlUmVmID0gc2VyaWFs aXplZFNjcmlwdFZhbHVlLmRlc2VyaWFsaXplKFtjb250ZXh0IEpTR2xvYmFsQ29udGV4dFJlZl0s IDApOworICAgICAgICAgICAgSlNWYWx1ZSAqdmFsdWUgPSBbSlNWYWx1ZSB2YWx1ZVdpdGhKU1Zh bHVlUmVmOnZhbHVlUmVmIGluQ29udGV4dDpjb250ZXh0XTsKKyAgICAgICAgICAgIGlkIGJvZHkg PSBbdmFsdWUgdG9PYmplY3RdOworCisgICAgICAgICAgICBSZXRhaW5QdHI8V0tTY3JpcHRNZXNz YWdlPiBtZXNzYWdlID0gYWRvcHROUyhbW1dLU2NyaXB0TWVzc2FnZSBhbGxvY10gX2luaXRXaXRo Qm9keTpib2R5IHdlYlZpZXc6ZnJvbVdlYlBhZ2VQcm94eShwYWdlKSBmcmFtZUluZm86ZnJhbWVJ bmZvLmdldCgpIG5hbWU6bV9uYW1lLmdldCgpXSk7CisgICAgICAgIAorICAgICAgICAgICAgW21f aGFuZGxlciB1c2VyQ29udGVudENvbnRyb2xsZXI6bV9jb250cm9sbGVyLmdldCgpIGRpZFJlY2Vp dmVTY3JpcHRNZXNzYWdlOm1lc3NhZ2UuZ2V0KCldOworICAgICAgICB9CiAgICAgfQogCiBwcml2 YXRlOgo=