Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initiate internal goroutines after a component tree is ready #1247

Open
3 tasks
Tracked by #1138
inancgumus opened this issue Mar 18, 2024 · 1 comment
Open
3 tasks
Tracked by #1138

Initiate internal goroutines after a component tree is ready #1247

inancgumus opened this issue Mar 18, 2024 · 1 comment
Labels
bug Something isn't working evaluate events CDP or internal events related. internal internal improvements and features

Comments

@inancgumus
Copy link
Member

inancgumus commented Mar 18, 2024

What?

The browser module might have race conditions because it sets the internal components to receive events before their parent fully completes their initialization. In that case, the event system listens to incoming CDP events and starts reacting to those events with internal components that haven't completed the initialization step.

For example:

Here, NewFrameSession calls NewNetworkManager, which calls NetworkManager.initEvents:

func (m *NetworkManager) initEvents() {
	chHandler := make(chan Event)
	m.session.on(m.ctx, []string{
		cdproto.EventNetworkLoadingFailed,
		cdproto.EventNetworkLoadingFinished,
		cdproto.EventNetworkRequestWillBeSent,
		cdproto.EventNetworkRequestServedFromCache,
		cdproto.EventNetworkResponseReceived,
		cdproto.EventFetchRequestPaused,
		cdproto.EventFetchAuthRequired,
	}, chHandler)

	go func() {
		for m.handleEvents(chHandler) {
		}
	}()
}

However, NewNetworkManager returns before the FrameSession initialization has finished. This might answer the question here. Before the inits are completed, we might start to receive events.

So how does a page end up with a nil mainFrame, or how does the mainFrame end up with a nil networkManager?

Why?

To prevent race conditions.

How?

A better approach might be to start event loops (internal goroutines (e.g., NetworkManager.handleEvents)) once we initialize all the components in the same tree (parent-child relationship).

  • Wait for the children to complete their initializations.
  • Initialize the children's internal goroutines from the parent once all the children are initialized.

Tasks

Tasks

Related PR(s)/Issue(s)

No response

@inancgumus inancgumus added the bug Something isn't working label Mar 18, 2024
@inancgumus inancgumus added evaluate events CDP or internal events related. internal internal improvements and features labels Mar 18, 2024
@ankur22
Copy link
Collaborator

ankur22 commented May 15, 2024

This comment links to an issue which is related to the refactoring of internals to better schedule them to avoid race conditions and other issues.

While investigating #827, I eventually found that the issue was related to the ordering of the CDP requests we make when a new tab is opened after clicking on a link on the original page.

The following are the CDP requests that are made when a new page is created:

-> {"id":5,"method":"Target.createTarget","params":{"url":"about:blank","browserContextId":"F673CD0CD1376255A44A54650C2B2226"}}
-> {"id":6,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Target.setAutoAttach","params":{"autoAttach":true,"waitForDebuggerOnStart":true,"flatten":true}}
-> {"id":7,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Browser.getWindowForTarget","params":{"targetId":"A8D91F407737AEA6DB82462AAE0FD169"}}
-> {"id":8,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Page.enable"}
-> {"id":9,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Page.getFrameTree"}
-> {"id":10,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Runtime.evaluate","params":{"expression":"window.k6SpanId = '0000000000000000';","awaitPromise":true}}
-> {"id":11,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Log.enable"}
-> {"id":12,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Runtime.enable"}
-> {"id":13,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Target.setAutoAttach","params":{"autoAttach":true,"waitForDebuggerOnStart":true,"flatten":true}}
-> {"id":14,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Page.setLifecycleEventsEnabled","params":{"enabled":true}}
-> {"id":15,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Page.createIsolatedWorld","params":{"frameId":"A8D91F407737AEA6DB82462AAE0FD169","worldName":"__k6_browser_utility_world__","grantUniveralAccess":true}}
-> {"id":16,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Page.addScriptToEvaluateOnNewDocument","params":{"source":"//# sourceURL=__xk6_browser_evaluation_script__","worldName":"__k6_browser_utility_world__"}}
-> {"id":17,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Network.enable","params":{}}
-> {"id":18,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Emulation.setDeviceMetricsOverride","params":{"width":1280,"height":720,"deviceScaleFactor":1,"mobile":false,"screenWidth":1280,"screenHeight":720,"screenOrientation":{"type":"landscapePrimary","angle":90}}}
-> {"id":19,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Browser.setWindowBounds","params":{"windowId":1994774730,"bounds":{"width":1280,"height":799}}}
-> {"id":20,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Emulation.setLocaleOverride","params":{"locale":"en-US"}}
-> {"id":21,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Emulation.setEmulatedMedia","params":{"media":"screen","features":[{"name":"prefers-color-scheme","value":"light"},{"name":"prefers-reduced-motion","value":""}]}}
-> {"id":22,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Emulation.setFocusEmulationEnabled","params":{"enabled":true}}
-> {"id":23,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Emulation.setUserAgentOverride","params":{"userAgent":"","acceptLanguage":"en-US"}}
-> {"id":24,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Runtime.runIfWaitingForDebugger"}

They are ran sequentially one after the other. When a new page is opened after clicking a link (with _blank as the target) k6 browser will attempt to run these requests. Unfortunately some of these CDP requests block indefinitely (such as Network.enable) until Runtime.runIfWaitingForDebugger is called. So there are two issues with our current approach:

  1. We're calling Runtime.runIfWaitingForDebugger too late.
  2. We're performing this sequentially.

Playwright on the other hand run these CDP requests concurrently, which queues the CDP requests up in Chrome which it will handle once it receives the Runtime.runIfWaitingForDebugger.

It might be a good idea to refactor how we create a new page so that we work with errgroup to perform all the necessary CDP requests to mimic PW's create page behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working evaluate events CDP or internal events related. internal internal improvements and features
Projects
None yet
Development

No branches or pull requests

2 participants