Fix do not unnecessarily set mainframe

Also:
Add some sanity checks and protect the mainframe
field behind a mutex in places that forgot to
use it.

Author: inancgumus

chromium/browser_type.go

@@ -174,6 +174,7 @@ func makeLogger(ctx context.Context, launchOpts *common.LaunchOptions) (*common.
 		reCategoryFilter, _ = regexp.Compile(launchOpts.LogCategoryFilter)
 		logger              = common.NewLogger(ctx, k6Logger, launchOpts.Debug, reCategoryFilter)
 	)
+
 	// set the log level from the launch options (usually from a script's options).
 	if launchOpts.Debug {
 		_ = logger.SetLevel("debug")

common/frame.go

@@ -166,7 +166,7 @@ func (f *Frame) clearLifecycle() {
 	}
 	f.lifecycleEventsMu.Unlock()
 
-	f.page.frameManager.mainFrame.recalculateLifecycle()
+	f.page.frameManager.MainFrame().recalculateLifecycle()
 
 	// keep the request related to the document if present
 	// in f.inflightRequests
@@ -419,7 +419,7 @@ func (f *Frame) position() *Position {
 	if frame == nil {
 		return nil
 	}
-	if frame == f.page.frameManager.mainFrame {
+	if frame == f.page.frameManager.MainFrame() {
 		return &Position{X: 0, Y: 0}
 	}
 	element := frame.FrameElement()
@@ -1107,6 +1107,10 @@ func (f *Frame) IsVisible(selector string, opts goja.Value) bool {
 
 // ID returns the frame id
 func (f *Frame) ID() string {
+	if f == nil {
+		return ""
+	}
+
 	f.propertiesMu.RLock()
 	defer f.propertiesMu.RUnlock()
 
@@ -1115,6 +1119,10 @@ func (f *Frame) ID() string {
 
 // LoaderID returns the ID of the frame that loaded this frame
 func (f *Frame) LoaderID() string {
+	if f == nil {
+		return ""
+	}
+
 	f.propertiesMu.RLock()
 	defer f.propertiesMu.RUnlock()
 
@@ -1123,6 +1131,10 @@ func (f *Frame) LoaderID() string {
 
 // Name returns the frame name
 func (f *Frame) Name() string {
+	if f == nil {
+		return ""
+	}
+
 	f.propertiesMu.RLock()
 	defer f.propertiesMu.RUnlock()
 
@@ -1358,6 +1370,10 @@ func (f *Frame) Uncheck(selector string, opts goja.Value) {
 
 // URL returns the frame URL.
 func (f *Frame) URL() string {
+	if f == nil {
+		return ""
+	}
+
 	f.propertiesMu.RLock()
 	defer f.propertiesMu.RUnlock()
 

common/frame_manager.go

@@ -203,7 +203,7 @@ func (m *FrameManager) frameLifecycleEvent(frameID cdp.FrameID, event LifecycleE
 	frame := m.getFrameByID(frameID)
 	if frame != nil {
 		frame.onLifecycleEvent(event)
-		m.mainFrame.recalculateLifecycle() // Recalculate life cycle state from the top
+		m.MainFrame().recalculateLifecycle() // Recalculate life cycle state from the top
 	}
 }
 
@@ -253,23 +253,26 @@ func (m *FrameManager) frameNavigated(frameID cdp.FrameID, parentFrameID cdp.Fra
 		}
 	}
 
-	if isMainFrame {
-		if frame != nil {
-			m.logger.Debugf("FrameManager:frameNavigated:MainFrame:delete",
-				"fmid:%d fid:%v pfid:%v docid:%s fname:%s furl:%s initial:%t oldfid:%v",
-				m.ID(), frameID, parentFrameID, documentID, name, url, initial, frame.ID())
+	var mainFrame *Frame
+	if isMainFrame && frame == nil {
+		m.logger.Debugf("FrameManager:frameNavigated:MainFrame:initialMainFrameNavigation",
+			"fmid:%d fid:%v pfid:%v docid:%s fname:%s furl:%s initial:%t",
+			m.ID(), frameID, parentFrameID, documentID, name, url, initial)
 
-			// Update frame ID to retain frame identity on cross-process navigation.
-			delete(m.frames, cdp.FrameID(frame.ID()))
-			frame.setID(frameID)
-		} else {
-			m.logger.Debugf("FrameManager:frameNavigated:MainFrame:initialMainFrameNavigation",
-				"fmid:%d fid:%v pfid:%v docid:%s fname:%s furl:%s initial:%t",
-				m.ID(), frameID, parentFrameID, documentID, name, url, initial)
+		// Initial main frame navigation.
+		frame = NewFrame(m.ctx, m, nil, frameID, m.logger)
+		mainFrame = frame
+	} else if isMainFrame && frame.ID() != string(frameID) {
+		m.logger.Debugf("FrameManager:frameNavigated:MainFrame:delete",
+			"fmid:%d fid:%v pfid:%v docid:%s fname:%s furl:%s initial:%t oldfid:%v",
+			m.ID(), frameID, parentFrameID, documentID, name, url, initial, frame.ID())
 
-			// Initial main frame navigation.
-			frame = NewFrame(m.ctx, m, nil, frameID, m.logger)
-		}
+		// Update frame ID to retain frame identity on cross-process navigation.
+		delete(m.frames, cdp.FrameID(frame.ID()))
+		frame.setID(frameID)
+		mainFrame = frame
+	}
+	if mainFrame != nil {
 		m.frames[frameID] = frame
 		m.setMainFrame(frame)
 	}
@@ -548,6 +551,11 @@ func (m *FrameManager) MainFrame() *Frame {
 func (m *FrameManager) setMainFrame(f *Frame) {
 	m.mainFrameMu.Lock()
 	defer m.mainFrameMu.Unlock()
+
+	m.logger.Debugf("FrameManager:setMainFrame",
+		"fmid:%d fid:%v furl:%s",
+		m.ID(), f.ID(), f.URL())
+
 	m.mainFrame = f
 }
 

common/page.go

@@ -693,7 +693,7 @@ func (p *Page) Reload(opts goja.Value) api.Response {
 		k6common.Throw(rt, fmt.Errorf("failed parsing options: %w", err))
 	}
 
-	ch, evCancelFn := createWaitForEventHandler(p.ctx, p.frameManager.mainFrame, []string{EventFrameNavigation}, func(data interface{}) bool {
+	ch, evCancelFn := createWaitForEventHandler(p.ctx, p.frameManager.MainFrame(), []string{EventFrameNavigation}, func(data interface{}) bool {
 		return true // Both successful and failed navigations are considered
 	})
 	defer evCancelFn() // Remove event handler
@@ -713,7 +713,7 @@ func (p *Page) Reload(opts goja.Value) api.Response {
 	}
 
 	if p.frameManager.mainFrame.hasSubtreeLifecycleEventFired(parsedOpts.WaitUntil) {
-		waitForEvent(p.ctx, p.frameManager.mainFrame, []string{EventFrameAddLifecycle}, func(data interface{}) bool {
+		_, _ = waitForEvent(p.ctx, p.frameManager.MainFrame(), []string{EventFrameAddLifecycle}, func(data interface{}) bool {
 			return data.(LifecycleEvent) == parsedOpts.WaitUntil
 		}, parsedOpts.Timeout)
 	}

common/request.go

@@ -264,5 +264,8 @@ func (r *Request) Timing() goja.Value {
 
 // URL returns the request URL
 func (r *Request) URL() string {
+	if r == nil {
+		return ""
+	}
 	return r.url.String()
 }

common/screenshotter.go

@@ -51,7 +51,7 @@ func (s *screenshotter) fullPageSize(p *Page) (*Size, error) {
 		forceCallable: true,
 		returnByValue: true,
 	}
-	result, err := p.frameManager.mainFrame.evaluate(s.ctx, mainWorld, opts, rt.ToValue(`
+	result, err := p.frameManager.MainFrame().evaluate(s.ctx, mainWorld, opts, rt.ToValue(`
         () => {
             if (!document.body || !document.documentElement) {
                 return null;
@@ -95,7 +95,7 @@ func (s *screenshotter) originalViewportSize(p *Page) (*Size, *Size, error) {
 		forceCallable: true,
 		returnByValue: true,
 	}
-	result, err := p.frameManager.mainFrame.evaluate(s.ctx, mainWorld, opts, rt.ToValue(`
+	result, err := p.frameManager.MainFrame().evaluate(s.ctx, mainWorld, opts, rt.ToValue(`
 	() => (
 		{ width: window.innerWidth, height: window.innerHeight }
 	)`))