1132522 - Treat false return value from certain Proxy handler methods as failure

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

Split off from bug 1113369 which introduces these boolean "strict mode failure" return values everywhere.

Implementing this for scripted proxies turns out to break a lot of code, so it may be a gut check landing it.

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: part 1 - Treat false return from proxyHandler.defineProperty() as strict mode failure

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: part 2 - Treat false return from proxyHandler.set() as strict mode failure

Comment 3

[Eric Faust [:efaust]]

Comment on attachment 8564257 [details] [diff] [review]
part 2 - Treat false return from proxyHandler.set() as strict mode failure

Review of attachment 8564257 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

::: js/src/tests/ecma_6/TypedArray/from_proxy.js
@@ +17,5 @@
>          log.push("target", target);
>          var h = {
>              defineProperty: function (t, id) {
>                  log.push("define", id);
> +                return true;

shouldn't this be in the other patch?

Comment 4

[Jason Orendorff [:jorendorff]]

(In reply to Eric Faust [:efaust] from comment #3)
> part 2 - Treat false return from proxyHandler.set() as strict mode failure
> >              defineProperty: function (t, id) {
> >                  log.push("define", id);
> > +                return true;
> 
> shouldn't this be in the other patch?

Yeah, but it's never called, so it doesn't matter. :)

Comment 5

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/95fa879e6855
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f393119201c

Comment 6

[Carsten Book [:Tomcat]]

sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=306ea1a7975d for Jetpack Test failures like :

https://treeherder.mozilla.org/logviewer.html#?job_id=7371703&repo=mozilla-inbound

Comment 7

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/46de54aebe951b0c8335b0f7c5894fc5baf1b669
Update proxy handler for ES6

See [bug 1132522](https://bugzilla.mozilla.org/show_bug.cgi?id=1132522).

We definitely need this particular change. I'm about 95% sure this is the only one, since we won't be changing the behavior of legacy Proxy.create() proxies.

https://github.com/mozilla/addon-sdk/commit/4694b75478a49edc9362f25fcfa2dac07c44ec84
Merge pull request #1888 from jorendorff/patch-1

Bug 1132522 - Update proxy handlers in the Addon SDK for ES6 error handling. r=Mossop

Comment 8

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f2a7b760ad5c
https://hg.mozilla.org/integration/mozilla-inbound/rev/911612d952e6

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f2a7b760ad5c
https://hg.mozilla.org/mozilla-central/rev/911612d952e6

Comment 10

[Florian Scholz (Open Web Docs)]

https://developer.mozilla.org/en-US/Firefox/Releases/39#JavaScript
https://developer.mozilla.org/en-US/Firefox/Releases/39/Site_Compatibility#Proxy_handers_may_throw_under_specific_conditions

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/handler/defineProperty
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/handler/set