Throw TypeError if [[DefineOwnProperty]] returns false

[ShortDevelopment]

Issue

According to es spec Object.defineProperty should throw if internal [[DefineOwnProperty]] returns false-ish.
This happens specifically if the defineProperty proxy trap returns false (See #6505).

Changes

• Throw TypeError in JavascriptObject::EntryDefineProperty if DefineOwnPropertyHelper returns false-ish
• Changed content of JSERR_ProxyHandlerReturnedFalse
• Routed PropertyOperationFlags through the call stack

Open questions

• Why is test\Bugs\misc_bugs.js tagged with exclude_windows?

---

@rhuanjl

Related to PR #5412
Fixes #6505

[rhuanjl]

test\Bugs\misc_bugs.js being marked to exclude_windows looks like a mistake (I can see the PR where that was set - it was me that did it but... I can't see why I did) unless there's a weird test in there that can't be run on windows, I'd suggest removing the tag and seeing if the tests pass.

[rhuanjl]

Why should this always be strict mode?

[ShortDevelopment]

As far as I understood, calls to Object.defineXXX should always throw on error (be strict) opposed to using the assignment operator (property = value).

The error is actually only thrown, if throwOnError == true.

ChakraCore/lib/Runtime/Library/JavascriptProxy.cpp

Line 1762 in fcdbd7b

if (throwOnError && flags & PropertyOperation_StrictMode)

[ljharb]

indeed, i'm pretty sure all builtins always are effectively ran in strict mode

[ShortDevelopment]

I guess it seems strange as (in my understanding) PropertyOperation_StrictModein CC code base always meant “strict mode” has been turned on explicitly; That’s reflected by CC error messages like

ChakraCore/lib/Parser/rterrors.h

Line 155 in 320a7a8

RT_ERROR_MSG(JSERR_CantDeleteExpr, 5041, "Calling delete on '%s' is not allowed in strict mode", "Object member not configurable", kjstTypeError, 0) // string 4

The error messages throw by me do not explicitly state “strict mode” (According to the behaviour of node).

[rhuanjl]

The strict mode check in the Proxy Trap seems like it may be wrong - this call should throw if it's strict mode or not.

Trying to think if there's any case where throwOnError would be true and strict mode would be false and it should NOT throw - what could actually get there?

[ShortDevelopment]

You could use the assignment operator in non-strict mode; that should not throw.

ChakraCore/test/Bugs/misc_bugs.js

Lines 202 to 204 in 625ba77

	assert.doesNotThrow(() => {
	proxy.a = {};
	}, "Set property in NON-strict mode does NOT throw if trap returns falsy");

[rhuanjl]

But would the throwOnError parameter be true at that point?

[ShortDevelopment]

It seems to be like throwOnError is true in both scenarios so we need a seperate way to check for strict mode.

[rhuanjl]

test\Bugs\misc_bugs.js being marked to exclude_windows looks like a mistake (I can see the PR where that was set - it was me that did it but... I can't see why I did) unless there's a weird test in there that can't be run on windows, I'd suggest removing the tag and seeing if the tests pass.

Unfortunately there clearly is an issue with that test case on windows - I should probably try and fix it but, that's beyond the scope of this PR can you drop your last commit.

[rhuanjl]

Note on the test fails:

1. exclude_windows for misc_bugs is an unrelated issue I'll look into
2. macOS test fail is an unrelated issue brought in by an update to the macOS test image being used - I'll investigate that seperately
3. Windows x86 is an unrelated known intermittent bug

Hence approving in spite of these.