187542 2018-07-10 17:33:27 -0700 Web Inspector: console.log fires getters for deep properties 2018-08-13 10:46:59 -0700 1 1 1 Unclassified WebKit Web Inspector Safari 11 All All RESOLVED FIXED InRadar P2 Normal --- 1 kierkegaurd joepeck bburg commit-queue ews-watchlist inspector-bugzilla-changes joepeck keith_miller mark.lam msaboff saam webkit-bug-importer oldest_to_newest 1441039 0 kierkegaurd 2018-07-10 17:33:27 -0700 With the following code the "ref" getter will be called. This is inconsistent with Firefox and Chrome, which do not fire the getter. ``` let inner = {}; Object.defineProperty(inner, "ref", { get: () => console.log("getter fired"), }); let outer = { inner }; console.log(outer); ``` This came up in the React repo: https://github.com/facebook/react/issues/13179 1447382 1 webkit-bug-importer 2018-08-02 14:25:38 -0700 <rdar://problem/42873158> 1447429 2 joepeck 2018-08-02 16:18:48 -0700 Interesting that this happens when logging `outer` but not `inner`. Seems then it might be an accident in the property preview path. 1449392 3 joepeck 2018-08-10 15:07:27 -0700 Switching to `console.log` to `console.trace` gives us: [Log] Trace: getter fired Console Evaluation (Console Evaluation 1:6) _isPreviewableObjectInternal _appendPropertyPreviews _generatePreview RemoteObject create log Console Evaluation (Console Evaluation 1:13) evaluateWithScopeExtension _evaluateOn _evaluateAndWrap It looks like at the bottom of _isPreviewableObjectInternal we access properties `object[propertyName]` without regard to whether or not it may be a getter. _isPreviewableObjectInternal(object, knownObjects, depth) { ... // Objects are simple if they have 3 or less simple properties. let ownPropertyNames = Object.getOwnPropertyNames(object); if (ownPropertyNames.length > 3) return false; for (let i = 0; i < ownPropertyNames.length; ++i) { let propertyName = ownPropertyNames[i]; if (!this._isPreviewableObjectInternal(object[propertyName], knownObjects, depth)) return false; } return true; } 1449402 4 346936 joepeck 2018-08-10 15:45:38 -0700 Created attachment 346936 [PATCH] Proposed Fix 1449417 5 346936 saam 2018-08-10 17:06:39 -0700 Comment on attachment 346936 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=346936&action=review > Source/JavaScriptCore/inspector/InjectedScriptSource.js:1295 > + let descriptor = Object.getOwnPropertyDescriptor(object, propertyName); Might be worth verifying that we're ok w/ this doing effects? Like I believe this calls a trap in a proxy. 1449445 6 joepeck 2018-08-10 19:04:24 -0700 (In reply to Saam Barati from comment #5) > Comment on attachment 346936 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=346936&action=review > > > Source/JavaScriptCore/inspector/InjectedScriptSource.js:1295 > > + let descriptor = Object.getOwnPropertyDescriptor(object, propertyName); > > Might be worth verifying that we're ok w/ this doing effects? Like I believe > this calls a trap in a proxy. Is there a way to tell if its a getter without an observable effect (without being a built-in)? 1449449 7 346936 saam 2018-08-10 19:15:30 -0700 Comment on attachment 346936 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=346936&action=review r=me >>> Source/JavaScriptCore/inspector/InjectedScriptSource.js:1295 >>> + let descriptor = Object.getOwnPropertyDescriptor(object, propertyName); >> >> Might be worth verifying that we're ok w/ this doing effects? Like I believe this calls a trap in a proxy. > > Is there a way to tell if its a getter without an observable effect (without being a built-in)? Nope, not in a general way. 1449455 8 346936 commit-queue 2018-08-10 19:43:54 -0700 Comment on attachment 346936 [PATCH] Proposed Fix Clearing flags on attachment: 346936 Committed r234780: <https://trac.webkit.org/changeset/234780> 1449456 9 commit-queue 2018-08-10 19:43:55 -0700 All reviewed patches have been landed. Closing bug. 346936 2018-08-10 15:45:38 -0700 2018-08-10 19:43:54 -0700 [PATCH] Proposed Fix invoke-getter-1.patch text/plain 4628 joepeck ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCBiZjkzNWEwMmFlNS4uNTIzYjE3ZjdjNmMgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3Rz L0NoYW5nZUxvZworKysgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAor MjAxOC0wOC0xMCAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUuY29tPgorCisgICAg ICAgIFdlYiBJbnNwZWN0b3I6IGNvbnNvbGUubG9nIGZpcmVzIGdldHRlcnMgZm9yIGRlZXAgcHJv cGVydGllcworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTg3NTQyCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS80Mjg3MzE1OD4KKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGluc3BlY3Rvci9pbmplY3RlZC1z Y3JpcHQvYXZvaWQtZ2V0dGVyLWludm9jYXRpb24tZXhwZWN0ZWQudHh0OiBBZGRlZC4KKyAgICAg ICAgKiBpbnNwZWN0b3IvaW5qZWN0ZWQtc2NyaXB0L2F2b2lkLWdldHRlci1pbnZvY2F0aW9uLmh0 bWw6IEFkZGVkLgorCiAyMDE4LTA4LTA5ICBKb3NlcGggUGVjb3Jhcm8gIDxwZWNvcmFyb0BhcHBs ZS5jb20+CiAKICAgICAgICAgRWxpbWluYXRlIHJlbWFpbmluZyBpbnN0YW5jZXMgb2YgIkVsQ2Fw aXRhbiIgZnJvbSBMYXlvdXRUZXN0cyAvIFRvb2xzCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9p bnNwZWN0b3IvaW5qZWN0ZWQtc2NyaXB0L2F2b2lkLWdldHRlci1pbnZvY2F0aW9uLWV4cGVjdGVk LnR4dCBiL0xheW91dFRlc3RzL2luc3BlY3Rvci9pbmplY3RlZC1zY3JpcHQvYXZvaWQtZ2V0dGVy LWludm9jYXRpb24tZXhwZWN0ZWQudHh0Cm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAw MDAwMDAwLi4xOTk4YjdlMDNjMAotLS0gL2Rldi9udWxsCisrKyBiL0xheW91dFRlc3RzL2luc3Bl Y3Rvci9pbmplY3RlZC1zY3JpcHQvYXZvaWQtZ2V0dGVyLWludm9jYXRpb24tZXhwZWN0ZWQudHh0 CkBAIC0wLDAgKzEsOSBAQAorQ09OU09MRSBNRVNTQUdFOiBsaW5lIDExOiBbb2JqZWN0IE9iamVj dF0KK0NPTlNPTEUgTUVTU0FHRTogbGluZSAxMjogW29iamVjdCBPYmplY3RdCitDT05TT0xFIE1F U1NBR0U6IGxpbmUgMTM6IFtvYmplY3QgT2JqZWN0XQorVGVzdCB0aGF0IGNlcnRhaW4gSW5qZWN0 ZWRTY3JpcHRTb3VyY2Ugb3BlcmF0aW9ucyBkbyBub3QgaW52b2tlIHVzZXIgZ2V0dGVycy4KKwor Cis9PSBSdW5uaW5nIHRlc3Qgc3VpdGU6IEluamVjdGVkU2NyaXB0LkdldHRlckludm9jYXRpb24u UHJldmlldworLS0gUnVubmluZyB0ZXN0IGNhc2U6IEluamVjdGVkU2NyaXB0LkdldHRlckludm9j YXRpb24uUHJldmlldworCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9pbnNwZWN0b3IvaW5qZWN0 ZWQtc2NyaXB0L2F2b2lkLWdldHRlci1pbnZvY2F0aW9uLmh0bWwgYi9MYXlvdXRUZXN0cy9pbnNw ZWN0b3IvaW5qZWN0ZWQtc2NyaXB0L2F2b2lkLWdldHRlci1pbnZvY2F0aW9uLmh0bWwKbmV3IGZp bGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMDAwMDAuLjM1YjhiNDI4Y2FiCi0tLSAvZGV2L251 bGwKKysrIGIvTGF5b3V0VGVzdHMvaW5zcGVjdG9yL2luamVjdGVkLXNjcmlwdC9hdm9pZC1nZXR0 ZXItaW52b2NhdGlvbi5odG1sCkBAIC0wLDAgKzEsMzUgQEAKKzwhRE9DVFlQRSBodG1sPgorPGh0 bWw+Cis8aGVhZD4KKzxzY3JpcHQgc3JjPSIuLi8uLi9odHRwL3Rlc3RzL2luc3BlY3Rvci9yZXNv dXJjZXMvaW5zcGVjdG9yLXRlc3QuanMiPjwvc2NyaXB0PgorPHNjcmlwdD4KK2Z1bmN0aW9uIHRy aWdnZXJMb2dzKCkgeworICAgIGxldCB0ZXN0ID0ge307CisgICAgT2JqZWN0LmRlZmluZVByb3Bl cnR5KHRlc3QsICJmb28iLCB7CisgICAgICAgIGdldCgpIHsgY29uc29sZS5sb2coIkZBSUxVUkU6 IFNob3VsZCBub3QgZmlyZSBnZXR0ZXIiKSB9CisgICAgfSk7CisgICAgY29uc29sZS5sb2codGVz dCk7CisgICAgY29uc29sZS5sb2coe3Rlc3R9KTsKKyAgICBjb25zb2xlLmxvZyhbdGVzdF0pOwor fQorCitmdW5jdGlvbiB0ZXN0KCkKK3sKKyAgICBsZXQgc3VpdGUgPSBJbnNwZWN0b3JUZXN0LmNy ZWF0ZUFzeW5jU3VpdGUoIkluamVjdGVkU2NyaXB0LkdldHRlckludm9jYXRpb24uUHJldmlldyIp OworCisgICAgc3VpdGUuYWRkVGVzdENhc2UoeworICAgICAgICBuYW1lOiAiSW5qZWN0ZWRTY3Jp cHQuR2V0dGVySW52b2NhdGlvbi5QcmV2aWV3IiwKKyAgICAgICAgZGVzY3JpcHRpb246ICJHZXR0 ZXJzIHNob3VsZCBub3QgYmUgaW52b2tlZCBkdXJpbmcgb2JqZWN0IHByZXZpZXdzLiIsCisgICAg ICAgIGFzeW5jIHRlc3QoKSB7CisgICAgICAgICAgICBhd2FpdCBJbnNwZWN0b3JUZXN0LmV2YWx1 YXRlSW5QYWdlKGB0cmlnZ2VyTG9ncygpYCk7CisgICAgICAgIH0KKyAgICB9KTsKKworICAgIHN1 aXRlLnJ1blRlc3RDYXNlc0FuZEZpbmlzaCgpOworfQorPC9zY3JpcHQ+Cis8L2hlYWQ+Cis8Ym9k eSBvbmxvYWQ9InJ1blRlc3QoKSI+Cis8cD5UZXN0IHRoYXQgY2VydGFpbiBJbmplY3RlZFNjcmlw dFNvdXJjZSBvcGVyYXRpb25zIGRvIG5vdCBpbnZva2UgdXNlciBnZXR0ZXJzLjwvcD4KKzwvYm9k eT4KKzwvaHRtbD4KZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cg Yi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IGI0YzhhYWQ3YTBiLi4xZTUz MDY2NzM0NyAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZworKysg Yi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTgt MDgtMTAgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKworICAgICAgICBX ZWIgSW5zcGVjdG9yOiBjb25zb2xlLmxvZyBmaXJlcyBnZXR0ZXJzIGZvciBkZWVwIHByb3BlcnRp ZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4NzU0 MgorICAgICAgICA8cmRhcjovL3Byb2JsZW0vNDI4NzMxNTg+CisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBpbnNwZWN0b3IvSW5qZWN0ZWRTY3JpcHRT b3VyY2UuanM6CisgICAgICAgIChSZW1vdGVPYmplY3QucHJvdG90eXBlLl9pc1ByZXZpZXdhYmxl T2JqZWN0KToKKyAgICAgICAgQXZvaWQgZ2V0dGVycy9zZXR0ZXJzIHdoZW4gY2hlY2tpbmcgZm9y IHNpbXBsZSBwcm9wZXJ0aWVzIHRvIHByZXZpZXcuCisgICAgICAgIEhlcmUgd2UgYXZvaWQgaW52 b2tpbmcgYG9iamVjdFtwcm9wZXJ0eV1gIGlmIGl0IGNvdWxkIGJlIGEgdXNlciBnZXR0ZXIuCisK IDIwMTgtMDgtMDUgIERhcmluIEFkbGVyICA8ZGFyaW5AYXBwbGUuY29tPgogCiAgICAgICAgIFtD b2NvYV0gTW9yZSB0d2Vha3MgYW5kIHJlZmFjdG9yaW5nIHRvIHByZXBhcmUgZm9yIEFSQwpkaWZm IC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2luc3BlY3Rvci9JbmplY3RlZFNjcmlwdFNv dXJjZS5qcyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9pbnNwZWN0b3IvSW5qZWN0ZWRTY3JpcHRT b3VyY2UuanMKaW5kZXggNDUxMThhNTNlNjQuLmQ2Yjc1ZjFmZWNkIDEwMDY0NAotLS0gYS9Tb3Vy Y2UvSmF2YVNjcmlwdENvcmUvaW5zcGVjdG9yL0luamVjdGVkU2NyaXB0U291cmNlLmpzCisrKyBi L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9pbnNwZWN0b3IvSW5qZWN0ZWRTY3JpcHRTb3VyY2UuanMK QEAgLTEyODYsMTIgKzEyODYsMTUgQEAgbGV0IFJlbW90ZU9iamVjdCA9IGNsYXNzIFJlbW90ZU9i amVjdAogICAgICAgICBpZiAob2JqZWN0Ll9fcHJvdG9fXyAmJiBvYmplY3QuX19wcm90b19fLl9f cHJvdG9fXykKICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKIAotICAgICAgICAvLyBPYmplY3Rz IGFyZSBzaW1wbGUgaWYgdGhleSBoYXZlIDMgb3IgbGVzcyBzaW1wbGUgcHJvcGVydGllcy4KKyAg ICAgICAgLy8gT2JqZWN0cyBhcmUgc2ltcGxlIGlmIHRoZXkgaGF2ZSAzIG9yIGxlc3Mgc2ltcGxl IHZhbHVlIHByb3BlcnRpZXMuCiAgICAgICAgIGxldCBvd25Qcm9wZXJ0eU5hbWVzID0gT2JqZWN0 LmdldE93blByb3BlcnR5TmFtZXMob2JqZWN0KTsKICAgICAgICAgaWYgKG93blByb3BlcnR5TmFt ZXMubGVuZ3RoID4gMykKICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKICAgICAgICAgZm9yIChs ZXQgaSA9IDA7IGkgPCBvd25Qcm9wZXJ0eU5hbWVzLmxlbmd0aDsgKytpKSB7CiAgICAgICAgICAg ICBsZXQgcHJvcGVydHlOYW1lID0gb3duUHJvcGVydHlOYW1lc1tpXTsKKyAgICAgICAgICAgIGxl dCBkZXNjcmlwdG9yID0gT2JqZWN0LmdldE93blByb3BlcnR5RGVzY3JpcHRvcihvYmplY3QsIHBy b3BlcnR5TmFtZSk7CisgICAgICAgICAgICBpZiAoZGVzY3JpcHRvciAmJiAhKCJ2YWx1ZSIgaW4g ZGVzY3JpcHRvcikpCisgICAgICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgICAgICAgICAg aWYgKCF0aGlzLl9pc1ByZXZpZXdhYmxlT2JqZWN0SW50ZXJuYWwob2JqZWN0W3Byb3BlcnR5TmFt ZV0sIGtub3duT2JqZWN0cywgZGVwdGgpKQogICAgICAgICAgICAgICAgIHJldHVybiBmYWxzZTsK ICAgICAgICAgfQo=