From c6e9c6d6ab7fb68b6a9cbabab255c87b8d63395e Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Mon, 12 Jul 2021 01:37:51 +0200
Subject: [PATCH] LibJS: Follow the spec more closely when determining the this
 value

Co-authored-by: davidot <david.tuin@gmail.com>
---
 Userland/Libraries/LibJS/AST.cpp        |  8 ++----
 Userland/Libraries/LibJS/Runtime/VM.cpp | 37 +++++++++++++++++++++----
 Userland/Libraries/LibJS/Runtime/VM.h   |  2 ++
 3 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp
index 674471acf0..5867f5b56d 100644
--- a/Userland/Libraries/LibJS/AST.cpp
+++ b/Userland/Libraries/LibJS/AST.cpp
@@ -161,12 +161,8 @@ CallExpression::ThisAndCallee CallExpression::compute_this_and_callee(Interprete
         return { this_value, callee };
     }
 
-    if (interpreter.vm().in_strict_mode()) {
-        // If we are in strict mode, |this| should never be bound to global object by default.
-        return { js_undefined(), m_callee->execute(interpreter, global_object) };
-    }
-
-    return { &global_object, m_callee->execute(interpreter, global_object) };
+    // [[Call]] will handle that in non-strict mode the this value becomes the global object
+    return { js_undefined(), m_callee->execute(interpreter, global_object) };
 }
 
 // 13.3.8.1 Runtime Semantics: ArgumentListEvaluation, https://tc39.es/ecma262/#sec-runtime-semantics-argumentlistevaluation
diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp
index 808c564026..9503b56b6e 100644
--- a/Userland/Libraries/LibJS/Runtime/VM.cpp
+++ b/Userland/Libraries/LibJS/Runtime/VM.cpp
@@ -589,6 +589,36 @@ void VM::prepare_for_ordinary_call(FunctionObject& function, ExecutionContext& c
     // 15. Return calleeContext. (See NOTE above about how contexts are allocated on the C++ stack.)
 }
 
+// 10.2.1.2 OrdinaryCallBindThis ( F, calleeContext, thisArgument ), https://tc39.es/ecma262/#sec-ordinarycallbindthis
+void VM::ordinary_call_bind_this(FunctionObject& function, ExecutionContext& callee_context, Value this_argument)
+{
+    auto this_mode = function.this_mode();
+    auto* callee_realm = function.realm();
+
+    auto* local_environment = callee_context.lexical_environment;
+    auto& function_environment = verify_cast<FunctionEnvironment>(*local_environment);
+
+    // This is not completely as the spec describes it however without this stuff breaks
+    // (Could be related to the note at https://tc39.es/ecma262/#sec-runtime-semantics-instantiatearrowfunctionexpression )
+    if (!callee_realm || this_mode == FunctionObject::ThisMode::Lexical) {
+        function_environment.bind_this_value(function.global_object(), callee_context.this_value);
+        return;
+    }
+
+    Value this_value;
+    if (function.is_strict_mode()) {
+        this_value = this_argument;
+    } else if (this_argument.is_nullish()) {
+        auto& global_environment = callee_realm->environment();
+        this_value = global_environment.global_this_value();
+    } else {
+        this_value = this_argument.to_object(function.global_object());
+    }
+
+    function_environment.bind_this_value(function.global_object(), this_value);
+    callee_context.this_value = this_value;
+}
+
 Value VM::call_internal(FunctionObject& function, Value this_value, Optional<MarkedValueList> arguments)
 {
     VERIFY(!exception());
@@ -611,11 +641,8 @@ Value VM::call_internal(FunctionObject& function, Value this_value, Optional<Mar
     if (arguments.has_value())
         callee_context.arguments.extend(arguments.value().values());
 
-    if (auto* environment = callee_context.lexical_environment) {
-        auto& function_environment = verify_cast<FunctionEnvironment>(*environment);
-        VERIFY(function_environment.this_binding_status() == FunctionEnvironment::ThisBindingStatus::Uninitialized);
-        function_environment.bind_this_value(function.global_object(), callee_context.this_value);
-    }
+    if (callee_context.lexical_environment)
+        ordinary_call_bind_this(function, callee_context, this_value);
 
     if (exception())
         return {};
diff --git a/Userland/Libraries/LibJS/Runtime/VM.h b/Userland/Libraries/LibJS/Runtime/VM.h
index 06e5783d54..d06d8b5d7f 100644
--- a/Userland/Libraries/LibJS/Runtime/VM.h
+++ b/Userland/Libraries/LibJS/Runtime/VM.h
@@ -260,6 +260,8 @@ public:
 private:
     VM();
 
+    void ordinary_call_bind_this(FunctionObject&, ExecutionContext&, Value this_argument);
+
     [[nodiscard]] Value call_internal(FunctionObject&, Value this_value, Optional<MarkedValueList> arguments);
     void prepare_for_ordinary_call(FunctionObject&, ExecutionContext& callee_context, Value new_target);