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

Implement Network.loadNetworkResource etc in C++ #44845

Closed
wants to merge 1 commit into from

Conversation

robhogan
Copy link
Contributor

@robhogan robhogan commented Jun 9, 2024

Summary:

Design

  • NetworkIOAgent is owned by the HostAgent.
  • NetworkIOAgent is passed any CDP requests not handled by the HostAgent itself, and before delegating to InstanceAgent.
  • It handles:
  • NetworkIOAgent.loadNetworkResource creates a Stream corresponding to a single resource download/upload. A reference is held in a map streams_ until an error, the agent is disconnected (destroyed) or it is discarded by the frontend with IO.close.
  • delegate.loadNetworkResource is called with a stream-scoped executor, which it uses to call back with headers, data and errors.
  • Callbacks for IO.read requests are held by the Stream until the incoming data is complete or enough data is available to fill the request (an implementation choice to optimise for fewest round trips). Any incoming data or error causes any pending requests to be rechecked.

{F1719616688}

Unimplemented platforms

  • Platforms may optionally implement HostTargetDelegate.networkRequest (as of this diff, none do). If they don't we report a CDP "not implemented" error, similar to the status quo where it was unimplemented by the C++ agent.

Changelog:
[General][Added] Debugging: implement common C++ layer of CDP Network.loadNetworkResource

Differential Revision: D54309633

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Jun 9, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D54309633

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D54309633

robhogan added a commit to robhogan/react-native that referenced this pull request Jun 9, 2024
Summary:
Pull Request resolved: facebook#44845

## Design
 - `NetworkIO` is an object owned by the `HostAgent`, created by `HostTarget` where it is given a scoped executor.
 - `HostAgent` passes most handling of CDP `Network.loadNetworkResource` through `NetworkIO`.
 - `NetworkIO.loadNetworkResource` creates and holds a shared_ptr to a `Stream` representing a single resource load, and owning received headers and data. A reference is held in a map `streams_` until an error or it is closed with `IO.close`. `delegate.networkRequest` is called with the `stream`, which it retains for the lifetime of the request, and uses its methods to call back with headers, data and errors.

 - Callbacks for `IO.read` requests are held by the `Stream` until the incoming data is complete or enough data is available to fill the request (an implementation choice to optimise for fewest round trips). Any incoming data or error causes any pending requests to be rechecked.

## Unimplemented platforms
 - Platforms may optionally implement `HostTargetDelegate.networkRequest` (as of this diff, none do). If they don't we report a CDP "not implemented" error, similar to the status quo where it was unimplemented by the C++ agent.

Differential Revision: D54309633
robhogan added a commit to robhogan/react-native that referenced this pull request Jun 9, 2024
Summary:
Pull Request resolved: facebook#44845

## Design
 - `NetworkIO` is an object owned by the `HostAgent`, created by `HostTarget` where it is given a scoped executor.
 - `HostAgent` passes most handling of CDP `Network.loadNetworkResource` through `NetworkIO`.
 - `NetworkIO.loadNetworkResource` creates and holds a shared_ptr to a `Stream` representing a single resource load, and owning received headers and data. A reference is held in a map `streams_` until an error or it is closed with `IO.close`. `delegate.networkRequest` is called with the `stream`, which it retains for the lifetime of the request, and uses its methods to call back with headers, data and errors.

 - Callbacks for `IO.read` requests are held by the `Stream` until the incoming data is complete or enough data is available to fill the request (an implementation choice to optimise for fewest round trips). Any incoming data or error causes any pending requests to be rechecked.

## Unimplemented platforms
 - Platforms may optionally implement `HostTargetDelegate.networkRequest` (as of this diff, none do). If they don't we report a CDP "not implemented" error, similar to the status quo where it was unimplemented by the C++ agent.

Differential Revision: D54309633
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D54309633

1 similar comment
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D54309633

robhogan added a commit to robhogan/react-native that referenced this pull request Jun 9, 2024
Summary:
Pull Request resolved: facebook#44845

## Design
 - `NetworkIO` is an object owned by the `HostAgent`, created by `HostTarget` where it is given a scoped executor.
 - `HostAgent` passes most handling of CDP `Network.loadNetworkResource` through `NetworkIO`.
 - `NetworkIO.loadNetworkResource` creates and holds a shared_ptr to a `Stream` representing a single resource load, and owning received headers and data. A reference is held in a map `streams_` until an error or it is closed with `IO.close`. `delegate.networkRequest` is called with the `stream`, which it retains for the lifetime of the request, and uses its methods to call back with headers, data and errors.

 - Callbacks for `IO.read` requests are held by the `Stream` until the incoming data is complete or enough data is available to fill the request (an implementation choice to optimise for fewest round trips). Any incoming data or error causes any pending requests to be rechecked.

## Unimplemented platforms
 - Platforms may optionally implement `HostTargetDelegate.networkRequest` (as of this diff, none do). If they don't we report a CDP "not implemented" error, similar to the status quo where it was unimplemented by the C++ agent.

Differential Revision: D54309633
Summary:
Pull Request resolved: facebook#44845

## Design
 - `NetworkIO` is an object owned by the `HostAgent`, created by `HostTarget` where it is given a scoped executor.
 - `HostAgent` passes most handling of CDP `Network.loadNetworkResource` through `NetworkIO`.
 - `NetworkIO.loadNetworkResource` creates and holds a shared_ptr to a `Stream` representing a single resource load, and owning received headers and data. A reference is held in a map `streams_` until an error or it is closed with `IO.close`. `delegate.networkRequest` is called with the `stream`, which it retains for the lifetime of the request, and uses its methods to call back with headers, data and errors.

 - Callbacks for `IO.read` requests are held by the `Stream` until the incoming data is complete or enough data is available to fill the request (an implementation choice to optimise for fewest round trips). Any incoming data or error causes any pending requests to be rechecked.

## Unimplemented platforms
 - Platforms may optionally implement `HostTargetDelegate.networkRequest` (as of this diff, none do). If they don't we report a CDP "not implemented" error, similar to the status quo where it was unimplemented by the C++ agent.

Differential Revision: D54309633
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D54309633

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 193cdc3.

Copy link

This pull request was successfully merged by @robhogan in 193cdc3.

When will my fix make it into a release? | How to file a pick request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants