NativeScript / android

NativeScript for Android using v8
https://docs.nativescript.org/guide/android-marshalling
Apache License 2.0
519 stars 135 forks source link

Don't store V8 context in callback info #1760

Closed ptomato closed 1 year ago

ptomato commented 1 year ago

Description

This change is from an idea by @edusperoni, that it might not be necessary to store a Persistent<Context> in frame and main thread callback info. That seems to be the case indeed, if we use GetCreationContextChecked() to get the context that the callback JS function belongs to and use that instead. GetCreationContextChecked() can potentially be more expensive in terms of performance than storing the context in a persistent, but I estimate it shouldn't be too bad since it's called only once for each execution of a callback.

Related Pull Requests

None.

Does your pull request have unit tests?

I believe this is covered by the unit tests at least partly. I also added the following snippet to MyActivity.js in the test app to test manually that the callbacks continue to work:

--- a/test-app/app/src/main/assets/app/MyActivity.js
+++ b/test-app/app/src/main/assets/app/MyActivity.js
@@ -56,6 +56,13 @@ var MyActivity = (function (_super) {
                        onClick: function () {
                                button.setBackgroundColor(colors[taps % colors.length]);
                                taps++;
+                              textView.setText('Waiting for frame callback...');
+                              __postFrameCallback(() => {
+                                  textView.setText(`Message after one second: ${new Date()}`);
+                              }, 1000 /* ms */);
+                              __runOnMainThread(() => {
+                                  button.setText(`Hit me ${5 - taps} more times`);
+                              });
                        },
                })
         );