{"action":"created","issue":{"url":"https://api.github.com/repos/servo/servo/issues/8412","labels_url":"https://api.github.com/repos/servo/servo/issues/8412/labels{/name}","comments_url":"https://api.github.com/repos/servo/servo/issues/8412/comments","events_url":"https://api.github.com/repos/servo/servo/issues/8412/events","html_url":"https://github.com/servo/servo/pull/8412","id":115739495,"number":8412,"title":"Add WebIDL sequence return values","user":{"login":"ecoal95","id":1323194,"avatar_url":"https://avatars.githubusercontent.com/u/1323194?v=3","gravatar_id":"","url":"https://api.github.com/users/ecoal95","html_url":"https://github.com/ecoal95","followers_url":"https://api.github.com/users/ecoal95/followers","following_url":"https://api.github.com/users/ecoal95/following{/other_user}","gists_url":"https://api.github.com/users/ecoal95/gists{/gist_id}","starred_url":"https://api.github.com/users/ecoal95/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ecoal95/subscriptions","organizations_url":"https://api.github.com/users/ecoal95/orgs","repos_url":"https://api.github.com/users/ecoal95/repos","events_url":"https://api.github.com/users/ecoal95/events{/privacy}","received_events_url":"https://api.github.com/users/ecoal95/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/servo/servo/labels/S-needs-code-changes","name":"S-needs-code-changes","color":"d4c5f9"}],"state":"open","locked":false,"assignee":{"login":"jdm","id":27658,"avatar_url":"https://avatars.githubusercontent.com/u/27658?v=3","gravatar_id":"","url":"https://api.github.com/users/jdm","html_url":"https://github.com/jdm","followers_url":"https://api.github.com/users/jdm/followers","following_url":"https://api.github.com/users/jdm/following{/other_user}","gists_url":"https://api.github.com/users/jdm/gists{/gist_id}","starred_url":"https://api.github.com/users/jdm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdm/subscriptions","organizations_url":"https://api.github.com/users/jdm/orgs","repos_url":"https://api.github.com/users/jdm/repos","events_url":"https://api.github.com/users/jdm/events{/privacy}","received_events_url":"https://api.github.com/users/jdm/received_events","type":"User","site_admin":false},"milestone":null,"comments":3,"created_at":"2015-11-08T15:24:54Z","updated_at":"2015-11-08T16:07:41Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/servo/servo/pulls/8412","html_url":"https://github.com/servo/servo/pull/8412","diff_url":"https://github.com/servo/servo/pull/8412.diff","patch_url":"https://github.com/servo/servo/pull/8412.patch"},"body":"And use it for `WebGLRenderingContext::getSupportedExtensions`.\r\nPart of #544\n\n<!-- Reviewable:start -->\n[<img src=\"https://reviewable.io/review_button.png\" height=40 alt=\"Review on Reviewable\"/>](https://reviewable.io/reviews/servo/servo/8412)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/servo/servo/issues/comments/154835555","html_url":"https://github.com/servo/servo/pull/8412#issuecomment-154835555","issue_url":"https://api.github.com/repos/servo/servo/issues/8412","id":154835555,"user":{"login":"jdm","id":27658,"avatar_url":"https://avatars.githubusercontent.com/u/27658?v=3","gravatar_id":"","url":"https://api.github.com/users/jdm","html_url":"https://github.com/jdm","followers_url":"https://api.github.com/users/jdm/followers","following_url":"https://api.github.com/users/jdm/following{/other_user}","gists_url":"https://api.github.com/users/jdm/gists{/gist_id}","starred_url":"https://api.github.com/users/jdm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdm/subscriptions","organizations_url":"https://api.github.com/users/jdm/orgs","repos_url":"https://api.github.com/users/jdm/repos","events_url":"https://api.github.com/users/jdm/events{/privacy}","received_events_url":"https://api.github.com/users/jdm/received_events","type":"User","site_admin":false},"created_at":"2015-11-08T16:07:41Z","updated_at":"2015-11-08T16:07:41Z","body":"-S-awaiting-review +S-needs-code-changes \n<!-- Reviewable comment -K2bq_7FQhpTesEyaRNf:1458266033 -->\n\n---\n\nReviewed 2 of 2 files at r1, 2 of 2 files at r2, 4 of 4 files at r3.\nReview status: all files reviewed at latest revision, 3 unresolved discussions.\n\n---\n\n<sup>**[components/script/dom/bindings/conversions.rs, line 817 \\[r1\\]](https://reviewable.io:443/reviews/servo/servo/8412#-K2bn4ngoGnu7aa9hW3S)** ([raw file](https://github.com/servo/servo/blob/3ef89a47c74ab18654aa1a6f7cc25d2c7a316564/components/script/dom/bindings/conversions.rs#L817)):</sup>\nThere are a couple unsafe things about, this, unfortunately. If `obj.to_jsval` triggers a GC, all of the previous elements that have been been will become dangling pointers since they're not rooted. We should do this instead:\n\n```rust\nlet mut js_objects = RootedVec::new();\nfor obj in &self {\n    let mut val = RootedValue::new(cx, UndefinedValue());\n    obj.to_jsval(cx, val.handle_mut());\n    js_objects.push(val.handle().get());\n}\n```\nThis will ensure that each converted value is stored in a rooted vector, so subsequent conversions will not risk GCing them.\n<!-- Reviewable comment -K2bn4nhFwl9J-H8nywI:2105700195 -->\n\n---\n\n<sup>**[components/script/dom/bindings/conversions.rs, line 827 \\[r1\\]](https://reviewable.io:443/reviews/servo/servo/8412#-K2bpCFpgEQ_kKCVorjm)** ([raw file](https://github.com/servo/servo/blob/3ef89a47c74ab18654aa1a6f7cc25d2c7a316564/components/script/dom/bindings/conversions.rs#L827)):</sup>\nObjectValue, since we've already checked the presence of null.\n<!-- Reviewable comment -K2bpCFpgEQ_kKCVorjn:-1332814764 -->\n\n---\n\n<sup>**[components/script/dom/bindings/conversions.rs, line 814 \\[r3\\]](https://reviewable.io:443/reviews/servo/servo/8412#-K2bgvafxycDLLc0CgSs-r3-814)** ([raw file](https://github.com/servo/servo/blob/82c243a63f6d99e43885636b22c4f99ca89b24d7/components/script/dom/bindings/conversions.rs#L814)):</sup>\nSee below :) from_marked_location is a huge red flag, in general, since it means \"I swear that the address I'm passing here is known to the GC\".\n<!-- Reviewable comment -K2boWXglrgTgat39MR4:576544824 -->\n\n---\n\n\nComments from the [review on Reviewable.io](https://reviewable.io:443/reviews/servo/servo/8412)\n<!-- Sent from Reviewable.io -->\n"}}