add use counters for __defineGetter__ failing

add use counters for __defineGetter__ failing

We deviate from spec in that, in our implementation, __defineGetter__ on non-
configurable properties returns false instead of throwing a TypeError. This commit
adds a use counter to track how often we would be throwing an error we currently
avoid, to determine if we can change to align with spec or if the spec is not
implementable.

BUG=v8:5070
Committed: https://crrev.com/b2ce1fa20cd741bc4a63f888ea55291b0a5be620
Cr-Commit-Position: refs/heads/master@{#37259}

[bakkot, 2016-06-20 23:57:08]

The CQ bit was checked by bakkot@google.com

[bakkot, 2016-06-20 23:57:09]

The CQ bit was unchecked by bakkot@google.com

[bakkot, 2016-06-20 23:57:17]

The CQ bit was unchecked by bakkot@google.com

[bakkot, 2016-06-20 23:58:21]

Description was changed from

==========
add use counters for __defineGetter__ failing

We deviate from spec in that, in our implementation, __defineGetter__ on
non-configurable properties returns false instead of throwing a TypeError. This
commit adds a use counter to track how often we would be throwing an error we
currently avoid, to determine if we can change to align with spec or if the spec
is not implementable.

BUG=5070
==========

to

==========
add use counters for __defineGetter__ failing

We deviate from spec in that, in our implementation, __defineGetter__ on
non-configurable properties returns false instead of throwing a TypeError. This
commit adds a use counter to track how often we would be throwing an error we
currently avoid, to determine if we can change to align with spec or if the spec
is not implementable.

BUG=v8:5070
==========

[bakkot, 2016-06-20 23:59:29]

bakkot@google.com changed reviewers:
+ littledan@chromium.org

[Dan Ehrenberg, 2016-06-21 00:04:49]

https://codereview.chromium.org/2089533002/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/2089533002/diff/1/src/builtins.cc#newcode1711
src/builtins.cc:1711: if (!success.FromJust()) {
This will crash if there is an exception. Instead, do

success.IsJust() && !success.fromJust()

You can test this by running on a Proxy whose defineProperty trap throws.

https://codereview.chromium.org/2089533002/diff/1/test/cctest/test-usecounter...
File test/cctest/test-usecounters.cc (right):

https://codereview.chromium.org/2089533002/diff/1/test/cctest/test-usecounter...
test/cctest/test-usecounters.cc:26: // OF THIS SOFTWARE, EVEN IF ADVISED OF THE
POSSIBILITY OF SUCH DAMAGE.
For new code, use an abbreviated copyright notice, e.g., as found in
src/objects.cc. Use the current year.

[bakkot, 2016-06-21 00:48:34]

https://codereview.chromium.org/2089533002/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/2089533002/diff/1/src/builtins.cc#newcode1711
src/builtins.cc:1711: if (!success.FromJust()) {
On 2016/06/21 00:04:49, Dan Ehrenberg wrote:
> This will crash if there is an exception. Instead, do
> 
> success.IsJust() && !success.fromJust()
> 
> You can test this by running on a Proxy whose defineProperty trap throws.

MAYBE_RETURN will have returned if success is Nothing, so the IsJust test is
unnecessary.

https://codereview.chromium.org/2089533002/diff/1/test/cctest/test-usecounter...
File test/cctest/test-usecounters.cc (right):

https://codereview.chromium.org/2089533002/diff/1/test/cctest/test-usecounter...
test/cctest/test-usecounters.cc:26: // OF THIS SOFTWARE, EVEN IF ADVISED OF THE
POSSIBILITY OF SUCH DAMAGE.
On 2016/06/21 00:04:49, Dan Ehrenberg wrote:
> For new code, use an abbreviated copyright notice, e.g., as found in
> src/objects.cc. Use the current year.

Addressed in new patchset.

[bakkot, 2016-06-24 18:55:52]

Description was changed from

==========
add use counters for __defineGetter__ failing

We deviate from spec in that, in our implementation, __defineGetter__ on
non-configurable properties returns false instead of throwing a TypeError. This
commit adds a use counter to track how often we would be throwing an error we
currently avoid, to determine if we can change to align with spec or if the spec
is not implementable.

BUG=v8:5070
==========

to

==========
add use counters for __defineGetter__ failing

We deviate from spec in that, in our implementation, __defineGetter__ on non-
configurable properties returns false instead of throwing a TypeError. This
commit
adds a use counter to track how often we would be throwing an error we currently
avoid, to determine if we can change to align with spec or if the spec is not
implementable.

BUG=v8:5070
==========

[bakkot, 2016-06-24 20:24:08]

The CQ bit was checked by bakkot@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-24 20:24:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-24 20:55:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-24 20:55:35]

Dry run: This issue passed the CQ dry run.

[Dan Ehrenberg, 2016-06-24 21:46:43]

lgtm

[bakkot, 2016-06-24 22:01:16]

The CQ bit was checked by bakkot@google.com

[commit-bot: I haz the power, 2016-06-24 22:01:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-24 22:04:05]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-24 22:05:18]

Description was changed from

==========
add use counters for __defineGetter__ failing

We deviate from spec in that, in our implementation, __defineGetter__ on non-
configurable properties returns false instead of throwing a TypeError. This
commit
adds a use counter to track how often we would be throwing an error we currently
avoid, to determine if we can change to align with spec or if the spec is not
implementable.

BUG=v8:5070
==========

to

==========
add use counters for __defineGetter__ failing

We deviate from spec in that, in our implementation, __defineGetter__ on non-
configurable properties returns false instead of throwing a TypeError. This
commit
adds a use counter to track how often we would be throwing an error we currently
avoid, to determine if we can change to align with spec or if the spec is not
implementable.

BUG=v8:5070

Committed: https://crrev.com/b2ce1fa20cd741bc4a63f888ea55291b0a5be620
Cr-Commit-Position: refs/heads/master@{#37259}
==========

[commit-bot: I haz the power, 2016-06-24 22:05:19]

Patchset 2 (id:??) landed as
https://crrev.com/b2ce1fa20cd741bc4a63f888ea55291b0a5be620
Cr-Commit-Position: refs/heads/master@{#37259}

[jochen (gone - plz use gerrit), 2016-07-19 12:35:23]

jochen@chromium.org changed reviewers:
+ jochen@chromium.org

[jochen (gone - plz use gerrit), 2016-07-19 12:35:24]

https://codereview.chromium.org/2089533002/diff/20001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/2089533002/diff/20001/include/v8.h#newcode5697
include/v8.h:5697: // If you add new values here, you'll also need to update
Chromium's:
I want to draw your attention to this comment ^^^