Desobekify Browser

- Moves BrowserContextOptions out of the business logic.
- Sets default options in NewBrowserContext to prevent nil pointer
  issues in the rest of the module. Later, we might think about turning
some of these pointer fields into value types to take advantage of their
zero values.

Author: inancgumus

browser/browser_mapping.go

@@ -10,7 +10,7 @@ import (
 )
 
 // mapBrowser to the JS module.
-func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop
+func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop,gocognit
 	return mapping{
 		"context": func() (mapping, error) {
 			b, err := vu.browser()
@@ -36,23 +36,24 @@ func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop
 			return b.IsConnected(), nil
 		},
 		"newContext": func(opts sobek.Value) (*sobek.Promise, error) {
+			popts := common.NewBrowserContextOptions()
+			if err := popts.Parse(vu.Context(), opts); err != nil {
+				return nil, fmt.Errorf("parsing browser.newContext options: %w", err)
+			}
 			return k6ext.Promise(vu.Context(), func() (any, error) {
 				b, err := vu.browser()
 				if err != nil {
 					return nil, err
 				}
-				bctx, err := b.NewContext(opts)
+				bctx, err := b.NewContext(popts)
 				if err != nil {
 					return nil, err //nolint:wrapcheck
 				}
-
 				if err := initBrowserContext(bctx, vu.testRunID); err != nil {
 					return nil, err
 				}
 
-				m := mapBrowserContext(vu, bctx)
-
-				return m, nil
+				return mapBrowserContext(vu, bctx), nil
 			}), nil
 		},
 		"userAgent": func() (string, error) {
@@ -69,23 +70,26 @@ func mapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop
 			}
 			return b.Version(), nil
 		},
-		"newPage": func(opts sobek.Value) *sobek.Promise {
+		"newPage": func(opts sobek.Value) (*sobek.Promise, error) {
+			popts := common.NewBrowserContextOptions()
+			if err := popts.Parse(vu.Context(), opts); err != nil {
+				return nil, fmt.Errorf("parsing browser.newPage options: %w", err)
+			}
 			return k6ext.Promise(vu.Context(), func() (any, error) {
 				b, err := vu.browser()
 				if err != nil {
 					return nil, err
 				}
-				page, err := b.NewPage(opts)
+				page, err := b.NewPage(popts)
 				if err != nil {
 					return nil, err //nolint:wrapcheck
 				}
-
 				if err := initBrowserContext(b.Context(), vu.testRunID); err != nil {
 					return nil, err
 				}
 
 				return mapPage(vu, page), nil
-			})
+			}), nil
 		},
 	}
 }

browser/mapping_test.go

@@ -264,8 +264,8 @@ type browserAPI interface {
 	Context() *common.BrowserContext
 	CloseContext()
 	IsConnected() bool
-	NewContext(opts sobek.Value) (*common.BrowserContext, error)
-	NewPage(opts sobek.Value) (*common.Page, error)
+	NewContext(opts *common.BrowserContextOptions) (*common.BrowserContext, error)
+	NewPage(opts *common.BrowserContextOptions) (*common.Page, error)
 	On(string) (bool, error)
 	UserAgent() string
 	Version() string

browser/sync_browser_mapping.go

@@ -1,7 +1,11 @@
 package browser
 
 import (
+	"fmt"
+
 	"github.com/grafana/sobek"
+
+	"github.com/grafana/xk6-browser/common"
 )
 
 // syncMapBrowser is like mapBrowser but returns synchronous functions.
@@ -30,11 +34,16 @@ func syncMapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop
 			return b.IsConnected(), nil
 		},
 		"newContext": func(opts sobek.Value) (*sobek.Object, error) {
+			popts := common.NewBrowserContextOptions()
+			if err := popts.Parse(vu.Context(), opts); err != nil {
+				return nil, fmt.Errorf("parsing browser.newContext options: %w", err)
+			}
+
 			b, err := vu.browser()
 			if err != nil {
 				return nil, err
 			}
-			bctx, err := b.NewContext(opts)
+			bctx, err := b.NewContext(popts)
 			if err != nil {
 				return nil, err //nolint:wrapcheck
 			}
@@ -62,11 +71,16 @@ func syncMapBrowser(vu moduleVU) mapping { //nolint:funlen,cyclop
 			return b.Version(), nil
 		},
 		"newPage": func(opts sobek.Value) (mapping, error) {
+			popts := common.NewBrowserContextOptions()
+			if err := popts.Parse(vu.Context(), opts); err != nil {
+				return nil, fmt.Errorf("parsing browser.newContext options: %w", err)
+			}
+
 			b, err := vu.browser()
 			if err != nil {
 				return nil, err
 			}
-			page, err := b.NewPage(opts)
+			page, err := b.NewPage(popts)
 			if err != nil {
 				return nil, err //nolint:wrapcheck
 			}

common/browser.go

@@ -14,7 +14,6 @@ import (
 	"github.com/chromedp/cdproto/cdp"
 	"github.com/chromedp/cdproto/target"
 	"github.com/gorilla/websocket"
-	"github.com/grafana/sobek"
 
 	"github.com/grafana/xk6-browser/k6ext"
 	"github.com/grafana/xk6-browser/log"
@@ -569,7 +568,7 @@ func (b *Browser) IsConnected() bool {
 }
 
 // NewContext creates a new incognito-like browser context.
-func (b *Browser) NewContext(opts sobek.Value) (*BrowserContext, error) {
+func (b *Browser) NewContext(opts *BrowserContextOptions) (*BrowserContext, error) {
 	_, span := TraceAPICall(b.ctx, "", "browser.newContext")
 	defer span.End()
 
@@ -588,14 +587,7 @@ func (b *Browser) NewContext(opts sobek.Value) (*BrowserContext, error) {
 		return nil, err
 	}
 
-	browserCtxOpts := NewBrowserContextOptions()
-	if err := browserCtxOpts.Parse(b.ctx, opts); err != nil {
-		err := fmt.Errorf("parsing newContext options: %w", err)
-		spanRecordError(span, err)
-		return nil, err
-	}
-
-	browserCtx, err := NewBrowserContext(b.ctx, b, browserContextID, browserCtxOpts, b.logger)
+	browserCtx, err := NewBrowserContext(b.ctx, b, browserContextID, opts, b.logger)
 	if err != nil {
 		err := fmt.Errorf("new context: %w", err)
 		spanRecordError(span, err)
@@ -610,7 +602,7 @@ func (b *Browser) NewContext(opts sobek.Value) (*BrowserContext, error) {
 }
 
 // NewPage creates a new tab in the browser window.
-func (b *Browser) NewPage(opts sobek.Value) (*Page, error) {
+func (b *Browser) NewPage(opts *BrowserContextOptions) (*Page, error) {
 	_, span := TraceAPICall(b.ctx, "", "browser.newPage")
 	defer span.End()
 

common/browser_context.go

@@ -90,6 +90,11 @@ type BrowserContext struct {
 func NewBrowserContext(
 	ctx context.Context, browser *Browser, id cdp.BrowserContextID, opts *BrowserContextOptions, logger *log.Logger,
 ) (*BrowserContext, error) {
+	// set the default options if none provided.
+	if opts == nil {
+		opts = NewBrowserContextOptions()
+	}
+
 	b := BrowserContext{
 		BaseEventEmitter: NewBaseEventEmitter(ctx),
 		ctx:              ctx,
@@ -101,7 +106,7 @@ func NewBrowserContext(
 		timeoutSettings:  NewTimeoutSettings(nil),
 	}
 
-	if opts != nil && len(opts.Permissions) > 0 {
+	if len(opts.Permissions) > 0 {
 		err := b.GrantPermissions(opts.Permissions, NewGrantPermissionsOptions())
 		if err != nil {
 			return nil, err

tests/browser_context_options_test.go

@@ -51,14 +51,12 @@ func TestBrowserContextOptionsSetViewport(t *testing.T) {
 	t.Parallel()
 
 	tb := newTestBrowser(t)
-	bctx, err := tb.NewContext(tb.toSobekValue(struct {
-		Viewport common.Viewport `js:"viewport"`
-	}{
-		Viewport: common.Viewport{
-			Width:  800,
-			Height: 600,
-		},
-	}))
+	opts := common.NewBrowserContextOptions()
+	opts.Viewport = &common.Viewport{
+		Width:  800,
+		Height: 600,
+	}
+	bctx, err := tb.NewContext(opts)
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		if err := bctx.Close(); err != nil {
@@ -77,19 +75,19 @@ func TestBrowserContextOptionsExtraHTTPHeaders(t *testing.T) {
 	t.Parallel()
 
 	tb := newTestBrowser(t, withHTTPServer())
-	bctx, err := tb.NewContext(tb.toSobekValue(struct {
-		ExtraHTTPHeaders map[string]string `js:"extraHTTPHeaders"`
-	}{
-		ExtraHTTPHeaders: map[string]string{
-			"Some-Header": "Some-Value",
-		},
-	}))
+
+	opts := common.NewBrowserContextOptions()
+	opts.ExtraHTTPHeaders = map[string]string{
+		"Some-Header": "Some-Value",
+	}
+	bctx, err := tb.NewContext(opts)
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		if err := bctx.Close(); err != nil {
 			t.Log("closing browser context:", err)
 		}
 	})
+
 	p, err := bctx.NewPage()
 	require.NoError(t, err)
 

tests/element_handle_test.go

@@ -181,17 +181,15 @@ func TestElementHandleClickConcealedLink(t *testing.T) {
 	)
 
 	tb := newTestBrowser(t, withFileServer())
-	bc, err := tb.NewContext(
-		tb.toSobekValue(struct {
-			Viewport common.Viewport `js:"viewport"`
-		}{
-			Viewport: common.Viewport{
-				Width:  500,
-				Height: 240,
-			},
-		}),
-	)
+
+	bcopts := common.NewBrowserContextOptions()
+	bcopts.Viewport = &common.Viewport{
+		Width:  500,
+		Height: 240,
+	}
+	bc, err := tb.NewContext(bcopts)
 	require.NoError(t, err)
+
 	p, err := bc.NewPage()
 	require.NoError(t, err)
 

tests/network_manager_test.go

@@ -112,21 +112,20 @@ func TestBasicAuth(t *testing.T) {
 		tb.Helper()
 
 		browser := newTestBrowser(t, withHTTPServer())
-		bc, err := browser.NewContext(
-			browser.toSobekValue(struct {
-				HttpCredentials *common.Credentials `js:"httpCredentials"` //nolint:revive
-			}{
-				HttpCredentials: &common.Credentials{
-					Username: user,
-					Password: pass,
-				},
-			}))
+
+		bcopts := common.NewBrowserContextOptions()
+		bcopts.HttpCredentials = &common.Credentials{
+			Username: validUser,
+			Password: validPassword,
+		}
+		bc, err := browser.NewContext(bcopts)
 		require.NoError(t, err)
+
 		p, err := bc.NewPage()
 		require.NoError(t, err)
 
 		url := browser.url(
-			fmt.Sprintf("/basic-auth/%s/%s", validUser, validPassword),
+			fmt.Sprintf("/basic-auth/%s/%s", user, pass),
 		)
 		opts := &common.FrameGotoOptions{
 			WaitUntil: common.LifecycleEventLoad,

tests/test_browser.go

@@ -253,7 +253,7 @@ func withSkipClose() func(*testBrowser) {
 
 // NewPage is a wrapper around Browser.NewPage that fails the test if an
 // error occurs. Added this helper to avoid boilerplate code in tests.
-func (b *testBrowser) NewPage(opts sobek.Value) *common.Page {
+func (b *testBrowser) NewPage(opts *common.BrowserContextOptions) *common.Page {
 	b.t.Helper()
 
 	p, err := b.Browser.NewPage(opts)