diff --git a/internal/js/modules/k6/browser/common/frame.go b/internal/js/modules/k6/browser/common/frame.go index 1bcc6e368..cec660cbb 100644 --- a/internal/js/modules/k6/browser/common/frame.go +++ b/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 { diff --git a/internal/js/modules/k6/browser/common/frame_manager.go b/internal/js/modules/k6/browser/common/frame_manager.go index 512aa1bcd..c47de3e7b 100644 --- a/internal/js/modules/k6/browser/common/frame_manager.go +++ b/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,7 +730,7 @@ 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 { @@ -728,12 +738,8 @@ func (m *FrameManager) NavigateFrame(frame *Frame, url string, parsedOpts *Frame 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. diff --git a/internal/js/modules/k6/browser/tests/webvital_test.go b/internal/js/modules/k6/browser/tests/webvital_test.go index dcfa03410..bd0e92d01 100644 --- a/internal/js/modules/k6/browser/tests/webvital_test.go +++ b/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))