111487 2013-03-05 15:05:37 -0800 Objective-C API: JSValue should implement init and return nil in exceptional cases 2013-03-05 16:18:40 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mhahnenberg mhahnenberg webkit.review.bot oldest_to_newest 848124 0 mhahnenberg 2013-03-05 15:05:37 -0800 In keeping with good Objective-C practice, JSValue should override init and throw an exception since there's not really a way to put the returned object in a coherent state that won't crash at some point later in time. Better to fail fast. 848129 1 191576 mhahnenberg 2013-03-05 15:07:49 -0800 Created attachment 191576 patch 848130 2 mhahnenberg 2013-03-05 15:09:11 -0800 <rdar://problem/13351901> 848132 3 191576 darin 2013-03-05 15:12:54 -0800 Comment on attachment 191576 patch View in context: https://bugs.webkit.org/attachment.cgi?id=191576&action=review > Source/JavaScriptCore/API/JSValue.mm:955 > + @throw [NSException exceptionWithName:@"InvalidMessageException" > + reason:@"JSValue cannot safely support init." > + userInfo:nil]; Why throw? Just return nil. And the initWithValue method should also return nil when the arguments are nil. That’s how Cocoa init functions normally work. Unless I’m missing some compelling argument for throwing exceptions instead. Please don’t align the lines like this. WebKit coding style says just indent 4 spaces. 848144 4 191579 mhahnenberg 2013-03-05 15:20:00 -0800 Created attachment 191579 return nil 848146 5 191579 mhahnenberg 2013-03-05 15:21:10 -0800 Comment on attachment 191579 return nil Need to update ChangeLog. 848151 6 191582 mhahnenberg 2013-03-05 15:24:19 -0800 Created attachment 191582 updated change log 848213 7 191582 webkit.review.bot 2013-03-05 16:18:37 -0800 Comment on attachment 191582 updated change log Clearing flags on attachment: 191582 Committed r144843: <http://trac.webkit.org/changeset/144843> 848214 8 webkit.review.bot 2013-03-05 16:18:40 -0800 All reviewed patches have been landed. Closing bug. 191576 2013-03-05 15:07:49 -0800 2013-03-05 15:20:00 -0800 patch the.patch text/plain 1545 mhahnenberg SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTQ0ODI2KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBA CisyMDEzLTAzLTA1ICBNYXJrIEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CisK KyAgICAgICAgT2JqZWN0aXZlLUMgQVBJOiBKU1ZhbHVlIHNob3VsZCBpbXBsZW1lbnQgaW5pdCBh bmQgdGhyb3cgYW4gZXhjZXB0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0xMTE0ODcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBJbiBrZWVwaW5nIHdpdGggZ29vZCBPYmplY3RpdmUtQyBwcmFjdGljZSwg SlNWYWx1ZSBzaG91bGQgb3ZlcnJpZGUgaW5pdCBhbmQgdGhyb3cgYW4gZXhjZXB0aW9uIHNpbmNl IAorICAgICAgICB0aGVyZSdzIG5vdCByZWFsbHkgYSB3YXkgdG8gcHV0IHRoZSByZXR1cm5lZCBv YmplY3QgaW4gYSBjb2hlcmVudCBzdGF0ZSB0aGF0IHdvbid0IGNyYXNoIGF0IHNvbWUgcG9pbnQg CisgICAgICAgIGxhdGVyIGluIHRpbWUuIEJldHRlciB0byBmYWlsIGZhc3QuCisKKyAgICAgICAg KiBBUEkvSlNWYWx1ZS5tbToKKyAgICAgICAgKC1bSlNWYWx1ZSBpbml0XSk6CisKIDIwMTMtMDMt MDUgIFNoZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5ib3RAZ21haWwuY29tPgogCiAgICAgICAg IFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHIxNDQ3MDguCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlw dENvcmUvQVBJL0pTVmFsdWUubW0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3Jl L0FQSS9KU1ZhbHVlLm1tCShyZXZpc2lvbiAxNDQ4MjQpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENv cmUvQVBJL0pTVmFsdWUubW0JKHdvcmtpbmcgY29weSkKQEAgLTk0OCw2ICs5NDgsMTMgQEAgKyAo SlNWYWx1ZSAqKXZhbHVlV2l0aFZhbHVlOihKU1ZhbHVlUmVmKQogICAgIHJldHVybiBbY29udGV4 dCB3cmFwcGVyRm9ySlNPYmplY3Q6dmFsdWVdOwogfQogCistIChKU1ZhbHVlICopaW5pdAorewor ICAgIEB0aHJvdyBbTlNFeGNlcHRpb24gZXhjZXB0aW9uV2l0aE5hbWU6QCJJbnZhbGlkTWVzc2Fn ZUV4Y2VwdGlvbiIKKyAgICAgICAgICAgICAgICAgICAgICAgIHJlYXNvbjpAIkpTVmFsdWUgY2Fu bm90IHNhZmVseSBzdXBwb3J0IGluaXQuIgorICAgICAgICAgICAgICAgICAgICAgICAgdXNlcklu Zm86bmlsXTsKK30KKwogLSAoSlNWYWx1ZSAqKWluaXRXaXRoVmFsdWU6KEpTVmFsdWVSZWYpdmFs dWUgaW5Db250ZXh0OihKU0NvbnRleHQgKiljb250ZXh0CiB7CiAgICAgc2VsZiA9IFtzdXBlciBp bml0XTsK 191579 2013-03-05 15:20:00 -0800 2013-03-05 15:24:19 -0800 return nil the.patch text/plain 1608 mhahnenberg SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTQ0ODI2KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBA CisyMDEzLTAzLTA1ICBNYXJrIEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CisK KyAgICAgICAgT2JqZWN0aXZlLUMgQVBJOiBKU1ZhbHVlIHNob3VsZCBpbXBsZW1lbnQgaW5pdCBh bmQgdGhyb3cgYW4gZXhjZXB0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0xMTE0ODcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBJbiBrZWVwaW5nIHdpdGggZ29vZCBPYmplY3RpdmUtQyBwcmFjdGljZSwg SlNWYWx1ZSBzaG91bGQgb3ZlcnJpZGUgaW5pdCBhbmQgdGhyb3cgYW4gZXhjZXB0aW9uIHNpbmNl IAorICAgICAgICB0aGVyZSdzIG5vdCByZWFsbHkgYSB3YXkgdG8gcHV0IHRoZSByZXR1cm5lZCBv YmplY3QgaW4gYSBjb2hlcmVudCBzdGF0ZSB0aGF0IHdvbid0IGNyYXNoIGF0IHNvbWUgcG9pbnQg CisgICAgICAgIGxhdGVyIGluIHRpbWUuIEJldHRlciB0byBmYWlsIGZhc3QuCisKKyAgICAgICAg KiBBUEkvSlNWYWx1ZS5tbToKKyAgICAgICAgKC1bSlNWYWx1ZSBpbml0XSk6CisKIDIwMTMtMDMt MDUgIFNoZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5ib3RAZ21haWwuY29tPgogCiAgICAgICAg IFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHIxNDQ3MDguCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlw dENvcmUvQVBJL0pTVmFsdWUubW0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3Jl L0FQSS9KU1ZhbHVlLm1tCShyZXZpc2lvbiAxNDQ4MjQpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENv cmUvQVBJL0pTVmFsdWUubW0JKHdvcmtpbmcgY29weSkKQEAgLTk0OCwxMyArOTQ4LDIwIEBAICsg KEpTVmFsdWUgKil2YWx1ZVdpdGhWYWx1ZTooSlNWYWx1ZVJlZikKICAgICByZXR1cm4gW2NvbnRl eHQgd3JhcHBlckZvckpTT2JqZWN0OnZhbHVlXTsKIH0KIAorLSAoSlNWYWx1ZSAqKWluaXQKK3sK KyAgICByZXR1cm4gbmlsOworfQorCiAtIChKU1ZhbHVlICopaW5pdFdpdGhWYWx1ZTooSlNWYWx1 ZVJlZil2YWx1ZSBpbkNvbnRleHQ6KEpTQ29udGV4dCAqKWNvbnRleHQKIHsKKyAgICBpZiAoIXZh bHVlIHx8ICFjb250ZXh0KQorICAgICAgICByZXR1cm4gbmlsOworCiAgICAgc2VsZiA9IFtzdXBl ciBpbml0XTsKICAgICBpZiAoIXNlbGYpCiAgICAgICAgIHJldHVybiBuaWw7CiAKLSAgICBBU1NF UlQodmFsdWUpOwogICAgIF9jb250ZXh0ID0gW2NvbnRleHQgcmV0YWluXTsKICAgICBtX3ZhbHVl ID0gdmFsdWU7CiAgICAgSlNWYWx1ZVByb3RlY3QoW19jb250ZXh0IGdsb2JhbENvbnRleHRSZWZd LCBtX3ZhbHVlKTsK 191582 2013-03-05 15:24:19 -0800 2013-03-05 16:18:37 -0800 updated change log the.patch text/plain 1588 mhahnenberg SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTQ0ODI3KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBA CisyMDEzLTAzLTA1ICBNYXJrIEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CisK KyAgICAgICAgT2JqZWN0aXZlLUMgQVBJOiBKU1ZhbHVlIHNob3VsZCBpbXBsZW1lbnQgaW5pdCBh bmQgcmV0dXJuIG5pbCBpbiBleGNlcHRpb25hbCBjYXNlcworICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTExNDg3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBBUEkvSlNWYWx1ZS5tbToKKyAgICAgICAgKC1b SlNWYWx1ZSBpbml0XSk6IFdlIHJldHVybiBuaWwgaGVyZSBiZWNhdXNlIHRoZXJlIGlzIG5vIHdh eSB0byBnZXQgdGhlIGluc3RhbmNlIGludG8gYSBjb2hlcmVudCBzdGF0ZQorICAgICAgICB3aXRo b3V0IGEgSlNDb250ZXh0LgorICAgICAgICAoLVtKU1ZhbHVlIGluaXRXaXRoVmFsdWU6aW5Db250 ZXh0Ol0pOiBTaW1pbGFybHksIHdlIHNob3VsZCBhbHNvIHJldHVybiBuaWwgaGVyZSBpZiBlaXRo ZXIgb2YgdGhlIGFyZ3VtZW50cyBpcyAwLgorCiAyMDEzLTAzLTA1ICBTaGVyaWZmIEJvdCAgPHdl YmtpdC5yZXZpZXcuYm90QGdtYWlsLmNvbT4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xsaW5n IG91dCByMTQ0NzA4LgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL0FQSS9KU1ZhbHVlLm1t Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9BUEkvSlNWYWx1ZS5tbQkocmV2 aXNpb24gMTQ0ODI0KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL0FQSS9KU1ZhbHVlLm1tCSh3 b3JraW5nIGNvcHkpCkBAIC05NDgsMTMgKzk0OCwyMCBAQCArIChKU1ZhbHVlICopdmFsdWVXaXRo VmFsdWU6KEpTVmFsdWVSZWYpCiAgICAgcmV0dXJuIFtjb250ZXh0IHdyYXBwZXJGb3JKU09iamVj dDp2YWx1ZV07CiB9CiAKKy0gKEpTVmFsdWUgKilpbml0Cit7CisgICAgcmV0dXJuIG5pbDsKK30K KwogLSAoSlNWYWx1ZSAqKWluaXRXaXRoVmFsdWU6KEpTVmFsdWVSZWYpdmFsdWUgaW5Db250ZXh0 OihKU0NvbnRleHQgKiljb250ZXh0CiB7CisgICAgaWYgKCF2YWx1ZSB8fCAhY29udGV4dCkKKyAg ICAgICAgcmV0dXJuIG5pbDsKKwogICAgIHNlbGYgPSBbc3VwZXIgaW5pdF07CiAgICAgaWYgKCFz ZWxmKQogICAgICAgICByZXR1cm4gbmlsOwogCi0gICAgQVNTRVJUKHZhbHVlKTsKICAgICBfY29u dGV4dCA9IFtjb250ZXh0IHJldGFpbl07CiAgICAgbV92YWx1ZSA9IHZhbHVlOwogICAgIEpTVmFs dWVQcm90ZWN0KFtfY29udGV4dCBnbG9iYWxDb250ZXh0UmVmXSwgbV92YWx1ZSk7Cg==