169638 2017-03-14 15:37:06 -0700 Web Inspector: Exception when fetching computed styles can break future updates of section 2017-03-14 17:05:34 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck commit-queue hi inspector-bugzilla-changes joepeck webkit-bug-importer oldest_to_newest 1287839 0 joepeck 2017-03-14 15:37:06 -0700 Summary: Our style fetching should be resilient in the face of exceptions. Steps to Reproduce: 1. Introduce an exception inside of `fetchedComputedStyle` or its friends 2. Inspect a page 3. Show Computed Styles sidebar 4. Select a bunch of nodes 5. Switch between nodes => No updates 1287840 1 joepeck 2017-03-14 15:37:16 -0700 <rdar://problem/30588688> 1287841 2 joepeck 2017-03-14 15:38:03 -0700 This was originally detected with an Augmented JSContext with CSS Styles. In that case certain assumptions (like the computed style must contain certain properties) fails and we get unexpected exceptions. In any case we should be resilient against that. 1287845 3 304436 joepeck 2017-03-14 15:38:56 -0700 Created attachment 304436 [PATCH] Proposed Fix 1287847 4 304436 bburg 2017-03-14 15:42:21 -0700 Comment on attachment 304436 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=304436&action=review > Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js:89 > + promise.resolve(); Shouldn't this reject (and ignore if appropriate where the promise is chained)? 1287849 5 304436 hi 2017-03-14 15:47:50 -0700 Comment on attachment 304436 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=304436&action=review > Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js:84 > + return function() { I'm not sure if this will ever happen, but I don't think this will capture `this` correctly. You might want to use an arrow function instead. An example: function wrap() { console.log("outer", this); return function() { console.log("inner", this); } } wrap.call()() // outer - Window // inner - Window wrap.call({})() // outer - {} // inner - Window 1287855 6 304436 joepeck 2017-03-14 16:04:19 -0700 Comment on attachment 304436 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=304436&action=review >> Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js:84 >> + return function() { > > I'm not sure if this will ever happen, but I don't think this will capture `this` correctly. You might want to use an arrow function instead. An example: > > function wrap() { > console.log("outer", this); > return function() { > console.log("inner", this); > } > } > > wrap.call()() > // outer - Window > // inner - Window > > wrap.call({})() > // outer - {} > // inner - Window `this` is never captured. In here, `this` is bound by the caller of wrap. wrap(...).bind(this). Should I make it more explicit? >> Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js:89 >> + promise.resolve(); > > Shouldn't this reject (and ignore if appropriate where the promise is chained)? This won't trigger the Promise.all below. I can make that include a catch, but I figured it ultimately doesn't matter here. 1287865 7 304439 joepeck 2017-03-14 16:21:15 -0700 Created attachment 304439 [PATCH] Proposed Fix 1287869 8 304439 hi 2017-03-14 16:33:04 -0700 Comment on attachment 304439 [PATCH] Proposed Fix r=me 1287892 9 304439 commit-queue 2017-03-14 17:05:30 -0700 Comment on attachment 304439 [PATCH] Proposed Fix Clearing flags on attachment: 304439 Committed r213961: <http://trac.webkit.org/changeset/213961> 1287893 10 commit-queue 2017-03-14 17:05:34 -0700 All reviewed patches have been landed. Closing bug. 304436 2017-03-14 15:38:56 -0700 2017-03-14 16:21:15 -0700 [PATCH] Proposed Fix update-1.patch text/plain 3248 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGZhNGYwZTM3NmQyLi5hODkyMjUzNjY0ZSAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTctMDMtMTQgIEpvc2Vw aCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKworICAgICAgICBXZWIgSW5zcGVjdG9y OiBFeGNlcHRpb24gd2hlbiBmZXRjaGluZyBjb21wdXRlZCBzdHlsZXMgY2FuIGJyZWFrIGZ1dHVy ZSB1cGRhdGVzIG9mIHNlY3Rpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTE2OTYzOAorICAgICAgICA8cmRhcjovL3Byb2JsZW0vMzA1ODg2ODg+CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBVc2VySW50 ZXJmYWNlL01vZGVscy9ET01Ob2RlU3R5bGVzLmpzOgorICAgICAgICAoV2ViSW5zcGVjdG9yLkRP TU5vZGVTdHlsZXMucHJvdG90eXBlLnJlZnJlc2gud3JhcCk6CisgICAgICAgIChXZWJJbnNwZWN0 b3IuRE9NTm9kZVN0eWxlcy5wcm90b3R5cGUucmVmcmVzaCk6CisgICAgICAgIEdyYWNlZnVsbHkg aGFuZGxlIGV4Y2VwdGlvbnMuIElmIGFuIGV4Y2VwdGlvbiBkaWQgaGFwcGVuIHdlCisgICAgICAg IHdvdWxkIGJlIHVuYWJsZSB0byB1cGRhdGUgdGhlc2UgaW4gdGhlIGZ1dHVyZS4KKwogMjAxNy0w My0xNCAgRGV2aW4gUm91c3NvICA8d2Via2l0QGRldmlucm91c3NvLmNvbT4KIAogICAgICAgICBX ZWIgSW5zcGVjdG9yOiBSVEw6IGFkZCBzdXBwb3J0IGZvciBUaW1lbGluZSBncmFwaHMKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL01vZGVscy9ET01Ob2Rl U3R5bGVzLmpzIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvTW9kZWxzL0RP TU5vZGVTdHlsZXMuanMKaW5kZXggNTY0YmU1ZTUyMmUuLjMwOTE1ZDkwNzY5IDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9Nb2RlbHMvRE9NTm9kZVN0eWxl cy5qcworKysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9Nb2RlbHMvRE9N Tm9kZVN0eWxlcy5qcwpAQCAtNzksNiArNzksMTggQEAgV2ViSW5zcGVjdG9yLkRPTU5vZGVTdHls ZXMgPSBjbGFzcyBET01Ob2RlU3R5bGVzIGV4dGVuZHMgV2ViSW5zcGVjdG9yLk9iamVjdAogICAg ICAgICBsZXQgZmV0Y2hlZElubGluZVN0eWxlc1Byb21pc2UgPSBuZXcgV2ViSW5zcGVjdG9yLldy YXBwZWRQcm9taXNlOwogICAgICAgICBsZXQgZmV0Y2hlZENvbXB1dGVkU3R5bGVzUHJvbWlzZSA9 IG5ldyBXZWJJbnNwZWN0b3IuV3JhcHBlZFByb21pc2U7CiAKKyAgICAgICAgLy8gRW5zdXJlIHdl IHJlc29sdmUgdGhlc2UgcHJvbWlzZXMgZXZlbiBpbiB0aGUgY2FzZSBvZiBhbiBlcnJvci4KKyAg ICAgICAgZnVuY3Rpb24gd3JhcChmdW5jLCBwcm9taXNlKSB7CisgICAgICAgICAgICByZXR1cm4g ZnVuY3Rpb24oKSB7CisgICAgICAgICAgICAgICAgdHJ5IHsKKyAgICAgICAgICAgICAgICAgICAg ZnVuYy5hcHBseSh0aGlzLCBhcmd1bWVudHMpOworICAgICAgICAgICAgICAgIH0gY2F0Y2ggKGUp IHsKKyAgICAgICAgICAgICAgICAgICAgY29uc29sZS5lcnJvcihlKTsKKyAgICAgICAgICAgICAg ICAgICAgcHJvbWlzZS5yZXNvbHZlKCk7CisgICAgICAgICAgICAgICAgfQorICAgICAgICAgICAg fQorICAgICAgICB9CisKICAgICAgICAgZnVuY3Rpb24gcGFyc2VSdWxlTWF0Y2hBcnJheVBheWxv YWQobWF0Y2hBcnJheSwgbm9kZSwgaW5oZXJpdGVkKQogICAgICAgICB7CiAgICAgICAgICAgICB2 YXIgcmVzdWx0ID0gW107CkBAIC0yMzksOSArMjUxLDkgQEAgV2ViSW5zcGVjdG9yLkRPTU5vZGVT dHlsZXMgPSBjbGFzcyBET01Ob2RlU3R5bGVzIGV4dGVuZHMgV2ViSW5zcGVjdG9yLk9iamVjdAog ICAgICAgICAvLyBGSVhNRTogQ29udmVydCB0byBwdXNoaW5nIFN0eWxlU2hlZXQgaW5mb3JtYXRp b24gdG8gdGhlIGZyb250ZW5kLiA8cmRhcjovL3Byb2JsZW0vMTMyMTM2ODA+CiAgICAgICAgIFdl Ykluc3BlY3Rvci5jc3NTdHlsZU1hbmFnZXIuZmV0Y2hTdHlsZVNoZWV0c0lmTmVlZGVkKCk7CiAK LSAgICAgICAgQ1NTQWdlbnQuZ2V0TWF0Y2hlZFN0eWxlc0Zvck5vZGUuaW52b2tlKHtub2RlSWQ6 IHRoaXMuX25vZGUuaWQsIGluY2x1ZGVQc2V1ZG86IHRydWUsIGluY2x1ZGVJbmhlcml0ZWQ6IHRy dWV9LCBmZXRjaGVkTWF0Y2hlZFN0eWxlcy5iaW5kKHRoaXMpKTsKLSAgICAgICAgQ1NTQWdlbnQu Z2V0SW5saW5lU3R5bGVzRm9yTm9kZS5pbnZva2Uoe25vZGVJZDogdGhpcy5fbm9kZS5pZH0sIGZl dGNoZWRJbmxpbmVTdHlsZXMuYmluZCh0aGlzKSk7Ci0gICAgICAgIENTU0FnZW50LmdldENvbXB1 dGVkU3R5bGVGb3JOb2RlLmludm9rZSh7bm9kZUlkOiB0aGlzLl9ub2RlLmlkfSwgZmV0Y2hlZENv bXB1dGVkU3R5bGUuYmluZCh0aGlzKSk7CisgICAgICAgIENTU0FnZW50LmdldE1hdGNoZWRTdHls ZXNGb3JOb2RlLmludm9rZSh7bm9kZUlkOiB0aGlzLl9ub2RlLmlkLCBpbmNsdWRlUHNldWRvOiB0 cnVlLCBpbmNsdWRlSW5oZXJpdGVkOiB0cnVlfSwgd3JhcChmZXRjaGVkTWF0Y2hlZFN0eWxlcywg ZmV0Y2hlZE1hdGNoZWRTdHlsZXNQcm9taXNlKS5iaW5kKHRoaXMpKTsKKyAgICAgICAgQ1NTQWdl bnQuZ2V0SW5saW5lU3R5bGVzRm9yTm9kZS5pbnZva2Uoe25vZGVJZDogdGhpcy5fbm9kZS5pZH0s IHdyYXAoZmV0Y2hlZElubGluZVN0eWxlcywgZmV0Y2hlZElubGluZVN0eWxlc1Byb21pc2UpLmJp bmQodGhpcykpOworICAgICAgICBDU1NBZ2VudC5nZXRDb21wdXRlZFN0eWxlRm9yTm9kZS5pbnZv a2Uoe25vZGVJZDogdGhpcy5fbm9kZS5pZH0sIHdyYXAoZmV0Y2hlZENvbXB1dGVkU3R5bGUsIGZl dGNoZWRDb21wdXRlZFN0eWxlc1Byb21pc2UpLmJpbmQodGhpcykpOwogCiAgICAgICAgIHRoaXMu X3BlbmRpbmdSZWZyZXNoVGFzayA9IFByb21pc2UuYWxsKFtmZXRjaGVkTWF0Y2hlZFN0eWxlc1By b21pc2UucHJvbWlzZSwgZmV0Y2hlZElubGluZVN0eWxlc1Byb21pc2UucHJvbWlzZSwgZmV0Y2hl ZENvbXB1dGVkU3R5bGVzUHJvbWlzZS5wcm9taXNlXSkKICAgICAgICAgLnRoZW4oKCkgPT4gewo= 304439 2017-03-14 16:21:15 -0700 2017-03-14 17:05:30 -0700 [PATCH] Proposed Fix v2.patch text/plain 3246 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGZhNGYwZTM3NmQyLi5hODkyMjUzNjY0ZSAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTctMDMtMTQgIEpvc2Vw aCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKworICAgICAgICBXZWIgSW5zcGVjdG9y OiBFeGNlcHRpb24gd2hlbiBmZXRjaGluZyBjb21wdXRlZCBzdHlsZXMgY2FuIGJyZWFrIGZ1dHVy ZSB1cGRhdGVzIG9mIHNlY3Rpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTE2OTYzOAorICAgICAgICA8cmRhcjovL3Byb2JsZW0vMzA1ODg2ODg+CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBVc2VySW50 ZXJmYWNlL01vZGVscy9ET01Ob2RlU3R5bGVzLmpzOgorICAgICAgICAoV2ViSW5zcGVjdG9yLkRP TU5vZGVTdHlsZXMucHJvdG90eXBlLnJlZnJlc2gud3JhcCk6CisgICAgICAgIChXZWJJbnNwZWN0 b3IuRE9NTm9kZVN0eWxlcy5wcm90b3R5cGUucmVmcmVzaCk6CisgICAgICAgIEdyYWNlZnVsbHkg aGFuZGxlIGV4Y2VwdGlvbnMuIElmIGFuIGV4Y2VwdGlvbiBkaWQgaGFwcGVuIHdlCisgICAgICAg IHdvdWxkIGJlIHVuYWJsZSB0byB1cGRhdGUgdGhlc2UgaW4gdGhlIGZ1dHVyZS4KKwogMjAxNy0w My0xNCAgRGV2aW4gUm91c3NvICA8d2Via2l0QGRldmlucm91c3NvLmNvbT4KIAogICAgICAgICBX ZWIgSW5zcGVjdG9yOiBSVEw6IGFkZCBzdXBwb3J0IGZvciBUaW1lbGluZSBncmFwaHMKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL01vZGVscy9ET01Ob2Rl U3R5bGVzLmpzIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvTW9kZWxzL0RP TU5vZGVTdHlsZXMuanMKaW5kZXggNTY0YmU1ZTUyMmUuLjA2ZDVlYWYxMDcwIDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9Nb2RlbHMvRE9NTm9kZVN0eWxl cy5qcworKysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9Nb2RlbHMvRE9N Tm9kZVN0eWxlcy5qcwpAQCAtNzksNiArNzksMTggQEAgV2ViSW5zcGVjdG9yLkRPTU5vZGVTdHls ZXMgPSBjbGFzcyBET01Ob2RlU3R5bGVzIGV4dGVuZHMgV2ViSW5zcGVjdG9yLk9iamVjdAogICAg ICAgICBsZXQgZmV0Y2hlZElubGluZVN0eWxlc1Byb21pc2UgPSBuZXcgV2ViSW5zcGVjdG9yLldy YXBwZWRQcm9taXNlOwogICAgICAgICBsZXQgZmV0Y2hlZENvbXB1dGVkU3R5bGVzUHJvbWlzZSA9 IG5ldyBXZWJJbnNwZWN0b3IuV3JhcHBlZFByb21pc2U7CiAKKyAgICAgICAgLy8gRW5zdXJlIHdl IHJlc29sdmUgdGhlc2UgcHJvbWlzZXMgZXZlbiBpbiB0aGUgY2FzZSBvZiBhbiBlcnJvci4KKyAg ICAgICAgZnVuY3Rpb24gd3JhcChmdW5jLCBwcm9taXNlKSB7CisgICAgICAgICAgICByZXR1cm4g KC4uLmFyZ3MpID0+IHsKKyAgICAgICAgICAgICAgICB0cnkgeworICAgICAgICAgICAgICAgICAg ICBmdW5jLmFwcGx5KHRoaXMsIGFyZ3MpOworICAgICAgICAgICAgICAgIH0gY2F0Y2ggKGUpIHsK KyAgICAgICAgICAgICAgICAgICAgY29uc29sZS5lcnJvcihlKTsKKyAgICAgICAgICAgICAgICAg ICAgcHJvbWlzZS5yZXNvbHZlKCk7CisgICAgICAgICAgICAgICAgfQorICAgICAgICAgICAgfTsK KyAgICAgICAgfQorCiAgICAgICAgIGZ1bmN0aW9uIHBhcnNlUnVsZU1hdGNoQXJyYXlQYXlsb2Fk KG1hdGNoQXJyYXksIG5vZGUsIGluaGVyaXRlZCkKICAgICAgICAgewogICAgICAgICAgICAgdmFy IHJlc3VsdCA9IFtdOwpAQCAtMjM5LDkgKzI1MSw5IEBAIFdlYkluc3BlY3Rvci5ET01Ob2RlU3R5 bGVzID0gY2xhc3MgRE9NTm9kZVN0eWxlcyBleHRlbmRzIFdlYkluc3BlY3Rvci5PYmplY3QKICAg ICAgICAgLy8gRklYTUU6IENvbnZlcnQgdG8gcHVzaGluZyBTdHlsZVNoZWV0IGluZm9ybWF0aW9u IHRvIHRoZSBmcm9udGVuZC4gPHJkYXI6Ly9wcm9ibGVtLzEzMjEzNjgwPgogICAgICAgICBXZWJJ bnNwZWN0b3IuY3NzU3R5bGVNYW5hZ2VyLmZldGNoU3R5bGVTaGVldHNJZk5lZWRlZCgpOwogCi0g ICAgICAgIENTU0FnZW50LmdldE1hdGNoZWRTdHlsZXNGb3JOb2RlLmludm9rZSh7bm9kZUlkOiB0 aGlzLl9ub2RlLmlkLCBpbmNsdWRlUHNldWRvOiB0cnVlLCBpbmNsdWRlSW5oZXJpdGVkOiB0cnVl fSwgZmV0Y2hlZE1hdGNoZWRTdHlsZXMuYmluZCh0aGlzKSk7Ci0gICAgICAgIENTU0FnZW50Lmdl dElubGluZVN0eWxlc0Zvck5vZGUuaW52b2tlKHtub2RlSWQ6IHRoaXMuX25vZGUuaWR9LCBmZXRj aGVkSW5saW5lU3R5bGVzLmJpbmQodGhpcykpOwotICAgICAgICBDU1NBZ2VudC5nZXRDb21wdXRl ZFN0eWxlRm9yTm9kZS5pbnZva2Uoe25vZGVJZDogdGhpcy5fbm9kZS5pZH0sIGZldGNoZWRDb21w dXRlZFN0eWxlLmJpbmQodGhpcykpOworICAgICAgICBDU1NBZ2VudC5nZXRNYXRjaGVkU3R5bGVz Rm9yTm9kZS5pbnZva2Uoe25vZGVJZDogdGhpcy5fbm9kZS5pZCwgaW5jbHVkZVBzZXVkbzogdHJ1 ZSwgaW5jbHVkZUluaGVyaXRlZDogdHJ1ZX0sIHdyYXAuY2FsbCh0aGlzLCBmZXRjaGVkTWF0Y2hl ZFN0eWxlcywgZmV0Y2hlZE1hdGNoZWRTdHlsZXNQcm9taXNlKSk7CisgICAgICAgIENTU0FnZW50 LmdldElubGluZVN0eWxlc0Zvck5vZGUuaW52b2tlKHtub2RlSWQ6IHRoaXMuX25vZGUuaWR9LCB3 cmFwLmNhbGwodGhpcywgZmV0Y2hlZElubGluZVN0eWxlcywgZmV0Y2hlZElubGluZVN0eWxlc1By b21pc2UpKTsKKyAgICAgICAgQ1NTQWdlbnQuZ2V0Q29tcHV0ZWRTdHlsZUZvck5vZGUuaW52b2tl KHtub2RlSWQ6IHRoaXMuX25vZGUuaWR9LCB3cmFwLmNhbGwodGhpcywgZmV0Y2hlZENvbXB1dGVk U3R5bGUsIGZldGNoZWRDb21wdXRlZFN0eWxlc1Byb21pc2UpKTsKIAogICAgICAgICB0aGlzLl9w ZW5kaW5nUmVmcmVzaFRhc2sgPSBQcm9taXNlLmFsbChbZmV0Y2hlZE1hdGNoZWRTdHlsZXNQcm9t aXNlLnByb21pc2UsIGZldGNoZWRJbmxpbmVTdHlsZXNQcm9taXNlLnByb21pc2UsIGZldGNoZWRD b21wdXRlZFN0eWxlc1Byb21pc2UucHJvbWlzZV0pCiAgICAgICAgIC50aGVuKCgpID0+IHsK