From 8b21fdf65f7e7a87b7412f30adceaec8d3388e49 Mon Sep 17 00:00:00 2001
From: ankur22 <ankur.agarwal@grafana.com>
Date: Fri, 19 Jan 2024 17:18:22 +0000
Subject: [PATCH] Refactor parse out of frame.click

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
https://github.com/grafana/xk6-browser/issues/1170.

The change also adds a temporary fix for page.click which will also be
refactored at a later stage.
---
 browser/mapping.go                  | 11 ++++++++---
 common/frame.go                     |  8 ++------
 common/page.go                      |  7 ++++++-
 tests/launch_options_slowmo_test.go |  2 +-
 4 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/browser/mapping.go b/browser/mapping.go
index 480cf6726..a110e69af 100644
--- a/browser/mapping.go
+++ b/browser/mapping.go
@@ -327,11 +327,16 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
 			}
 			return rt.ToValue(mcfs).ToObject(rt)
 		},
-		"click": func(selector string, opts goja.Value) *goja.Promise {
+		"click": func(selector string, opts goja.Value) (*goja.Promise, error) {
+			popts, err := parseFrameClickOptions(vu.Context(), opts, f.Timeout())
+			if err != nil {
+				return nil, err
+			}
+
 			return k6ext.Promise(vu.Context(), func() (any, error) {
-				err := f.Click(selector, opts)
+				err := f.Click(selector, popts)
 				return nil, err //nolint:wrapcheck
-			})
+			}), nil
 		},
 		"content":       f.Content,
 		"dblclick":      f.Dblclick,
diff --git a/common/frame.go b/common/frame.go
index 90bb53557..c0e6a8d8f 100644
--- a/common/frame.go
+++ b/common/frame.go
@@ -545,14 +545,10 @@ func (f *Frame) ChildFrames() []*Frame {
 }
 
 // Click clicks the first element found that matches selector.
-func (f *Frame) Click(selector string, opts goja.Value) error {
+func (f *Frame) Click(selector string, opts *FrameClickOptions) error {
 	f.log.Debugf("Frame:Click", "fid:%s furl:%q sel:%q", f.ID(), f.URL(), selector)
 
-	popts := NewFrameClickOptions(f.defaultTimeout())
-	if err := popts.Parse(f.ctx, opts); err != nil {
-		k6ext.Panic(f.ctx, "parsing click options %q: %w", selector, err)
-	}
-	if err := f.click(selector, popts); err != nil {
+	if err := f.click(selector, opts); err != nil {
 		return fmt.Errorf("clicking on %q: %w", selector, err)
 	}
 
diff --git a/common/page.go b/common/page.go
index 12f270bc3..3b5be5c40 100644
--- a/common/page.go
+++ b/common/page.go
@@ -652,7 +652,12 @@ func (p *Page) IsChecked(selector string, opts goja.Value) bool {
 func (p *Page) Click(selector string, opts goja.Value) error {
 	p.logger.Debugf("Page:Click", "sid:%v selector:%s", p.sessionID(), selector)
 
-	return p.MainFrame().Click(selector, opts) //nolint:wrapcheck
+	popts := NewFrameClickOptions(p.defaultTimeout())
+	if err := popts.Parse(p.ctx, opts); err != nil {
+		k6ext.Panic(p.ctx, "parsing click options %q: %w", selector, err)
+	}
+
+	return p.MainFrame().Click(selector, popts)
 }
 
 // Close closes the page.
diff --git a/tests/launch_options_slowmo_test.go b/tests/launch_options_slowmo_test.go
index c79a29870..48e344c28 100644
--- a/tests/launch_options_slowmo_test.go
+++ b/tests/launch_options_slowmo_test.go
@@ -172,7 +172,7 @@ func TestBrowserOptionsSlowMo(t *testing.T) {
 			t.Parallel()
 			tb := newTestBrowser(t, withFileServer())
 			testFrameSlowMoImpl(t, tb, func(_ *testBrowser, f *common.Frame) {
-				err := f.Click("button", nil)
+				err := f.Click("button", common.NewFrameClickOptions(f.Timeout()))
 				assert.NoError(t, err)
 			})
 		})