browser: fix frame document ordering

internal/js/modules/k6/browser/common/frame.go

@@ -27,6 +27,10 @@ type DocumentInfo struct {
 	request    *Request
 }
 
+func (d *DocumentInfo) is(id string) bool {
+	return d != nil && d.documentID == id
+}
+
 // DOMElementState represents a DOM element state.
 type DOMElementState int
 
@@ -110,9 +114,9 @@ type Frame struct {
 	inflightRequestsMu sync.RWMutex
 	inflightRequests   map[network.RequestID]bool
 
-	currentDocument   *DocumentInfo
-	pendingDocumentMu sync.RWMutex
-	pendingDocument   *DocumentInfo
+	currentDocument *DocumentInfo
+	pendingDocument *DocumentInfo
+	documentMu      sync.RWMutex
 
 	log *log.Logger
 }
@@ -313,6 +317,36 @@ func (f *Frame) hasLifecycleEventFired(event LifecycleEvent) bool {
 	return f.lifecycleEvents[event]
 }
 
+func (f *Frame) requestByDocumentID(id string) *Request {
+	if id == "" {
+		return nil
+	}
+
+	f.documentMu.RLock()
+	defer f.documentMu.RUnlock()
+
+	if f.pendingDocument.is(id) {
+		return f.pendingDocument.request
+	}
+	if f.currentDocument.is(id) {
+		return f.currentDocument.request
+	}
+
+	return nil
+}
+
+func (f *Frame) responseByDocumentID(id string) *Response {
+	request := f.requestByDocumentID(id)
+	if request == nil {
+		return nil
+	}
+
+	request.responseMu.RLock()
+	defer request.responseMu.RUnlock()
+
+	return request.response
+}
+
 func (f *Frame) navigated(name string, url string, loaderID string) {
 	f.log.Debugf("Frame:navigated", "fid:%s furl:%q lid:%s name:%q url:%q", f.ID(), f.URL(), loaderID, name, url)
 
@@ -2098,7 +2132,7 @@ func (f *Frame) WaitForNavigation(opts *FrameWaitForNavigationOptions, rm RegExM
 	}()
 
 	var (
-		resp       *Response
+		docID      string
 		sameDocNav bool
 	)
 	select {
@@ -2115,13 +2149,8 @@ func (f *Frame) WaitForNavigation(opts *FrameWaitForNavigationOptions, rm RegExM
 				sameDocNav = true
 				break
 			}
-			// request could be nil if navigating to e.g. BlankPage.
-			req := e.newDocument.request
-			if req != nil {
-				req.responseMu.RLock()
-				resp = req.response
-				req.responseMu.RUnlock()
-			}
+
+			docID = e.newDocument.documentID
 		}
 	case <-timeoutCtx.Done():
 		return nil, handleTimeoutError(ContextErr(timeoutCtx))
@@ -2138,6 +2167,11 @@ func (f *Frame) WaitForNavigation(opts *FrameWaitForNavigationOptions, rm RegExM
 		}
 	}
 
+	var resp *Response
+	if !sameDocNav {
+		resp = f.responseByDocumentID(docID)
+	}
+
 	// Since the response will be in an interface, it will never be nil,
 	// so we need to return nil explicitly.
 	if resp == nil {

internal/js/modules/k6/browser/common/frame_manager.go

@@ -102,14 +102,14 @@ func (m *FrameManager) frameAbortedNavigation(frameID cdp.FrameID, errorText, do
 		return
 	}
 
-	frame.pendingDocumentMu.Lock()
+	frame.documentMu.Lock()
 
 	if frame.pendingDocument == nil {
-		frame.pendingDocumentMu.Unlock()
+		frame.documentMu.Unlock()
 		return
 	}
 	if documentID != "" && frame.pendingDocument.documentID != documentID {
-		frame.pendingDocumentMu.Unlock()
+		frame.documentMu.Unlock()
 		return
 	}
 
@@ -125,7 +125,7 @@ func (m *FrameManager) frameAbortedNavigation(frameID cdp.FrameID, errorText, do
 	}
 	frame.pendingDocument = nil
 
-	frame.pendingDocumentMu.Unlock()
+	frame.documentMu.Unlock()
 
 	frame.emit(EventFrameNavigation, ne)
 }
@@ -298,8 +298,8 @@ func (m *FrameManager) frameNavigated(
 
 	frame.navigated(name, url, documentID)
 
-	frame.pendingDocumentMu.Lock()
-	defer frame.pendingDocumentMu.Unlock()
+	frame.documentMu.Lock()
+	defer frame.documentMu.Unlock()
 
 	var (
 		keepPending     *DocumentInfo
@@ -309,7 +309,7 @@ func (m *FrameManager) frameNavigated(
 		if pendingDocument.documentID == "" {
 			pendingDocument.documentID = documentID
 		}
-		if pendingDocument.documentID == documentID {
+		if pendingDocument.is(documentID) {
 			// Committing a pending document.
 			frame.currentDocument = pendingDocument
 		} else {
@@ -400,10 +400,10 @@ func (m *FrameManager) frameRequestedNavigation(frameID cdp.FrameID, url string,
 		b.AddFrameNavigation(frame)
 	}
 
-	frame.pendingDocumentMu.Lock()
-	defer frame.pendingDocumentMu.Unlock()
+	frame.documentMu.Lock()
+	defer frame.documentMu.Unlock()
 
-	if frame.pendingDocument != nil && frame.pendingDocument.documentID == documentID {
+	if frame.pendingDocument.is(documentID) {
 		m.logger.Debugf("FrameManager:frameRequestedNavigation:return",
 			"fmid:%d fid:%v furl:%s url:%s docid:%s pdocid:%s pdoc:dontSet",
 			m.ID(), frameID, frame.URL(), url, documentID,
@@ -478,10 +478,10 @@ func (m *FrameManager) requestFailed(req *Request, canceled bool) {
 	}
 	frame.deleteRequest(req.getID())
 
-	frame.pendingDocumentMu.RLock()
+	frame.documentMu.RLock()
 	if frame.pendingDocument == nil || frame.pendingDocument.request != req {
 		m.logger.Debugf("FrameManager:requestFailed:return", "fmid:%d pdoc:nil", m.ID())
-		frame.pendingDocumentMu.RUnlock()
+		frame.documentMu.RUnlock()
 		return
 	}
 
@@ -491,7 +491,7 @@ func (m *FrameManager) requestFailed(req *Request, canceled bool) {
 	}
 
 	docID := frame.pendingDocument.documentID
-	frame.pendingDocumentMu.RUnlock()
+	frame.documentMu.RUnlock()
 
 	m.frameAbortedNavigation(cdp.FrameID(frame.ID()), errorText, docID)
 }
@@ -531,9 +531,19 @@ func (m *FrameManager) requestStarted(req *Request) {
 
 	frame.addRequest(req.getID())
 	if req.documentID != "" {
-		frame.pendingDocumentMu.Lock()
-		frame.pendingDocument = &DocumentInfo{documentID: req.documentID, request: req}
-		frame.pendingDocumentMu.Unlock()
+		frame.documentMu.Lock()
+		switch {
+		case frame.currentDocument.is(req.documentID):
+			frame.currentDocument.request = req
+			if frame.pendingDocument.is(req.documentID) {
+				frame.pendingDocument = nil
+			}
+		case frame.pendingDocument.is(req.documentID):
+			frame.pendingDocument.request = req
+		default:
+			frame.pendingDocument = &DocumentInfo{documentID: req.documentID, request: req}
+		}
+		frame.documentMu.Unlock()
 	}
 
 	if !m.page.hasRoutes() {
@@ -720,20 +730,16 @@ func (m *FrameManager) NavigateFrame(frame *Frame, url string, parsedOpts *Frame
 		return err // TODO maybe wrap this as well?
 	}
 
-	var resp *Response
+	var docID string
 	select {
 	case evt := <-navEvtCh:
 		if e, ok := evt.(*NavigationEvent); ok {
 			if e.err != nil {
 				return nil, e.err
 			}
 
-			req := e.newDocument.request
-			// Request could be nil in case of navigation to e.g. BlankPage.
-			if req != nil {
-				req.responseMu.RLock()
-				resp = req.response
-				req.responseMu.RUnlock()
+			if e.newDocument != nil {
+				docID = e.newDocument.documentID
 			}
 		}
 	case <-timeoutCtx.Done():
@@ -746,7 +752,7 @@ func (m *FrameManager) NavigateFrame(frame *Frame, url string, parsedOpts *Frame
 		return nil, wrapTimeoutError(ContextErr(timeoutCtx))
 	}
 
-	return resp, nil
+	return frame.responseByDocumentID(docID), nil
 }
 
 // Page returns the page that this frame manager belongs to.

internal/js/modules/k6/browser/tests/webvital_test.go

@@ -2,7 +2,6 @@ package tests
 
 import (
 	"context"
-	"runtime"
 	"testing"
 	"time"
 
@@ -19,9 +18,7 @@ import (
 // a web page.
 func TestWebVitalMetric(t *testing.T) {
 	t.Parallel()
-	if runtime.GOOS == "windows" {
-		t.Skip("timeouts on windows")
-	}
+
 	var (
 		samples  = make(chan k6metrics.SampleContainer)
 		browser  = newTestBrowser(t, withFileServer(), withSamples(samples))
@@ -92,9 +89,7 @@ func TestWebVitalMetric(t *testing.T) {
 
 func TestWebVitalMetricNoInteraction(t *testing.T) {
 	t.Parallel()
-	if runtime.GOOS == "windows" {
-		t.Skip("timeouts on windows")
-	}
+
 	var (
 		samples  = make(chan k6metrics.SampleContainer)
 		browser  = newTestBrowser(t, withFileServer(), withSamples(samples))