116304 2013-05-17 04:37:55 -0700 Remove custom code for MessageEvent.ports getter 2013-05-23 01:00:44 -0700 1 1 1 Unclassified WebKit Bindings 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED http://dev.w3.org/html5/pf-summary/comms.html#messageevent P2 Normal --- 1 cdumez cdumez alecflett benjamin commit-queue esprehn+autocc haraken jsbell laszlo.gombos rniwa oldest_to_newest 890286 0 cdumez 2013-05-17 04:37:55 -0700 The "ports" attribute in MessageEvent.idl is currently using a CustomGetter. The attribute is of type "MessagePortArray", which the JSC bindings generator has support for. Therefore, we should be able to get rid of the custom code with minor generator tweaks. 890289 1 202062 cdumez 2013-05-17 04:57:26 -0700 Created attachment 202062 Patch 890293 2 202062 haraken 2013-05-17 05:02:00 -0700 Comment on attachment 202062 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202062&action=review > Source/WebCore/dom/MessageEvent.idl:37 > + [InitializedByEventConstructor] readonly attribute MessagePortArray ports; Per the spec, this should be 'MessagePort[]?'. Would you fix it in a follow-up patch? http://dev.w3.org/html5/postmsg/#event-definitions 890300 3 202062 commit-queue 2013-05-17 05:24:18 -0700 Comment on attachment 202062 Patch Clearing flags on attachment: 202062 Committed r150249: <http://trac.webkit.org/changeset/150249> 890301 4 commit-queue 2013-05-17 05:24:20 -0700 All reviewed patches have been landed. Closing bug. 890303 5 cdumez 2013-05-17 05:28:22 -0700 (In reply to comment #2) > (From update of attachment 202062 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202062&action=review > > > Source/WebCore/dom/MessageEvent.idl:37 > > + [InitializedByEventConstructor] readonly attribute MessagePortArray ports; > > Per the spec, this should be 'MessagePort[]?'. Would you fix it in a follow-up patch? > > http://dev.w3.org/html5/postmsg/#event-definitions This is visibly a recent spec change as it wasn't nullable before. I don't know how other browsers behave so this would need to be checked. Do you know the reason behind this change? Having a nullable array makes life harder for JS developers and I'm not sure why having an empty array is not enough. 890311 6 haraken 2013-05-17 05:32:16 -0700 (In reply to comment #5) > This is visibly a recent spec change as it wasn't nullable before. I don't know how other browsers behave so this would need to be checked. Do you know the reason behind this change? Having a nullable array makes life harder for JS developers and I'm not sure why having an empty array is not enough. Yeah, needs verification of other browsers. I don't know about nullable, but replacing XXXArray with XXX[] looks like a recent trend. 890466 7 202062 ap 2013-05-17 09:54:27 -0700 Comment on attachment 202062 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202062&action=review > Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp:182 > + JSValue result = impl->ports() ? jsArray(exec, castedThis->globalObject(), *impl->ports()) : constructEmptyArray(exec, 0, castedThis->globalObject()); It seems unfortunate that we call into DOM twice now. Why can't the result of impl->ports() be cached? 890496 8 cdumez 2013-05-17 10:07:17 -0700 (In reply to comment #7) > (From update of attachment 202062 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202062&action=review > > > Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp:182 > > + JSValue result = impl->ports() ? jsArray(exec, castedThis->globalObject(), *impl->ports()) : constructEmptyArray(exec, 0, castedThis->globalObject()); > > It seems unfortunate that we call into DOM twice now. Why can't the result of impl->ports() be cached? I don't think it is a big issue currently because this code is only used for MessageEvent::ports() currently, which is inlined. However, you have a point that we should cache it to be safe. I will look into it, thanks. 892696 9 cdumez 2013-05-23 01:00:44 -0700 Addressing ap and Kentaro's feedback in Bug 116653. 202062 2013-05-17 04:57:26 -0700 2013-05-17 09:54:27 -0700 Patch 116304_MessageEvent_ports.patch text/plain 4899 cdumez ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAyZjBhYzQ5Li4yNDQyNGZjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjYg QEAKKzIwMTMtMDUtMTcgIENocmlzdG9waGUgRHVtZXogIDxjaC5kdW1lekBzaXNhLnNhbXN1bmcu Y29tPgorCisgICAgICAgIFJlbW92ZSBjdXN0b20gY29kZSBmb3IgTWVzc2FnZUV2ZW50LnBvcnRz IGdldHRlcgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTE2MzA0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg UmVtb3ZlIEN1c3RvbSBjb2RlIGZvciB0aGUgInBvcnRzIiBhdHRyaWJ1dGUgZ2V0dGVyIGluIE1l c3NhZ2VFdmVudAorICAgICAgICBJREwgaW50ZXJmYWNlLiBUaGUgSlNDIGJpbmRpbmdzIGdlbmVy YXRvciBhbHJlYWR5IHN1cHBvcnRzIGF0dHJpYnV0ZXMKKyAgICAgICAgb2YgdHlwZSBNZXNzYWdl UG9ydEFycmF5LgorCisgICAgICAgIE5vIG5ldyB0ZXN0cywgYWxyZWFkeSBjb3ZlcmVkIGJ5IGV4 aXN0aW5nIGxheW91dCB0ZXN0cyBhbmQgYmluZGluZ3MKKyAgICAgICAgdGVzdHMuCisKKyAgICAg ICAgKiBiaW5kaW5ncy9qcy9KU01lc3NhZ2VFdmVudEN1c3RvbS5jcHA6IFJlbW92ZSBDdXN0b20g Y29kZSBmb3IgcG9ydHMgZ2V0dGVyLgorICAgICAgICAqIGJpbmRpbmdzL3NjcmlwdHMvQ29kZUdl bmVyYXRvckpTLnBtOgorICAgICAgICAoTmF0aXZlVG9KU1ZhbHVlKTogU21hbGwgdHdlYWsgdG8g Y29ycmVjdGx5IGhhbmRsZSB0aGUgY2FzZSB3aGVyZSBpbXBsLT5wb3J0cygpCisgICAgICAgIHJl dHVybnMgTlVMTCBhbmQgYXZvaWQgY3Jhc2hpbmcgaW4gdGhpcyBjYXNlLgorICAgICAgICAqIGJp bmRpbmdzL3NjcmlwdHMvdGVzdC9KUy9KU1Rlc3RTZXJpYWxpemVkU2NyaXB0VmFsdWVJbnRlcmZh Y2UuY3BwOiBSZWJhc2VsaW5lIGR1ZQorICAgICAgICB0byBiaW5kaW5ncyBnZW5lcmF0b3IgdHdl YWsuCisgICAgICAgICogZG9tL01lc3NhZ2VFdmVudC5pZGw6IFJlbW92ZSBbQ3VzdG9tR2V0dGVy XSBleHRlbmRlZCBhdHRyaWJ1dGUgZm9yICdwb3J0cycgYXR0cmlidXRlCisgICAgICAgIGFuZCBz d2l0Y2ggdG8gTWVzc2FnZVBvcnRBcnJheSB0eXBlLgorCiAyMDEzLTA1LTE2ICBKZXN1cyBTYW5j aGV6LVBhbGVuY2lhICA8amVzdXMucGFsZW5jaWFAb3BlbmJvc3NhLm9yZz4KIAogICAgICAgICBB ZGQgc3R1YnMgZm9yIENvbnRleHRNZW51IGFuZCBDb250ZXh0TWVudUl0ZW0KZGlmZiAtLWdpdCBh L1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL2pzL0pTTWVzc2FnZUV2ZW50Q3VzdG9tLmNwcCBiL1Nv dXJjZS9XZWJDb3JlL2JpbmRpbmdzL2pzL0pTTWVzc2FnZUV2ZW50Q3VzdG9tLmNwcAppbmRleCA3 OWJiMjI1Li4zOGFlODI3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9qcy9K U01lc3NhZ2VFdmVudEN1c3RvbS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMv SlNNZXNzYWdlRXZlbnRDdXN0b20uY3BwCkBAIC04OCwxOCArODgsNiBAQCBKU1ZhbHVlIEpTTWVz c2FnZUV2ZW50OjpkYXRhKEV4ZWNTdGF0ZSogZXhlYykgY29uc3QKICAgICByZXR1cm4gcmVzdWx0 OwogfQogCi1KU1ZhbHVlIEpTTWVzc2FnZUV2ZW50Ojpwb3J0cyhFeGVjU3RhdGUqIGV4ZWMpIGNv bnN0Ci17Ci0gICAgTWVzc2FnZVBvcnRBcnJheSogcG9ydHMgPSBzdGF0aWNfY2FzdDxNZXNzYWdl RXZlbnQqPihpbXBsKCkpLT5wb3J0cygpOwotICAgIGlmICghcG9ydHMpCi0gICAgICAgIHJldHVy biBjb25zdHJ1Y3RFbXB0eUFycmF5KGV4ZWMsIDAsIGdsb2JhbE9iamVjdCgpKTsKLQotICAgIE1h cmtlZEFyZ3VtZW50QnVmZmVyIGxpc3Q7Ci0gICAgZm9yIChzaXplX3QgaSA9IDA7IGkgPCBwb3J0 cy0+c2l6ZSgpOyBpKyspCi0gICAgICAgIGxpc3QuYXBwZW5kKHRvSlMoZXhlYywgZ2xvYmFsT2Jq ZWN0KCksICgqcG9ydHMpW2ldLmdldCgpKSk7Ci0gICAgcmV0dXJuIGNvbnN0cnVjdEFycmF5KGV4 ZWMsIDAsIGdsb2JhbE9iamVjdCgpLCBsaXN0KTsKLX0KLQogc3RhdGljIEpTQzo6SlNWYWx1ZSBo YW5kbGVJbml0TWVzc2FnZUV2ZW50KEpTTWVzc2FnZUV2ZW50KiBqc0V2ZW50LCBKU0M6OkV4ZWNT dGF0ZSogZXhlYykKIHsKICAgICBjb25zdCBTdHJpbmcmIHR5cGVBcmcgPSBleGVjLT5hcmd1bWVu dCgwKS50b1N0cmluZyhleGVjKS0+dmFsdWUoZXhlYyk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2Vi Q29yZS9iaW5kaW5ncy9zY3JpcHRzL0NvZGVHZW5lcmF0b3JKUy5wbSBiL1NvdXJjZS9XZWJDb3Jl L2JpbmRpbmdzL3NjcmlwdHMvQ29kZUdlbmVyYXRvckpTLnBtCmluZGV4IGFjYTlkOGMuLmQyNTYx ZjYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3NjcmlwdHMvQ29kZUdlbmVy YXRvckpTLnBtCisrKyBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3NjcmlwdHMvQ29kZUdlbmVy YXRvckpTLnBtCkBAIC0zNTU3LDcgKzM1NTcsNyBAQCBzdWIgTmF0aXZlVG9KU1ZhbHVlCiAgICAg ICAgIEFkZFRvSW1wbEluY2x1ZGVzKCJNZXNzYWdlUG9ydC5oIiwgJGNvbmRpdGlvbmFsKTsKICAg ICAgICAgQWRkVG9JbXBsSW5jbHVkZXMoIkpTTWVzc2FnZVBvcnQuaCIsICRjb25kaXRpb25hbCk7 CiAgICAgICAgIEFkZFRvSW1wbEluY2x1ZGVzKCI8cnVudGltZS9KU0FycmF5Lmg+IiwgJGNvbmRp dGlvbmFsKTsKLSAgICAgICAgcmV0dXJuICJqc0FycmF5KGV4ZWMsICRnbG9iYWxPYmplY3QsICok dmFsdWUpIjsKKyAgICAgICAgcmV0dXJuICIkdmFsdWUgPyBqc0FycmF5KGV4ZWMsICRnbG9iYWxP YmplY3QsICokdmFsdWUpIDogY29uc3RydWN0RW1wdHlBcnJheShleGVjLCAwLCBjYXN0ZWRUaGlz LT5nbG9iYWxPYmplY3QoKSkiOwogICAgIH0gZWxzZSB7CiAgICAgICAgICMgRGVmYXVsdCwgaW5j bHVkZSBoZWFkZXIgd2l0aCBzYW1lIG5hbWUuCiAgICAgICAgIEFkZFRvSW1wbEluY2x1ZGVzKCJK UyR0eXBlLmgiLCAkY29uZGl0aW9uYWwpOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvYmlu ZGluZ3Mvc2NyaXB0cy90ZXN0L0pTL0pTVGVzdFNlcmlhbGl6ZWRTY3JpcHRWYWx1ZUludGVyZmFj ZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9zY3JpcHRzL3Rlc3QvSlMvSlNUZXN0U2Vy aWFsaXplZFNjcmlwdFZhbHVlSW50ZXJmYWNlLmNwcAppbmRleCAzNGI0NGRmLi44MGVmZjhiIDEw MDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9zY3JpcHRzL3Rlc3QvSlMvSlNUZXN0 U2VyaWFsaXplZFNjcmlwdFZhbHVlSW50ZXJmYWNlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9i aW5kaW5ncy9zY3JpcHRzL3Rlc3QvSlMvSlNUZXN0U2VyaWFsaXplZFNjcmlwdFZhbHVlSW50ZXJm YWNlLmNwcApAQCAtMTc5LDcgKzE3OSw3IEBAIEpTVmFsdWUganNUZXN0U2VyaWFsaXplZFNjcmlw dFZhbHVlSW50ZXJmYWNlUG9ydHMoRXhlY1N0YXRlKiBleGVjLCBKU1ZhbHVlIHNsb3RCCiAgICAg SlNUZXN0U2VyaWFsaXplZFNjcmlwdFZhbHVlSW50ZXJmYWNlKiBjYXN0ZWRUaGlzID0ganNDYXN0 PEpTVGVzdFNlcmlhbGl6ZWRTY3JpcHRWYWx1ZUludGVyZmFjZSo+KGFzT2JqZWN0KHNsb3RCYXNl KSk7CiAgICAgVU5VU0VEX1BBUkFNKGV4ZWMpOwogICAgIFRlc3RTZXJpYWxpemVkU2NyaXB0VmFs dWVJbnRlcmZhY2UqIGltcGwgPSBzdGF0aWNfY2FzdDxUZXN0U2VyaWFsaXplZFNjcmlwdFZhbHVl SW50ZXJmYWNlKj4oY2FzdGVkVGhpcy0+aW1wbCgpKTsKLSAgICBKU1ZhbHVlIHJlc3VsdCA9IGpz QXJyYXkoZXhlYywgY2FzdGVkVGhpcy0+Z2xvYmFsT2JqZWN0KCksICppbXBsLT5wb3J0cygpKTsK KyAgICBKU1ZhbHVlIHJlc3VsdCA9IGltcGwtPnBvcnRzKCkgPyBqc0FycmF5KGV4ZWMsIGNhc3Rl ZFRoaXMtPmdsb2JhbE9iamVjdCgpLCAqaW1wbC0+cG9ydHMoKSkgOiBjb25zdHJ1Y3RFbXB0eUFy cmF5KGV4ZWMsIDAsIGNhc3RlZFRoaXMtPmdsb2JhbE9iamVjdCgpKTsKICAgICByZXR1cm4gcmVz dWx0OwogfQogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9kb20vTWVzc2FnZUV2ZW50Lmlk bCBiL1NvdXJjZS9XZWJDb3JlL2RvbS9NZXNzYWdlRXZlbnQuaWRsCmluZGV4IDBhZjcyZDUuLmMy NTIwMmQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2RvbS9NZXNzYWdlRXZlbnQuaWRsCisr KyBiL1NvdXJjZS9XZWJDb3JlL2RvbS9NZXNzYWdlRXZlbnQuaWRsCkBAIC0zNCw3ICszNCw3IEBA CiAgICAgW0luaXRpYWxpemVkQnlFdmVudENvbnN0cnVjdG9yXSByZWFkb25seSBhdHRyaWJ1dGUg RE9NV2luZG93IHNvdXJjZTsKICNpZiBkZWZpbmVkKExBTkdVQUdFX0pBVkFTQ1JJUFQpICYmIExB TkdVQUdFX0pBVkFTQ1JJUFQKICAgICBbSW5pdGlhbGl6ZWRCeUV2ZW50Q29uc3RydWN0b3IsIENh Y2hlZEF0dHJpYnV0ZSwgQ3VzdG9tR2V0dGVyXSByZWFkb25seSBhdHRyaWJ1dGUgYW55IGRhdGE7 Ci0gICAgW0luaXRpYWxpemVkQnlFdmVudENvbnN0cnVjdG9yLCBDdXN0b21HZXR0ZXJdIHJlYWRv bmx5IGF0dHJpYnV0ZSBBcnJheSBwb3J0czsKKyAgICBbSW5pdGlhbGl6ZWRCeUV2ZW50Q29uc3Ry dWN0b3JdIHJlYWRvbmx5IGF0dHJpYnV0ZSBNZXNzYWdlUG9ydEFycmF5IHBvcnRzOwogCiAgICAg W0N1c3RvbV0gdm9pZCBpbml0TWVzc2FnZUV2ZW50KFtEZWZhdWx0PVVuZGVmaW5lZF0gb3B0aW9u YWwgRE9NU3RyaW5nIHR5cGVBcmcsIAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICBbRGVmYXVsdD1VbmRlZmluZWRdIG9wdGlvbmFsIGJvb2xlYW4gY2FuQnViYmxlQXJnLCAK