Separate Sobek and k6 logic from the module logic

[inancgumus]

This can also mitigate some of the known bugs caused by the concurrent Sobek usage throughout the extension 🤞

Runtime is most needed because of Sobek conversions. If we can provide a translation layer between Sobek and the extension code, we can greatly simplify it. For example:

func (b *BrowserContext) WaitForEvent(event string, optsOrPredicate sobek.Value) interface{} {
	rt := k6common.GetRuntime(b.ctx)

	if optsOrPredicate != nil && !sobek.IsUndefined(optsOrPredicate) && !sobek.IsNull(optsOrPredicate) {
		switch optsOrPredicate.ExportType() {
		case reflect.TypeOf(sobek.Object{}):
			opts := optsOrPredicate.ToObject(rt)
			for _, k := range opts.Keys() {
				switch k {
				case "predicate":
					predicateFn, isCallable = sobek.AssertFunction(opts.Get(k))
					if !isCallable {
						k6Throw(b.ctx, "expected callable predicate")
					}
				case "timeout":
					timeout = time.Duration(opts.Get(k).ToInteger()) * time.Millisecond
				}
			}

This kind of cluttering happens because of internally exposing Sobek, whereas we could do:

// Extension mechanics package (actual logic here)

package browser

func (c *Context) WaitForEvent(event string, predicate func()) (Event, error) {
	...
	return predicate(c.page)
}

// Translation layer (Sobek -> extension) — It's now named as the mapping layer.

type BrowserContext struct {
	...
	bc browser.Context
	...
}

func (b *BrowserContext) WaitForEvent(event string, optsOrPredicate sobek.Value) interface{} {
	// sobek stuff here
	r, err := b.bc.WaitForEvent(event predicate)
	if err != nil {
		k6Throw(...)
	}   
}

---

                      Test script
                           │
                           │
┌──────────────────────────▼────────────────────────────┐
│            Exposed methods to the test script         │
│          Extension translation layer: JS -> Sobek     │
└──────────────────────────┬────────────────────────────┘
                           │
┌──────────────────────────┴────────────────────────────┐
│                          ▼                            │
│                                                       │
│               Extension logic (Sobek-free)            │
│                                                       │
│                          │                            │
└──────────────────────────┼────────────────────────────┘
                           │
                           │
             ┌─────────────┼─────────────┐
             │             ▼             │
             │                           │
             │          Browser          │
             │                           │
             │                           │
             └───────────────────────────┘

See #268 (comment).

[imiric]

This is also important to avoid data races from sharing goja.Runtime. See #254 (comment) and #263 (review). The quickest relief would be to remove it from ElementHandle.eval().

[inancgumus]

quickest relief would be to remove it from ElementHandle.eval()

What's on your mind doing so? :)

[imiric]

Well, we currently pass goja.Values to this function, and to ExecutionContext.eval(), and do the conversion inside it:

xk6-browser/common/execution_context.go

Line 180 in 3ad187c

expression = pageFunc.ToString().String()

xk6-browser/common/helpers.go

Line 55 in 3ad187c

func convertArgument(ctx context.Context, execCtx *ExecutionContext, arg goja.Value) (*cdpruntime.CallArgument, error) {

Instead, this should happen much higher up in the stack, as close as possible or inside the methods we already expose to JS, via this JS->Go translation layer. This way we should(?) ensure a single goroutine has access to the goja.Runtime, and then when used internally we can worry about our own data races.

[inancgumus]

+1 That is exactly what I had in my mind: Goja should not enter into the extension logic realm.