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

Add throttleNetwork on page #1094

Merged
merged 8 commits into from
Nov 13, 2023
Merged

Add throttleNetwork on page #1094

merged 8 commits into from
Nov 13, 2023

Conversation

ankur22
Copy link
Collaborator

@ankur22 ankur22 commented Nov 9, 2023

What?

This adds a new throttleNetwork API on page which throttles the network from chrome's perspective.

Why?

A good use case for throttling the network is to see (within a lab environment) how the frontend reacts when the network is slow, e.g. on a slow 3G connection. With new measurements under a constraint network, users will be able to fine tune their website so that the most important information is returned in an acceptable timeframe.

Checklist

  • I have performed a self-review of my code
  • I have added tests for my changes
  • I have commented on my code, particularly in hard-to-understand areas

Related PR(s)/Issue(s)

Updates #887

@ankur22 ankur22 force-pushed the add/network-throttle branch from a96df23 to 0c7b9b2 Compare November 9, 2023 15:04
common/network_manager.go Outdated Show resolved Hide resolved
@ankur22 ankur22 force-pushed the add/network-throttle branch from 0c7b9b2 to 793c9c5 Compare November 9, 2023 15:10
@ankur22 ankur22 requested review from inancgumus and ka3de November 9, 2023 15:17
@ankur22 ankur22 force-pushed the add/network-throttle branch 2 times, most recently from a38f32c to 16c8a12 Compare November 9, 2023 17:08
Copy link
Collaborator

@ka3de ka3de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @ankur22 👏
Just a couple of considerations.

common/network_manager.go Show resolved Hide resolved
browser/mapping_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@ka3de ka3de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👏

Copy link
Member

@inancgumus inancgumus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Thanks 👏

This is the bare bones mapping and implementation. It also adds the
necessary new NetworkProfile type to enable this to work.
This adds the implementation to call chrome to perform the CDP request
which will throttle the network in chrome itself.
A page can be made up of many frames, and each frame has its own
session. This session maps directly to one in chrome. Generally a
website will have one main frame.

The new throttling api is scoped to the page so all the frames will
need to have their sessions updated for a single page. This change
helps with that.
Now we wire up the page with the frames.
The state of the network profile needs to be stored in memory so that
it isn't overriden when the offline state is changed with setOffline.
Renaming them by removing throughput. It should be clear from the
context that they reference the throughput and not any other aspect of
the network.
@ankur22 ankur22 force-pushed the add/network-throttle branch from 98693e5 to ba04a12 Compare November 13, 2023 12:01
@ankur22 ankur22 merged commit 49070fd into main Nov 13, 2023
17 checks passed
@ankur22 ankur22 deleted the add/network-throttle branch November 13, 2023 13:26
@ankur22 ankur22 mentioned this pull request Nov 13, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants