Implement [Unforgeable] #7988

nox · 2015-10-13T00:47:46Z

This is mostly stolen from Gecko. As there, we define the unforgeable members
on an object stored in the slots of the prototype object. They are then copied
onto instance objects when they are instantiated. It should be noted that
proxy objects see their unforgeable memebers defined on their expando object.

Unforgeable attributes aren't properly inherited in codegen (in a similar
fashion as getters and setters as filed in #5875) and require to be redefined
in derived interfaces. Fortunately, there are currently no such interfaces.

No unforgeable members can be included into the TestBinding interfaces for good
measure because they are not compatible with setters.

highfive · 2015-10-13T00:47:49Z

Warning

These commits modify unsafe code. Please review it carefully!

bors-servo · 2015-10-14T21:27:36Z

☔ The latest upstream changes (presumably #7727) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2015-10-20T00:39:07Z

☔ The latest upstream changes (presumably #8089) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2015-10-25T23:36:31Z

☔ The latest upstream changes (presumably #8196) made this pull request unmergeable. Please resolve the merge conflicts.

metajack · 2015-11-03T21:20:08Z

r? @Ms2ger

jdm · 2015-11-11T20:51:30Z

This matches up well with Gecko's implementation. I'm happy with it!
-S-awaiting-review +@jdm

Reviewed 9 of 9 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2214 [r1] (raw file):}
These functions return Result, so we need to check them. Gecko passes in a failureCode argument; we might be able to just assert that it's Ok.

Comments from the review on Reviewable.io

jdm · 2015-11-11T20:52:41Z

That being said, I'd like to have a test for overwriting an unforgeable property as well as unforgeable properties on proxies.

jdm · 2015-11-11T20:53:28Z

Here's one from gecko we can steal: http://mxr.mozilla.org/mozilla-central/source/dom/bindings/test/test_unforgeablesonexpando.html?force=1

nox · 2015-11-12T11:22:41Z

I added a test. The failing one is unrelated to unforgeable attributes but I included it nonetheless.

-S-needs-rebase

Review status: 2 of 9 files reviewed at latest revision, 1 unresolved discussion.

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2214 [r1] (raw file):}
Done.

Comments from the review on Reviewable.io

nox · 2015-12-01T22:22:51Z

I amended the patch with the test expectations.

jdm · 2015-12-02T20:50:01Z

This can merge with the requested comment addition.
-S-awaiting-review +S-needs-code-changes

Reviewed 8 of 8 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

components/script/dom/bindings/codegen/CodegenRust.py, line 4706 [r3] (raw file):
Let's add a comment explaining why this is necessary.

Comments from the review on Reviewable.io

This is mostly stolen from Gecko. As there, we define the unforgeable members on an object stored in the slots of the prototype object. They are then copied onto instance objects when they are instantiated. It should be noted that proxy objects see their unforgeable memebers defined on their expando object. Unforgeable attributes aren't properly inherited in codegen (in a similar fashion as getters and setters as filed in servo#5875) and require to be redefined in derived interfaces. Fortunately, there are currently no such interfaces. No unforgeable members can be included into the TestBinding interfaces for good measure because they are not compatible with setters. Given the unforgeable holder object has the same prototype as actual instances of the interface, the finalize hook needs to check its slot pointer for nullity before dropping it. The new failing test isn't related to Unforgeable attributes, but to the fact that all Document instances currently have a Location, even if their window isn't in a browsing context.

nox · 2015-12-02T21:37:12Z

Comment added.

@bors-servo r=jdm

bors-servo · 2015-12-02T21:37:13Z

📌 Commit 6097640 has been approved by jdm

bors-servo · 2015-12-03T03:52:56Z

⌛ Testing commit 6097640 with merge b371f23...

Implement [Unforgeable] This is mostly stolen from Gecko. As there, we define the unforgeable members on an object stored in the slots of the prototype object. They are then copied onto instance objects when they are instantiated. It should be noted that proxy objects see their unforgeable memebers defined on their expando object. Unforgeable attributes aren't properly inherited in codegen (in a similar fashion as getters and setters as filed in #5875) and require to be redefined in derived interfaces. Fortunately, there are currently no such interfaces. No unforgeable members can be included into the TestBinding interfaces for good measure because they are not compatible with setters.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7988)

bors-servo · 2015-12-03T04:27:42Z

💔 Test failed - mac-rel-wpt

KiChjang · 2015-12-03T05:02:16Z

Ran 3717 tests finished in 870.0 seconds.
  • 3716 ran as expected. 710 tests skipped.
  • 1 tests timed out unexpectedly

Tests with unexpected results:
  ▶ TIMEOUT [expected PASS] /_mozilla/css/iframe/hide_layers2.html
  │ 
  │ thread 'Constellation' panicked at 'unable to find pipeline - this is a bug', ../src/libcore/option.rs:335
  │ stack backtrace:
  │    1:        0x107c70d08 - sys::backtrace::tracing::imp::write::h3ed615ef3cda9b6b7It
  │    2:        0x107c72e4f - panicking::log_panic::_<closure>::closure.40650
  │    3:        0x107c728f2 - panicking::log_panic::hdfd4a4ec7b55d6a5MAx
  │    4:        0x107c5dde6 - sys_common::unwind::begin_unwind_inner::h2f70decd24e7a2917Ls
  │    5:        0x107c5e1ce - sys_common::unwind::begin_unwind_fmt::h29da0d611b24747ddLs
  │    6:        0x107c70327 - rust_begin_unwind
  │    7:        0x107c95430 - panicking::panic_fmt::h2f3428e3725b99d6VFK
  │    8:        0x10669962d - constellation::_<impl>::handle_request::handle_request::h14084396330141331367
  │    9:        0x10667eaa7 - sys_common::unwind::try::try_fn::try_fn::h5790274250769403721
  │   10:        0x107c70148 - __rust_try
  │   11:        0x107c6d24e - sys_common::unwind::try::inner_try::hd7e4f8fc422767d2FIs
  │   12:        0x106680b6a - boxed::_<impl>::call_box::call_box::h8113799359328547308
  │   13:        0x107c720dd - sys::thread::_<impl>::new::thread_start::ha2c46efb1706c462jVw
  │   14:     0x7fff91449059 - _pthread_body

KiChjang · 2015-12-03T05:03:34Z

@bors-servo retry

Intermittent timeout in /_mozilla/css/iframe/hide_layers2.html (Constellation panic) #8769

bors-servo · 2015-12-03T05:33:06Z

⌛ Testing commit 6097640 with merge 20df7fb...

Implement [Unforgeable] This is mostly stolen from Gecko. As there, we define the unforgeable members on an object stored in the slots of the prototype object. They are then copied onto instance objects when they are instantiated. It should be noted that proxy objects see their unforgeable memebers defined on their expando object. Unforgeable attributes aren't properly inherited in codegen (in a similar fashion as getters and setters as filed in #5875) and require to be redefined in derived interfaces. Fortunately, there are currently no such interfaces. No unforgeable members can be included into the TestBinding interfaces for good measure because they are not compatible with setters.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7988)

bors-servo · 2015-12-03T06:26:17Z

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 13, 2015

nox added A-content/dom Interacting with the DOM from web content A-content/bindings The DOM bindings labels Oct 13, 2015

highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 14, 2015

nox force-pushed the unforgeable branch from ba41013 to 6010f5b Compare October 15, 2015 10:51

nox removed the S-needs-rebase There are merge conflict errors. label Oct 15, 2015

highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 20, 2015

nox force-pushed the unforgeable branch from 6010f5b to 1e62cf3 Compare October 20, 2015 08:58

nox removed the S-needs-rebase There are merge conflict errors. label Oct 20, 2015

highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 25, 2015

highfive assigned Ms2ger Nov 3, 2015

jdm removed the S-awaiting-review There is new code that needs to be reviewed. label Nov 11, 2015

jdm assigned jdm and unassigned Ms2ger Nov 11, 2015

jdm added the C-needs-test label Nov 11, 2015

nox force-pushed the unforgeable branch from 1e62cf3 to 5313497 Compare November 12, 2015 11:21

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 12, 2015

nox removed the S-needs-rebase There are merge conflict errors. label Nov 12, 2015

nox force-pushed the unforgeable branch from 5fec0dd to 5f9115e Compare December 1, 2015 22:22

highfive removed the S-needs-code-changes Changes have not yet been made that were requested by a reviewer. label Dec 1, 2015

jdm added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Dec 2, 2015

nox force-pushed the unforgeable branch from 5f9115e to 6097640 Compare December 2, 2015 21:15

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Dec 2, 2015

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Dec 2, 2015

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Dec 3, 2015

highfive removed the S-tests-failed The changes caused existing tests to fail. label Dec 3, 2015

highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 3, 2015

bors-servo merged commit 6097640 into servo:master Dec 3, 2015

bors-servo mentioned this pull request Dec 3, 2015

Uncomment some [PutForwards] attributes #8005

Closed

jdm mentioned this pull request Dec 13, 2015

Make Event's isTrusted attribute unforgeable #8956

Closed

nox deleted the unforgeable branch December 15, 2015 10:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement [Unforgeable] #7988

Implement [Unforgeable] #7988

nox commented Oct 13, 2015

highfive commented Oct 13, 2015

bors-servo commented Oct 14, 2015

bors-servo commented Oct 20, 2015

bors-servo commented Oct 25, 2015

metajack commented Nov 3, 2015

jdm commented Nov 11, 2015

jdm commented Nov 11, 2015

jdm commented Nov 11, 2015

nox commented Nov 12, 2015

nox commented Dec 1, 2015

jdm commented Dec 2, 2015

nox commented Dec 2, 2015

bors-servo commented Dec 2, 2015

bors-servo commented Dec 3, 2015

bors-servo commented Dec 3, 2015

KiChjang commented Dec 3, 2015

KiChjang commented Dec 3, 2015

bors-servo commented Dec 3, 2015

bors-servo commented Dec 3, 2015

Implement [Unforgeable] #7988

Implement [Unforgeable] #7988

Conversation

nox commented Oct 13, 2015

highfive commented Oct 13, 2015

bors-servo commented Oct 14, 2015

bors-servo commented Oct 20, 2015

bors-servo commented Oct 25, 2015

metajack commented Nov 3, 2015

jdm commented Nov 11, 2015

jdm commented Nov 11, 2015

jdm commented Nov 11, 2015

nox commented Nov 12, 2015

nox commented Dec 1, 2015

jdm commented Dec 2, 2015

nox commented Dec 2, 2015

bors-servo commented Dec 2, 2015

bors-servo commented Dec 3, 2015

bors-servo commented Dec 3, 2015

KiChjang commented Dec 3, 2015

KiChjang commented Dec 3, 2015

bors-servo commented Dec 3, 2015

bors-servo commented Dec 3, 2015