Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the call to goja runtime from elementHandle.newPointerAction #1170

Closed
2 tasks done
Tracked by #1182
ankur22 opened this issue Jan 19, 2024 · 0 comments
Closed
2 tasks done
Tracked by #1182

Remove the call to goja runtime from elementHandle.newPointerAction #1170

ankur22 opened this issue Jan 19, 2024 · 0 comments
Assignees
Labels
bug Something isn't working refactor

Comments

@ankur22
Copy link
Collaborator

ankur22 commented Jan 19, 2024

What

Can elementHandle.newPointerAction be de-gojafied? All click APIs use it.

How

After #1189, there's only one method to de-gojafy: ElementHandle.offsetPosition.

Tasks

  1. bug refactor
    inancgumus
  2. inancgumus
@ankur22 ankur22 changed the title Can [elementHandle.newPointerAction](https://github.com/grafana/xk6-browser/blob/5f754c2c2252896016fd77d1fd3234a795049a0e/common/element_handle.go#L1407-L1492) be de-gojafied? All click APIs use it. Can elementHandle.newPointerAction be de-gojafied? All click APIs use it. Jan 19, 2024
@ankur22 ankur22 changed the title Can elementHandle.newPointerAction be de-gojafied? All click APIs use it. Remove the call to goja runtime from elementHandle.newPointerAction Jan 19, 2024
ankur22 added a commit that referenced this issue Jan 19, 2024
Move the parsing of the options for elementHandle.click outside of the
promise goroutine and back onto the main goja thread in the mapping
layer. This will help mitigate the risk of a panic if more than one
goroutine is accessing the goja runtime off the main goja thread.

This doesn't solve the problem completely though since this API calls
to other areas of the codebase which does still interact with the goja
runtime. See #1170 for
further details.
ankur22 added a commit that referenced this issue Jan 19, 2024
Move the parsing of the options for elementHandle.click outside of the
promise goroutine and back onto the main goja thread in the mapping
layer. This will help mitigate the risk of a panic if more than one
goroutine is accessing the goja runtime off the main goja thread.

This doesn't solve the problem completely though since this API calls
to other areas of the codebase which does still interact with the goja
runtime. See #1170 for
further details.
ankur22 added a commit that referenced this issue Jan 19, 2024
Move the parsing of the options for elementHandle.click outside of the
promise goroutine and back onto the main goja thread in the mapping
layer. This will help mitigate the risk of a panic if more than one
goroutine is accessing the goja runtime off the main goja thread.

This doesn't solve the problem completely though since this API calls
to other areas of the codebase which does still interact with the goja
runtime. See #1170 for
further details.
ankur22 added a commit that referenced this issue Jan 19, 2024
Move the parsing of the options for elementHandle.click outside of the
promise goroutine and back onto the main goja thread in the mapping
layer. This will help mitigate the risk of a panic if more than one
goroutine is accessing the goja runtime off the main goja thread.

This doesn't solve the problem completely though since this API calls
to other areas of the codebase which does still interact with the goja
runtime. See #1170 for
further details.
ankur22 added a commit that referenced this issue Jan 19, 2024
This helps avoid using the goja runtime when it comes to parsing the
options. The options parsing has been moved to the mapping layer and so
has been moved back on to the main goja thread, which will mitigate the
possibility of a panic which could occur when multiple goroutines try
to work with the goja runtime (which is not thread safe).

There is another issue to tackle more of the goja refactoring in
#1170.

The change also adds a temporary fix for page.click which will also be
refactored at a later stage.
ankur22 added a commit that referenced this issue Jan 19, 2024
This helps avoid using the goja runtime when it comes to parsing the
options. The options parsing has been moved to the mapping layer and so
has been moved back on to the main goja thread, which will mitigate the
possibility of a panic which could occur when multiple goroutines try
to work with the goja runtime (which is not thread safe).

There is another issue to tackle more of the goja refactoring in
#1170.

The change also adds a temporary fix for page.click which will also be
refactored at a later stage.
ankur22 added a commit that referenced this issue Jan 19, 2024
This helps avoid using the goja runtime when it comes to parsing the
options. The options parsing has been moved to the mapping layer and so
has been moved back on to the main goja thread, which will mitigate the
possibility of a panic which could occur when multiple goroutines try
to work with the goja runtime (which is not thread safe).

There is another issue to tackle more of the goja refactoring in
#1170.

The change also adds a temporary fix for page.click which will also be
refactored at a later stage.
@ankur22 ankur22 self-assigned this Jan 22, 2024
@ankur22 ankur22 added bug Something isn't working async supports async (promises) and removed async supports async (promises) labels Jan 22, 2024
ankur22 added a commit that referenced this issue Jan 23, 2024
Move the parsing of the options for elementHandle.click outside of the
promise goroutine and back onto the main goja thread in the mapping
layer. This will help mitigate the risk of a panic if more than one
goroutine is accessing the goja runtime off the main goja thread.

This doesn't solve the problem completely though since this API calls
to other areas of the codebase which does still interact with the goja
runtime. See #1170 for
further details.
ankur22 added a commit that referenced this issue Jan 23, 2024
This helps avoid using the goja runtime when it comes to parsing the
options. The options parsing has been moved to the mapping layer and so
has been moved back on to the main goja thread, which will mitigate the
possibility of a panic which could occur when multiple goroutines try
to work with the goja runtime (which is not thread safe).

There is another issue to tackle more of the goja refactoring in
#1170.

The change also adds a temporary fix for page.click which will also be
refactored at a later stage.
ankur22 added a commit that referenced this issue Jan 23, 2024
Move the parsing of the options for elementHandle.click outside of the
promise goroutine and back onto the main goja thread in the mapping
layer. This will help mitigate the risk of a panic if more than one
goroutine is accessing the goja runtime off the main goja thread.

This doesn't solve the problem completely though since this API calls
to other areas of the codebase which does still interact with the goja
runtime. See #1170 for
further details.
ankur22 added a commit that referenced this issue Jan 23, 2024
This helps avoid using the goja runtime when it comes to parsing the
options. The options parsing has been moved to the mapping layer and so
has been moved back on to the main goja thread, which will mitigate the
possibility of a panic which could occur when multiple goroutines try
to work with the goja runtime (which is not thread safe).

There is another issue to tackle more of the goja refactoring in
#1170.

The change also adds a temporary fix for page.click which will also be
refactored at a later stage.
ankur22 added a commit that referenced this issue Jan 23, 2024
This helps avoid using the goja runtime when it comes to parsing the
options. The options parsing has been moved to the mapping layer and so
has been moved back on to the main goja thread, which will mitigate the
possibility of a panic which could occur when multiple goroutines try
to work with the goja runtime (which is not thread safe).

There is another issue to tackle more of the goja refactoring in
#1170.

The change also adds a temporary fix for page.click which will also be
refactored at a later stage.
@ankur22 ankur22 removed their assignment Jan 23, 2024
@inancgumus inancgumus self-assigned this Jan 24, 2024
@inancgumus inancgumus reopened this Jan 26, 2024
inancgumus pushed a commit that referenced this issue Jan 29, 2024
Move the parsing of the options for elementHandle.click outside of the
promise goroutine and back onto the main goja thread in the mapping
layer. This will help mitigate the risk of a panic if more than one
goroutine is accessing the goja runtime off the main goja thread.

This doesn't solve the problem completely though since this API calls
to other areas of the codebase which does still interact with the goja
runtime. See #1170 for
further details.
inancgumus pushed a commit that referenced this issue Jan 29, 2024
This helps avoid using the goja runtime when it comes to parsing the
options. The options parsing has been moved to the mapping layer and so
has been moved back on to the main goja thread, which will mitigate the
possibility of a panic which could occur when multiple goroutines try
to work with the goja runtime (which is not thread safe).

There is another issue to tackle more of the goja refactoring in
#1170.

The change also adds a temporary fix for page.click which will also be
refactored at a later stage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working refactor
Projects
None yet
Development

No branches or pull requests

2 participants