core(driver): fix legacy runner hanging oopifs in some cases #14074
Conversation
connorjclark
changed the title
connorjclark
changed the title
| await Promise.all([ | ||
| // Events from subtargets will be stringified and sent back on `Target.receivedMessageFromTarget`. | ||
| // We want to receive information about network requests from iframes, so enable the Network domain. | ||
| this.sendCommandToSession('Network.enable', event.sessionId), |
There was a problem hiding this comment.
Why does this fix the issue? Does it guarantee that Network.enable finishes after the target is resumed?
There was a problem hiding this comment.
The protocol doesn't respond to the Network.enable event until the underlying target has been resumed.
There was a problem hiding this comment.
Does it guarantee that Network.enable finishes after the target is resumed?
We don't care when the response is acked... and the order of the commands is guaranteed to be processed in the
order we sent them, so we're good.
There was a problem hiding this comment.
The protocol doesn't respond to the Network.enable event until the underlying target has been resumed.
If that's the case, shouldn't it always have failed and not been flakey?
There was a problem hiding this comment.
I'm also confused on that point, I mention in the OP.
|
Update: I made a mistake when confirming this was an issue. I was accidentally testing things with this line commented out: Without that line, all frames are set to be auto-attached/paused on start. I'm not sure what the difference really is, because the So this isn't fixing a present issue, I guess, but seems good to do anyway. It may end up being necessary / helping with debugging #13861 |
|
why did I comment that out? because in devtools all of these frame navigated events have a parentId, so I was curious what this line was even for. Turns out, without it OOPIFs hang (in legacy runner CLI). |
connorjclark
changed the title
While investigating #13861, I noticed that the legacy runner takes a long time on the oopif-scripts.html smoke fixture. This is because the OOPIF target hangs forever, and is never loaded. This was made clear when the second iframe on the test page never rendered any content.
Turns out, we can't do
Network.enableand await its response if the target is paused.It seems like the
iframespecific part of_handleTargetAttachedmay have always been busted? It's also possible that Chrome CDP internals changed underneath us.I also added a clarifying comment in
_handleTargetAttachednoting that all that logic is only for OOPIFs.