159542 2016-07-07 20:11:43 -0700 We may add a ReadOnly property without setting the corresponding bit on Structure 2016-07-08 18:30:40 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Local Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 saam saam benjamin commit-queue fpizlo ggaren gskachkov keith_miller mark.lam msaboff oliver sukolsak webkit-bug-importer ysuzuki oldest_to_newest 1208947 0 saam 2016-07-07 20:11:43 -0700 ... 1208948 1 saam 2016-07-07 20:13:06 -0700 <rdar://problem/27084591> 1209350 2 283228 saam 2016-07-08 17:21:07 -0700 Created attachment 283228 patch 1209351 3 283228 saam 2016-07-08 17:22:08 -0700 Comment on attachment 283228 patch View in context: https://bugs.webkit.org/attachment.cgi?id=283228&action=review > Source/JavaScriptCore/runtime/JSObject.h:1569 > + if (attributes & ReadOnly) > + structure->setContainsReadOnlyProperties(); I put this here to be consistent with JSObject::putDirectInternal, but I almost feel like putting it inside Structure::addPropertyWithoutTransition is a more natural place for it. 1209364 4 283228 benjamin 2016-07-08 17:29:21 -0700 Comment on attachment 283228 patch View in context: https://bugs.webkit.org/attachment.cgi?id=283228&action=review >> Source/JavaScriptCore/runtime/JSObject.h:1569 >> + structure->setContainsReadOnlyProperties(); > > I put this here to be consistent with JSObject::putDirectInternal, > but I almost feel like putting it inside Structure::addPropertyWithoutTransition > is a more natural place for it. I think that makes sense in addPropertyWithoutTransition() for both call sites. What's wrong with that? 1209369 5 283228 saam 2016-07-08 17:33:50 -0700 Comment on attachment 283228 patch View in context: https://bugs.webkit.org/attachment.cgi?id=283228&action=review >>> Source/JavaScriptCore/runtime/JSObject.h:1569 >>> + structure->setContainsReadOnlyProperties(); >> >> I put this here to be consistent with JSObject::putDirectInternal, >> but I almost feel like putting it inside Structure::addPropertyWithoutTransition >> is a more natural place for it. > > I think that makes sense in addPropertyWithoutTransition() for both call sites. What's wrong with that? I agree. I'm going to open another bug to consider this. I think we should also consider the Structure::addNewPropertyTransition to have it mark the bit on the new transition it's returning. 1209375 6 saam 2016-07-08 18:09:59 -0700 I opened: https://bugs.webkit.org/show_bug.cgi?id=159593 1209380 7 283228 commit-queue 2016-07-08 18:30:35 -0700 Comment on attachment 283228 patch Clearing flags on attachment: 283228 Committed r203015: <http://trac.webkit.org/changeset/203015> 1209381 8 commit-queue 2016-07-08 18:30:40 -0700 All reviewed patches have been landed. Closing bug. 283228 2016-07-08 17:21:07 -0700 2016-07-08 18:30:35 -0700 patch a-backup.diff text/plain 3092 saam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjAzMDA5KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIzIEBA CisyMDE2LTA3LTA4ICBTYWFtIEJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgorCisgICAgICAg IFdlIG1heSBhZGQgYSBSZWFkT25seSBwcm9wZXJ0eSB3aXRob3V0IHNldHRpbmcgdGhlIGNvcnJl c3BvbmRpbmcgYml0IG9uIFN0cnVjdHVyZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MTU5NTQyCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8yNzA4NDU5 MT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUg cmVhc29uIHRoaXMgdXN1YWxseSBpcyBPSyBpcyBkdWUgdG8gaGFwcGVuc3RhbmNlLiBPZnRlbiwg aW5zdGFuY2VzIHRoYXQgcHV0RGlyZWN0V2l0aG91dFRyYW5zaXRpb24KKyAgICAgICAgYWxzbyBo YXBwZW4gdG8gaGF2ZSBhIHN0YXRpYyBwcm9wZXJ0eSB0YWJsZS4gSGF2aW5nIGEgc3RhdGljIHBy b3BlcnR5IHRhYmxlIGNhdXNlcyB0aGUKKyAgICAgICAgSGFzUmVhZE9ubHlPckdldHRlclNldHRl clByb3BlcnRpZXNFeGNsdWRpbmdQcm90byBvbiB0aGUgc3RydWN0dXJlIHRvIGJlIHNldC4gSG93 ZXZlciwgCisgICAgICAgIHRoZXJlIGFyZSB0aW1lcyB3aGVyZSBhbiBvYmplY3QgY2FsbHMgcHV0 RGlyZWN0V2l0aG91dFRyYW5zaXRpb24sIGFuZCBpdCBkb2Vzbid0IGhhdmUgYQorICAgICAgICBz dGF0aWMgcHJvcGVydHkgaGFzaCB0YWJsZS4gVGhlIGZpeCBpcyBzaW1wbGUsIHB1dERpcmVjdFdp dGhUcmFuc2l0aW9uIG5lZWRzIHRvIHNldCB0aGUKKyAgICAgICAgSGFzUmVhZE9ubHlPckdldHRl clNldHRlclByb3BlcnRpZXNFeGNsdWRpbmdQcm90byBpZiBpdCBwdXRzIGEgUmVhZE9ubHkgcHJv cGVydHkuCisKKyAgICAgICAgKiBydW50aW1lL0pTT2JqZWN0Lmg6CisgICAgICAgIChKU0M6OkpT T2JqZWN0OjpwdXREaXJlY3RXaXRob3V0VHJhbnNpdGlvbik6CisgICAgICAgICogdGVzdHMvc3Ry ZXNzL3Byb3Blci1wcm9wZXJ0eS1zdG9yZS13aXRoLXByb3RvdHlwZS1wcm9wZXJ0eS10aGF0LWlz LW5vdC13cml0YWJsZS5qczogQWRkZWQuCisgICAgICAgIChhc3NlcnQpOgorCiAyMDE2LTA3LTA4 ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KIAogICAgICAgICBNb3ZlIENhbGxGcmFt ZSBoZWFkZXIgaW5mbyBmcm9tIEpTU3RhY2suaCB0byBDYWxsRnJhbWUuaApJbmRleDogU291cmNl L0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNPYmplY3QuaAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvcnVudGltZS9KU09iamVjdC5oCShyZXZpc2lvbiAyMDI5MjcpCisrKyBT b3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU09iamVjdC5oCSh3b3JraW5nIGNvcHkpCkBA IC0xNTY1LDYgKzE1NjUsOCBAQCBpbmxpbmUgdm9pZCBKU09iamVjdDo6cHV0RGlyZWN0V2l0aG91 dFRyCiAgICAgICAgIG5ld0J1dHRlcmZseSA9IGdyb3dPdXRPZkxpbmVTdG9yYWdlKHZtLCBzdHJ1 Y3R1cmUoKS0+b3V0T2ZMaW5lQ2FwYWNpdHkoKSwgc3RydWN0dXJlKCktPnN1Z2dlc3RlZE5ld091 dE9mTGluZVN0b3JhZ2VDYXBhY2l0eSgpKTsKICAgICBTdHJ1Y3R1cmUqIHN0cnVjdHVyZSA9IHRo aXMtPnN0cnVjdHVyZSgpOwogICAgIFByb3BlcnR5T2Zmc2V0IG9mZnNldCA9IHN0cnVjdHVyZS0+ YWRkUHJvcGVydHlXaXRob3V0VHJhbnNpdGlvbih2bSwgcHJvcGVydHlOYW1lLCBhdHRyaWJ1dGVz KTsKKyAgICBpZiAoYXR0cmlidXRlcyAmIFJlYWRPbmx5KQorICAgICAgICBzdHJ1Y3R1cmUtPnNl dENvbnRhaW5zUmVhZE9ubHlQcm9wZXJ0aWVzKCk7CiAgICAgYm9vbCBzaG91bGRPcHRpbWl6ZSA9 IGZhbHNlOwogICAgIHN0cnVjdHVyZS0+d2lsbFN0b3JlVmFsdWVGb3JOZXdUcmFuc2l0aW9uKHZt LCBwcm9wZXJ0eU5hbWUsIHZhbHVlLCBzaG91bGRPcHRpbWl6ZSk7CiAgICAgc2V0U3RydWN0dXJl QW5kQnV0dGVyZmx5KHZtLCBzdHJ1Y3R1cmUsIG5ld0J1dHRlcmZseSk7CkluZGV4OiBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvdGVzdHMvc3RyZXNzL3Byb3Blci1wcm9wZXJ0eS1zdG9yZS13aXRoLXBy b3RvdHlwZS1wcm9wZXJ0eS10aGF0LWlzLW5vdC13cml0YWJsZS5qcwo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBT b3VyY2UvSmF2YVNjcmlwdENvcmUvdGVzdHMvc3RyZXNzL3Byb3Blci1wcm9wZXJ0eS1zdG9yZS13 aXRoLXByb3RvdHlwZS1wcm9wZXJ0eS10aGF0LWlzLW5vdC13cml0YWJsZS5qcwkocmV2aXNpb24g MCkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS90ZXN0cy9zdHJlc3MvcHJvcGVyLXByb3BlcnR5 LXN0b3JlLXdpdGgtcHJvdG90eXBlLXByb3BlcnR5LXRoYXQtaXMtbm90LXdyaXRhYmxlLmpzCSh3 b3JraW5nIGNvcHkpCkBAIC0wLDAgKzEsMTMgQEAKK2Z1bmN0aW9uIGFzc2VydChiKSB7CisgICAg aWYgKCFiKQorICAgICAgICB0aHJvdyBuZXcgRXJyb3IoIkJhZCBhc3NlcnRpb24uIik7Cit9CisK K2xldCB4ID0gKG5ldyBTZXQpW1N5bWJvbC5pdGVyYXRvcl0oKTsKK2Fzc2VydCh4W1N5bWJvbC50 b1N0cmluZ1RhZ10gPT09ICJTZXQgSXRlcmF0b3IiKTsKKworbGV0IHkgPSB7X19wcm90b19fOiB4 fTsKK2Fzc2VydCh5W1N5bWJvbC50b1N0cmluZ1RhZ10gPT09ICJTZXQgSXRlcmF0b3IiKTsKK3lb U3ltYm9sLnRvU3RyaW5nVGFnXSA9IDI1OworYXNzZXJ0KHlbU3ltYm9sLnRvU3RyaW5nVGFnXSA9 PT0gIlNldCBJdGVyYXRvciIpOworYXNzZXJ0KHhbU3ltYm9sLnRvU3RyaW5nVGFnXSA9PT0gIlNl dCBJdGVyYXRvciIpOwo=