Merge pull request #149 from grafana/fix/53-interface-conversion-error

Fix interface conversion in document

Author: inancgumus

common/execution_context.go

@@ -217,3 +217,8 @@ func (e *ExecutionContext) EvaluateHandle(apiCtx context.Context, pageFunc goja.
 func (e *ExecutionContext) Frame() *Frame {
 	return e.frame
 }
+
+// ID returns the CDP runtime ID of this execution context.
+func (e *ExecutionContext) ID() runtime.ExecutionContextID {
+	return e.id
+}

common/frame.go

@@ -117,8 +117,8 @@ type Frame struct {
 
 	documentHandle *ElementHandle
 
-	mainExecutionContext             *ExecutionContext
-	utilityExecutionContext          *ExecutionContext
+	mainExecutionContext             frameExecutionContext
+	utilityExecutionContext          frameExecutionContext
 	mainExecutionContextCh           chan bool
 	utilityExecutionContextCh        chan bool
 	mainExecutionContextHasWaited    int32
@@ -311,11 +311,17 @@ func (f *Frame) document() (*ElementHandle, error) {
 	if f.documentHandle != nil {
 		return f.documentHandle, nil
 	}
+
 	f.waitForExecutionContext("main")
+
 	result, err := f.mainExecutionContext.evaluate(f.ctx, false, false, rt.ToValue("document"), nil)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("frame document: cannot evaluate in main execution context: %w", err)
+	}
+	if result == nil {
+		return nil, errors.New("frame document: evaluate result is nil in main execution context")
 	}
+
 	f.documentHandle = result.(*ElementHandle)
 	return f.documentHandle, err
 }
@@ -350,10 +356,10 @@ func (f *Frame) navigated(name string, url string, loaderID string) {
 }
 
 func (f *Frame) nullContext(execCtxID runtime.ExecutionContextID) {
-	if f.mainExecutionContext != nil && f.mainExecutionContext.id == execCtxID {
+	if f.mainExecutionContext != nil && f.mainExecutionContext.ID() == execCtxID {
 		f.mainExecutionContext = nil
 		f.documentHandle = nil
-	} else if f.utilityExecutionContext != nil && f.utilityExecutionContext.id == execCtxID {
+	} else if f.utilityExecutionContext != nil && f.utilityExecutionContext.ID() == execCtxID {
 		f.utilityExecutionContext = nil
 	}
 }
@@ -412,7 +418,7 @@ func (f *Frame) requestByID(reqID network.RequestID) *Request {
 	return frameSession.networkManager.requestFromID(reqID)
 }
 
-func (f *Frame) setContext(world string, execCtx *ExecutionContext) {
+func (f *Frame) setContext(world string, execCtx frameExecutionContext) {
 	if world == "main" {
 		f.mainExecutionContext = execCtx
 		if len(f.mainExecutionContextCh) == 0 {
@@ -507,13 +513,13 @@ func (f *Frame) waitForSelector(selector string, opts *FrameWaitForSelectorOptio
 
 func (f *Frame) AddScriptTag(opts goja.Value) {
 	rt := k6common.GetRuntime(f.ctx)
-	k6common.Throw(rt, errors.New("Frame.AddScriptTag() has not been implemented yet!"))
+	k6common.Throw(rt, errors.New("Frame.AddScriptTag() has not been implemented yet"))
 	applySlowMo(f.ctx)
 }
 
 func (f *Frame) AddStyleTag(opts goja.Value) {
 	rt := k6common.GetRuntime(f.ctx)
-	k6common.Throw(rt, errors.New("Frame.AddStyleTag() has not been implemented yet!"))
+	k6common.Throw(rt, errors.New("Frame.AddStyleTag() has not been implemented yet"))
 	applySlowMo(f.ctx)
 }
 
@@ -1244,3 +1250,46 @@ func (f *Frame) WaitForTimeout(timeout int64) {
 	case <-time.After(time.Duration(timeout) * time.Millisecond):
 	}
 }
+
+// frameExecutionContext represents a JS execution context that belongs to Frame.
+type frameExecutionContext interface {
+	// adoptBackendNodeId adopts specified backend node into this execution
+	// context from another execution context.
+	adoptBackendNodeId(backendNodeID cdp.BackendNodeID) (*ElementHandle, error)
+
+	// adoptElementHandle adopts the specified element handle into this
+	// execution context from another execution context.
+	adoptElementHandle(elementHandle *ElementHandle) (*ElementHandle, error)
+
+	// evaluate will evaluate provided callable within this execution
+	// context and return by value or handle.
+	evaluate(
+		apiCtx context.Context,
+		forceCallable bool, returnByValue bool,
+		pageFunc goja.Value, args ...goja.Value,
+	) (res interface{}, err error)
+
+	// getInjectedScript returns a JS handle to the injected script of helper
+	// functions.
+	getInjectedScript(apiCtx context.Context) (api.JSHandle, error)
+
+	// Evaluate will evaluate provided page function within this execution
+	// context.
+	Evaluate(
+		apiCtx context.Context,
+		pageFunc goja.Value, args ...goja.Value,
+	) (interface{}, error)
+
+	// EvaluateHandle will evaluate provided page function within this
+	// execution context.
+	EvaluateHandle(
+		apiCtx context.Context,
+		pageFunc goja.Value, args ...goja.Value,
+	) (api.JSHandle, error)
+
+	// Frame returns the frame that this execution context belongs to.
+	Frame() *Frame
+
+	// id returns the CDP runtime ID of this execution context.
+	ID() runtime.ExecutionContextID
+}

common/frame_test.go

@@ -0,0 +1,88 @@
+/*
+ *
+ * xk6-browser - a browser automation extension for k6
+ * Copyright (C) 2021 Load Impact
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package common
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/chromedp/cdproto/cdp"
+	"github.com/dop251/goja"
+	"github.com/stretchr/testify/require"
+)
+
+// Test calling Frame.document does not panic with a nil document.
+// See: issue #53 for details.
+func TestFrameNilDocument(t *testing.T) {
+	t.Parallel()
+
+	ctx := context.Background()
+	fm := NewFrameManager(ctx, nil, nil, nil, nil)
+	frame := NewFrame(ctx, fm, nil, cdp.FrameID("42"))
+
+	// frame should not panic with a nil document
+	stub := &executionContextTestStub{
+		evaluateFn: func(apiCtx context.Context, forceCallable bool, returnByValue bool, pageFunc goja.Value, args ...goja.Value) (res interface{}, err error) {
+			// return nil to test for panic
+			return nil, nil
+		},
+	}
+
+	// document() waits for the main execution context
+	ok := make(chan struct{}, 1)
+	go func() {
+		frame.setContext("main", stub)
+		ok <- struct{}{}
+	}()
+	select {
+	case <-ok:
+	case <-time.After(time.Second):
+		require.FailNow(t, "cannot set the main execution context, frame.setContext timed out")
+	}
+
+	require.NotPanics(t, func() {
+		_, err := frame.document()
+		require.Error(t, err)
+	})
+
+	// frame gets the document from the evaluate call
+	want := &ElementHandle{}
+	stub.evaluateFn = func(apiCtx context.Context, forceCallable bool, returnByValue bool, pageFunc goja.Value, args ...goja.Value) (res interface{}, err error) {
+		return want, nil
+	}
+	got, err := frame.document()
+	require.NoError(t, err)
+	require.Equal(t, want, got)
+
+	// frame sets documentHandle in the document method
+	got = frame.documentHandle
+	require.Equal(t, want, got)
+}
+
+type executionContextTestStub struct {
+	ExecutionContext
+	evaluateFn func(apiCtx context.Context, forceCallable bool, returnByValue bool, pageFunc goja.Value, args ...goja.Value) (res interface{}, err error)
+}
+
+func (e executionContextTestStub) evaluate(apiCtx context.Context, forceCallable bool, returnByValue bool, pageFunc goja.Value, args ...goja.Value) (res interface{}, err error) {
+	return e.evaluateFn(apiCtx, forceCallable, returnByValue, pageFunc, args...)
+}