Allow non-incognito window in connect() mode

[tmc]

Feature Description

It appears that when using connect(), the page opened is always in incognito mode - I’m not using the explicit context api.

having non-incognito windows can help in development to be able to skip earlier potentially costly steps (such as running through an authentication flow).

Suggested Solution (optional)

No response

Already existing or connected issues / PRs (optional)

No response

[tmc]

I think what's happening here is that NewPage is always creating a new context -> I think NewPage on Browser should not implicitly create a context.

I want this to stay logged into a service as I'm iterating on tests as a means to speed up development.

[ankur22]

I tried an experiment where newPage on browser worked with the defaultContext instead of creating a new one. This allowed the browser module to work with a non incognito browser session.

Diff of experiment

diff --git a/common/browser.go b/common/browser.go
index e8680615..46f77f9a 100644
--- a/common/browser.go
+++ b/common/browser.go
@@ -197,13 +197,13 @@ func (b *Browser) initEvents() error {
                                return
                        case event := <-chHandler:
                                if ev, ok := event.data.(*target.EventAttachedToTarget); ok {
-                                       b.logger.Debugf("Browser:initEvents:onAttachedToTarget", "sid:%v tid:%v", ev.SessionID, ev.TargetInfo.TargetID)
+                                       b.logger.Infof("Browser:initEvents:onAttachedToTarget", "sid:%v tid:%v", ev.SessionID, ev.TargetInfo.TargetID)
                                        b.onAttachedToTarget(ev)
                                } else if ev, ok := event.data.(*target.EventDetachedFromTarget); ok {
-                                       b.logger.Debugf("Browser:initEvents:onDetachedFromTarget", "sid:%v", ev.SessionID)
+                                       b.logger.Infof("Browser:initEvents:onDetachedFromTarget", "sid:%v", ev.SessionID)
                                        b.onDetachedFromTarget(ev)
                                } else if event.typ == EventConnectionClose {
-                                       b.logger.Debugf("Browser:initEvents:EventConnectionClose", "")
+                                       b.logger.Infof("Browser:initEvents:EventConnectionClose", "")
                                        return
                                }
                        }
@@ -238,9 +238,14 @@ func (b *Browser) connectionOnAttachedToTarget(eva *target.EventAttachedToTarget
        // we're waiting for it. So, we do the lock management in a function with its
        // own defer.
        isAllowedBrowserContext := func() bool {
-               b.contextMu.RLock()
-               defer b.contextMu.RUnlock()
-               return b.context == nil || b.context.id == eva.TargetInfo.BrowserContextID
+               // b.contextMu.RLock()
+               // defer b.contextMu.RUnlock()
+               // return b.context == nil || b.context.id == eva.TargetInfo.BrowserContextID
+
+               // The page that is attached has a valid browserContextID, whereas the
+               // defaultContext has no id when it was created. How can we match new targets
+               // which are destined for the defaultContext, but ignore all other targets?
+               return true
        }
 
        return isAllowedBrowserContext()
@@ -333,14 +338,18 @@ func (b *Browser) isAttachedPageValid(ev *target.EventAttachedToTarget, browserC
                        ev.SessionID, targetPage.TargetID, targetPage.BrowserContextID, browserCtx == nil, targetPage.Type)
                return false
        }
-       // If the target is not in the same browser context as the current one, ignore it.
-       if browserCtx.id != targetPage.BrowserContextID {
-               b.logger.Debugf(
-                       "Browser:isAttachedPageValid", "incorrect browser context sid:%v tid:%v bctxid:%v target bctxid:%v",
-                       ev.SessionID, targetPage.TargetID, targetPage.BrowserContextID, browserCtx.id,
-               )
-               return false
-       }
+       // Again, since the defaultContext has an empty ID, this check will not work.
+       // How can we match new targets which are destined for the defaultContext,
+       // but ignore all other targets?
+
+       // // If the target is not in the same browser context as the current one, ignore it.
+       // if browserCtx.id != targetPage.BrowserContextID {
+       //      b.logger.Debugf(
+       //              "Browser:isAttachedPageValid", "incorrect browser context sid:%v tid:%v bctxid:%v target bctxid:%v",
+       //              ev.SessionID, targetPage.TargetID, targetPage.BrowserContextID, browserCtx.id,
+       //      )
+       //      return false
+       // }
 
        return true
 }
@@ -582,7 +591,7 @@ func (b *Browser) NewPage(opts goja.Value) (*Page, error) {
        _, span := TraceAPICall(b.ctx, "", "browser.newPage")
        defer span.End()
 
-       browserCtx, err := b.NewContext(opts)
+       browserCtx, err := b.useDefaultContext(opts)
        if err != nil {
                return nil, fmt.Errorf("new page: %w", err)
        }
@@ -590,6 +599,15 @@ func (b *Browser) NewPage(opts goja.Value) (*Page, error) {
        return browserCtx.NewPage()
 }
 
+// NewContext creates a new incognito-like browser context.
+func (b *Browser) useDefaultContext(opts goja.Value) (*BrowserContext, error) {
+       b.contextMu.Lock()
+       defer b.contextMu.Unlock()
+       b.context = b.defaultContext
+
+       return b.defaultContext, nil
+}
+
 // On returns a Promise that is resolved when the browser process is disconnected.
 // The only accepted event value is "disconnected".
 func (b *Browser) On(event string) (bool, error) {
diff --git a/examples/screenshot.js b/examples/screenshot.js
index fb8b5940..9d155898 100644
--- a/examples/screenshot.js
+++ b/examples/screenshot.js
@@ -1,4 +1,5 @@
 import { browser } from 'k6/x/browser';
+import { sleep } from 'k6';
 
 export const options = {
   scenarios: {
@@ -17,8 +18,7 @@ export const options = {
 }
 
 export default async function() {
-  const context = browser.newContext();
-  const page = context.newPage();
+  const page = browser.newPage();
   
   try {
     await page.goto('https://test.k6.io/');
@@ -26,6 +26,7 @@ export default async function() {
     // TODO: Assert this somehow. Upload as CI artifact or just an external `ls`?
     // Maybe even do a fuzzy image comparison against a preset known good screenshot?
   } finally {
+    sleep(5);
     page.close();
   }
 }

It showed some promising results, but the codebase is setup so that a new page/target can only be attached to a browserContext that was explicitly created and not a defaultContext.

The main question that has been left unanswered from the experiment is "How can we match new targets which are destined for the defaultContext, but ignore all other targets?". It is very important for us to ignore targets that have no associated browserContext in the current iteration since it considerably reduces the compute resources that the k6 browser module needs to run tests.