diff --git a/common/browser.go b/common/browser.go index f1b7348ba..26a9456de 100644 --- a/common/browser.go +++ b/common/browser.go @@ -50,8 +50,7 @@ type Browser struct { // A *Connection is saved to this field, see: connect(). conn connection - contextsMu sync.RWMutex - contexts map[cdp.BrowserContextID]*BrowserContext + context *BrowserContext defaultContext *BrowserContext // Cancel function to stop event listening @@ -103,7 +102,6 @@ func newBrowser( state: int64(BrowserStateOpen), browserProc: browserProc, browserOpts: browserOpts, - contexts: make(map[cdp.BrowserContextID]*BrowserContext), pages: make(map[target.ID]*Page), sessionIDtoTargetID: make(map[target.SessionID]target.ID), vu: k6ext.GetVU(ctx), @@ -137,23 +135,19 @@ func (b *Browser) disposeContext(id cdp.BrowserContextID) error { return fmt.Errorf("disposing browser context ID %s: %w", id, err) } - b.contextsMu.Lock() - defer b.contextsMu.Unlock() - delete(b.contexts, id) + b.context = nil return nil } -// getDefaultBrowserContextOrByID returns the BrowserContext for the given page ID. +// getDefaultBrowserContextOrMatchedID returns the BrowserContext for the given browser context ID. // If the browser context is not found, the default BrowserContext is returned. -func (b *Browser) getDefaultBrowserContextOrByID(id cdp.BrowserContextID) *BrowserContext { - b.contextsMu.RLock() - defer b.contextsMu.RUnlock() - browserCtx := b.defaultContext - if bctx, ok := b.contexts[id]; ok { - browserCtx = bctx - } - return browserCtx +func (b *Browser) getDefaultBrowserContextOrMatchedID(id cdp.BrowserContextID) *BrowserContext { + if b.context == nil || b.context.id != id { + return b.defaultContext + } + + return b.context } func (b *Browser) getPages() []*Page { @@ -227,7 +221,7 @@ func (b *Browser) onAttachedToTarget(ev *target.EventAttachedToTarget) { var ( targetPage = ev.TargetInfo - browserCtx = b.getDefaultBrowserContextOrByID(targetPage.BrowserContextID) + browserCtx = b.getDefaultBrowserContextOrMatchedID(targetPage.BrowserContextID) ) if !b.isAttachedPageValid(ev, browserCtx) { @@ -377,11 +371,8 @@ func (b *Browser) onDetachedFromTarget(ev *target.EventDetachedFromTarget) { } func (b *Browser) newPageInContext(id cdp.BrowserContextID) (*Page, error) { - b.contextsMu.RLock() - browserCtx, ok := b.contexts[id] - b.contextsMu.RUnlock() - if !ok { - return nil, fmt.Errorf("missing browser context: %s", id) + if b.context == nil || b.context.id != id { + return nil, fmt.Errorf("missing browser context %s, current context is %s", id, b.context.id) } ctx, cancel := context.WithTimeout(b.ctx, b.browserOpts.Timeout) @@ -393,7 +384,7 @@ func (b *Browser) newPageInContext(id cdp.BrowserContextID) (*Page, error) { waitForPage, removeEventHandler := createWaitForEventHandler( ctx, - browserCtx, // browser context will emit the following event: + b.context, // browser context will emit the following event: []string{EventBrowserContextPage}, func(e any) bool { tid := <-targetID @@ -494,15 +485,11 @@ func (b *Browser) Close() { // Contexts returns list of browser contexts. func (b *Browser) Contexts() []api.BrowserContext { - b.contextsMu.RLock() - defer b.contextsMu.RUnlock() - - contexts := make([]api.BrowserContext, 0, len(b.contexts)) - for _, b := range b.contexts { - contexts = append(contexts, b) + if b.context == nil { + return []api.BrowserContext{} } - return contexts + return []api.BrowserContext{b.context} } // IsConnected returns whether the WebSocket connection to the browser process @@ -513,6 +500,10 @@ func (b *Browser) IsConnected() bool { // NewContext creates a new incognito-like browser context. func (b *Browser) NewContext(opts goja.Value) (api.BrowserContext, error) { + if b.context != nil { + return nil, errors.New("existing browser context must be closed before creating a new one") + } + action := target.CreateBrowserContext().WithDisposeOnDetach(true) browserContextID, err := action.Do(cdp.WithExecutor(b.ctx, b.conn)) b.logger.Debugf("Browser:NewContext", "bctxid:%v", browserContextID) @@ -525,13 +516,11 @@ func (b *Browser) NewContext(opts goja.Value) (api.BrowserContext, error) { k6ext.Panic(b.ctx, "parsing newContext options: %w", err) } - b.contextsMu.Lock() - defer b.contextsMu.Unlock() browserCtx, err := NewBrowserContext(b.ctx, b, browserContextID, browserCtxOpts, b.logger) if err != nil { return nil, fmt.Errorf("new context: %w", err) } - b.contexts[browserContextID] = browserCtx + b.context = browserCtx return browserCtx, nil } diff --git a/common/browser_test.go b/common/browser_test.go index c01539ade..9823fea03 100644 --- a/common/browser_test.go +++ b/common/browser_test.go @@ -31,11 +31,11 @@ func TestBrowserNewPageInContext(t *testing.T) { var err error vu := k6test.NewVU(t) ctx = k6ext.WithVU(ctx, vu) - b.contexts[id], err = NewBrowserContext(ctx, b, id, nil, nil) + b.context, err = NewBrowserContext(ctx, b, id, nil, nil) require.NoError(t, err) return &testCase{ b: b, - bc: b.contexts[id], + bc: b.context, } } diff --git a/tests/browser_test.go b/tests/browser_test.go index 9ec03f8ca..c88c707bc 100644 --- a/tests/browser_test.go +++ b/tests/browser_test.go @@ -22,22 +22,48 @@ import ( func TestBrowserNewPage(t *testing.T) { b := newTestBrowser(t) - p := b.NewPage(nil) + p1 := b.NewPage(nil) l := len(b.Contexts()) assert.Equal(t, 1, l, "expected there to be 1 browser context, but found %d", l) - p2 := b.NewPage(nil) - l = len(b.Contexts()) - assert.Equal(t, 2, l, "expected there to be 2 browser context, but found %d", l) + _, err := b.Browser.NewPage(nil) + assert.EqualError(t, err, "new page: existing browser context must be closed before creating a new one") - err := p.Close(nil) + err = p1.Close(nil) require.NoError(t, err) l = len(b.Contexts()) - assert.Equal(t, 2, l, "expected there to be 2 browser contexts after first page close, but found %d", l) - err = p2.Close(nil) - require.NoError(t, err) + assert.Equal(t, 1, l, "expected there to be 1 browser context, but found %d", l) + + _, err = b.Browser.NewPage(nil) + assert.EqualError(t, err, "new page: existing browser context must be closed before creating a new one") + + b.Contexts()[0].Close() + l = len(b.Contexts()) + assert.Equal(t, 0, l, "expected there to be 0 browser context, but found %d", l) + + _ = b.NewPage(nil) l = len(b.Contexts()) - assert.Equal(t, 2, l, "expected there to be 2 browser contexts after second page close, but found %d", l) + assert.Equal(t, 1, l, "expected there to be 1 browser context, but found %d", l) +} + +func TestBrowserNewContext(t *testing.T) { + b := newTestBrowser(t) + bc1, err := b.NewContext(nil) + assert.NoError(t, err) + l := len(b.Contexts()) + assert.Equal(t, 1, l, "expected there to be 1 browser context, but found %d", l) + + _, err = b.NewContext(nil) + assert.EqualError(t, err, "existing browser context must be closed before creating a new one") + + bc1.Close() + l = len(b.Contexts()) + assert.Equal(t, 0, l, "expected there to be 0 browser context, but found %d", l) + + _, err = b.NewContext(nil) + assert.NoError(t, err) + l = len(b.Contexts()) + assert.Equal(t, 1, l, "expected there to be 1 browser context, but found %d", l) } func TestTmpDirCleanup(t *testing.T) { diff --git a/tests/keyboard_test.go b/tests/keyboard_test.go index 00b1fbed4..c6082721f 100644 --- a/tests/keyboard_test.go +++ b/tests/keyboard_test.go @@ -12,9 +12,8 @@ import ( ) func TestKeyboardPress(t *testing.T) { - tb := newTestBrowser(t) - t.Run("all_keys", func(t *testing.T) { + tb := newTestBrowser(t) p := tb.NewPage(nil) kb := p.GetKeyboard() layout := keyboardlayout.GetKeyboardLayout("us") @@ -27,6 +26,7 @@ func TestKeyboardPress(t *testing.T) { }) t.Run("backspace", func(t *testing.T) { + tb := newTestBrowser(t) p := tb.NewPage(nil) kb := p.GetKeyboard() @@ -43,6 +43,7 @@ func TestKeyboardPress(t *testing.T) { }) t.Run("combo", func(t *testing.T) { + tb := newTestBrowser(t) p := tb.NewPage(nil) kb := p.GetKeyboard() @@ -68,6 +69,7 @@ func TestKeyboardPress(t *testing.T) { t.Run("meta", func(t *testing.T) { t.Skip("FIXME") // See https://github.com/grafana/xk6-browser/issues/424 + tb := newTestBrowser(t) p := tb.NewPage(nil) kb := p.GetKeyboard() @@ -92,6 +94,7 @@ func TestKeyboardPress(t *testing.T) { }) t.Run("type does not split on +", func(t *testing.T) { + tb := newTestBrowser(t) p := tb.NewPage(nil) kb := p.GetKeyboard() @@ -105,6 +108,7 @@ func TestKeyboardPress(t *testing.T) { }) t.Run("capitalization", func(t *testing.T) { + tb := newTestBrowser(t) p := tb.NewPage(nil) kb := p.GetKeyboard() @@ -130,6 +134,7 @@ func TestKeyboardPress(t *testing.T) { }) t.Run("type not affected by shift", func(t *testing.T) { + tb := newTestBrowser(t) p := tb.NewPage(nil) kb := p.GetKeyboard() @@ -146,6 +151,7 @@ func TestKeyboardPress(t *testing.T) { }) t.Run("newline", func(t *testing.T) { + tb := newTestBrowser(t) p := tb.NewPage(nil) kb := p.GetKeyboard() @@ -163,6 +169,7 @@ func TestKeyboardPress(t *testing.T) { // Replicates the test from https://playwright.dev/docs/api/class-keyboard t.Run("selection", func(t *testing.T) { + tb := newTestBrowser(t) p := tb.NewPage(nil) kb := p.GetKeyboard() diff --git a/tests/network_manager_test.go b/tests/network_manager_test.go index c43e9b0e3..04f575484 100644 --- a/tests/network_manager_test.go +++ b/tests/network_manager_test.go @@ -74,11 +74,10 @@ func TestBasicAuth(t *testing.T) { validPassword = "validpass" ) - browser := newTestBrowser(t, withHTTPServer()) - auth := func(tb testing.TB, user, pass string) api.Response { tb.Helper() + browser := newTestBrowser(t, withHTTPServer()) bc, err := browser.NewContext( browser.toGojaValue(struct { HttpCredentials *common.Credentials `js:"httpCredentials"` //nolint:revive