core(driver): attach to worker targets #14212
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are two disparate changes, can you split this off? |
|
👀 |
|
Split out #14217 |
connorjclark
approved these changes
Jul 18, 2022
This comment was marked as resolved.
This comment was marked as resolved.
adamraine
changed the title
adamraine
commented
Feb 15, 2023
| const target = await newSession.sendCommand('Target.getTargetInfo').catch(() => null); | ||
| const targetType = target?.targetInfo?.type; | ||
| const hasValidTargetType = targetType === 'page' || targetType === 'iframe'; | ||
| const {targetInfo} = await newSession.sendCommand('Target.getTargetInfo'); |
There was a problem hiding this comment.
Before M110 this command would throw an error on worker targets. With this patch we would require a Chrome version >=M110 which is the current stable.
copybara-service bot
pushed a commit
to ChromeDevTools/devtools-frontend
that referenced
this pull request
Mar 14, 2023
Resuming a worker that is not waiting on the debugger will resume the worker even if there is another worker session still waiting on the debugger. This CL is a workaround to prevent DevTools from prematurely resuming worker targets. This was causing issues for Lighthouse listening to network requests: GoogleChrome/lighthouse#14212 Bug: 1423096 Change-Id: I757f4fd3ee0969ea33e1acc95e1190c99146a023 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4326967 Commit-Queue: Adam Raine <asraine@chromium.org> Reviewed-by: Mathias Bynens <mathias@chromium.org> Reviewed-by: Simon Zünd <szuend@chromium.org>
adamraine
changed the title
adamraine
changed the title
adamraine
force-pushed
the
worker-events-legacy
branch
from
243088e
to
a7c76d7
Compare
adamraine
commented
Sep 6, 2023
| @@ -65,7 +65,7 @@ class ScriptElements extends BaseGatherer { | |||
|
|
|||
| const scriptRecords = networkRecords | |||
| .filter(record => record.resourceType === NetworkRequest.TYPES.Script) | |||
| .filter(record => !record.isOutOfProcessIframe); | |||
| .filter(record => record.sessionTargetType === 'page'); | |||
There was a problem hiding this comment.
There are several places isOutOfProcessIframe is used where it could make more sense to use a sessionTargetType === 'page' now that we have worker requests as well. This is just the only place that caused a test failure.
There was a problem hiding this comment.
Update: I changed the other usages of isOutOfProcessIframe too. We don't use isOutOfProcessIframe anywhere now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #14211
Worker requests will appear in
network-requestsandtotal-byte-weightbut are excluded from the page dependency graph used for lantern simulation.