Fix setContent racy access to options

grafana · Feb 11, 2025 · f7fa5af · f7fa5af
1 parent 9bc5e49
commit f7fa5af
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 16 deletions.
diff --git a/internal/js/modules/k6/browser/browser/frame_mapping.go b/internal/js/modules/k6/browser/browser/frame_mapping.go
@@ -275,10 +275,14 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping {
 				return nil, f.SetChecked(selector, checked, popts) //nolint:wrapcheck
 			}), nil
 		},
-		"setContent": func(html string, opts sobek.Value) *sobek.Promise {
+		"setContent": func(html string, opts sobek.Value) (*sobek.Promise, error) {
+			popts := common.NewFrameSetContentOptions(f.Page().NavigationTimeout())
+			if err := popts.Parse(vu.Context(), opts); err != nil {
+				return nil, fmt.Errorf("parsing setContent options: %w", err)
+			}
 			return k6ext.Promise(vu.Context(), func() (any, error) {
-				return nil, f.SetContent(html, opts) //nolint:wrapcheck
-			})
+				return nil, f.SetContent(html, popts) //nolint:wrapcheck
+			}), nil
 		},
 		"setInputFiles": func(selector string, files sobek.Value, opts sobek.Value) (*sobek.Promise, error) {
 			popts := common.NewFrameSetInputFilesOptions(f.Timeout())

diff --git a/internal/js/modules/k6/browser/browser/page_mapping.go b/internal/js/modules/k6/browser/browser/page_mapping.go
@@ -349,11 +349,14 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
 				return nil, p.SetChecked(selector, checked, popts) //nolint:wrapcheck
 			}), nil
 		},
-		"setContent": func(html string, opts sobek.Value) *sobek.Promise {
+		"setContent": func(html string, opts sobek.Value) (*sobek.Promise, error) {
+			popts := common.NewFrameSetContentOptions(p.MainFrame().NavigationTimeout())
+			if err := popts.Parse(vu.Context(), opts); err != nil {
+				return nil, fmt.Errorf("parsing setContent options: %w", err)
+			}
 			return k6ext.Promise(vu.Context(), func() (any, error) {
-				// TODO(@mstoykov): don't use sobek Values in a separate goroutine
-				return nil, p.SetContent(html, opts) //nolint:wrapcheck
-			})
+				return nil, p.SetContent(html, popts) //nolint:wrapcheck
+			}), nil
 		},
 		"setDefaultNavigationTimeout": p.SetDefaultNavigationTimeout,
 		"setDefaultTimeout":           p.SetDefaultTimeout,

diff --git a/internal/js/modules/k6/browser/common/frame.go b/internal/js/modules/k6/browser/common/frame.go
@@ -1474,16 +1474,9 @@ func (f *Frame) selectOption(selector string, values sobek.Value, opts *FrameSel
 }
 
 // SetContent replaces the entire HTML document content.
-func (f *Frame) SetContent(html string, opts sobek.Value) error {
+func (f *Frame) SetContent(html string, _ *FrameSetContentOptions) error {
 	f.log.Debugf("Frame:SetContent", "fid:%s furl:%q", f.ID(), f.URL())
 
-	parsedOpts := NewFrameSetContentOptions(
-		f.manager.timeoutSettings.navigationTimeout(),
-	)
-	if err := parsedOpts.Parse(f.ctx, opts); err != nil {
-		return fmt.Errorf("parsing set content options: %w", err)
-	}
-
 	js := `(html) => {
 		window.stop();
 		document.open();

diff --git a/internal/js/modules/k6/browser/common/page.go b/internal/js/modules/k6/browser/common/page.go
@@ -1332,7 +1332,7 @@ func (p *Page) SelectOption(selector string, values sobek.Value, popts *FrameSel
 }
 
 // SetContent replaces the entire HTML document content.
-func (p *Page) SetContent(html string, opts sobek.Value) error {
+func (p *Page) SetContent(html string, opts *FrameSetContentOptions) error {
 	p.logger.Debugf("Page:SetContent", "sid:%v", p.sessionID())
 
 	return p.MainFrame().SetContent(html, opts)