From bfedec6a98f01bcdf5e8abed8b26453d07e7e582 Mon Sep 17 00:00:00 2001
From: davidot <davidot@serenityos.org>
Date: Wed, 9 Mar 2022 17:54:55 +0100
Subject: [PATCH] LibJS: Keep PrivateEnvironment through NativeFunction calls

Previously the variable and lexical environments were already kept in a
NativeFunction call. However when we (try to) call a private method from
within an async function we go through async_block_start which sets up
a NativeFunction to call.
This is technically not exactly as the spec describes it, as that
requires you to actually "continue" the context. Since we don't have
that concept (yet) we use this as an implementation detail to access the
private environment from within a native function.

Note that this not allow general private environment access since most
things get blocked by the parser already.
---
 .../LibJS/Runtime/NativeFunction.cpp          |  3 +
 .../LibJS/Tests/classes/class-methods.js      | 63 +++++++++++++++++++
 .../LibJS/Tests/classes/class-static.js       | 61 ++++++++++++++++++
 3 files changed, 127 insertions(+)

diff --git a/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp b/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp
index a26db38685..ab4bf8544e 100644
--- a/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp
+++ b/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp
@@ -139,6 +139,9 @@ ThrowCompletionOr<Value> NativeFunction::internal_call(Value this_argument, Mark
 
     callee_context.lexical_environment = caller_context.lexical_environment;
     callee_context.variable_environment = caller_context.variable_environment;
+    // Note: Keeping the private environment is probably only needed because of async methods in classes
+    //       calling async_block_start which goes through a NativeFunction here.
+    callee_context.private_environment = caller_context.private_environment;
 
     // NOTE: This is a LibJS specific hack for NativeFunction to inherit the strictness of its caller.
     callee_context.is_strict_mode = vm.in_strict_mode();
diff --git a/Userland/Libraries/LibJS/Tests/classes/class-methods.js b/Userland/Libraries/LibJS/Tests/classes/class-methods.js
index d300c555fe..e99eba212e 100644
--- a/Userland/Libraries/LibJS/Tests/classes/class-methods.js
+++ b/Userland/Libraries/LibJS/Tests/classes/class-methods.js
@@ -61,3 +61,66 @@ test("method named 'async'", () => {
     expect("async" in a).toBeTrue();
     expect(a.async()).toBe("function named async");
 });
+
+test("can call other private methods from methods", () => {
+    class A {
+        #a() {
+            return 1;
+        }
+
+        async #b() {
+            return 2;
+        }
+
+        syncA() {
+            return this.#a();
+        }
+
+        async asyncA() {
+            return this.#a();
+        }
+
+        syncB() {
+            return this.#b();
+        }
+
+        async asyncB() {
+            return this.#b();
+        }
+    }
+
+    var called = false;
+
+    async function check() {
+        called = true;
+        const a = new A();
+
+        expect(a.syncA()).toBe(1);
+        expect(await a.asyncA()).toBe(1);
+        expect(await a.syncB()).toBe(2);
+        expect(await a.asyncB()).toBe(2);
+        return 3;
+    }
+
+    var error = null;
+    var failed = false;
+
+    check().then(
+        value => {
+            expect(called).toBeTrue();
+            expect(value).toBe(3);
+        },
+        thrownError => {
+            failed = true;
+            error = thrownError;
+        }
+    );
+
+    runQueuedPromiseJobs();
+
+    expect(called).toBeTrue();
+
+    if (failed) throw error;
+
+    expect(failed).toBeFalse();
+});
diff --git a/Userland/Libraries/LibJS/Tests/classes/class-static.js b/Userland/Libraries/LibJS/Tests/classes/class-static.js
index cb2436cf33..28bed5b489 100644
--- a/Userland/Libraries/LibJS/Tests/classes/class-static.js
+++ b/Userland/Libraries/LibJS/Tests/classes/class-static.js
@@ -81,3 +81,64 @@ test("static function named 'async'", () => {
     expect("async" in A).toBeTrue();
     expect(A.async()).toBe("static function named async");
 });
+
+test("can call other private methods from static method", () => {
+    class A {
+        static #a() {
+            return 1;
+        }
+
+        static async #b() {
+            return 2;
+        }
+
+        static syncA() {
+            return this.#a();
+        }
+
+        static async asyncA() {
+            return this.#a();
+        }
+
+        static syncB() {
+            return this.#b();
+        }
+
+        static async asyncB() {
+            return this.#b();
+        }
+    }
+
+    var called = false;
+
+    async function check() {
+        called = true;
+        expect(A.syncA()).toBe(1);
+        expect(await A.asyncA()).toBe(1);
+        expect(await A.syncB()).toBe(2);
+        expect(await A.asyncB()).toBe(2);
+        return 3;
+    }
+
+    var error = null;
+    var failed = false;
+
+    check().then(
+        value => {
+            expect(called).toBeTrue();
+            expect(value).toBe(3);
+        },
+        thrownError => {
+            failed = true;
+            error = thrownError;
+        }
+    );
+
+    runQueuedPromiseJobs();
+
+    expect(called).toBeTrue();
+
+    if (failed) throw error;
+
+    expect(failed).toBeFalse();
+});