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 throttleCPU on page #1095

Merged
merged 5 commits into from
Nov 14, 2023
Merged

Add throttleCPU on page #1095

merged 5 commits into from
Nov 14, 2023

Conversation

ankur22
Copy link
Collaborator

@ankur22 ankur22 commented Nov 9, 2023

What?

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

Why?

A good use case for throttling the cpu is to see (within a lab environment) how the frontend reacts when the test is ran on a mobile device. With new measurements under a constraint CPU, 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 changed the base branch from main to add/network-throttle November 9, 2023 17:00
@ankur22 ankur22 force-pushed the add/cpu-throttle branch 2 times, most recently from a786e84 to 17637e3 Compare November 9, 2023 17:04
@ankur22 ankur22 force-pushed the add/cpu-throttle branch 2 times, most recently from 8ea211b to 426fdf3 Compare November 9, 2023 17:21
examples/throttle.js Outdated Show resolved Hide resolved
@ankur22 ankur22 force-pushed the add/cpu-throttle branch 3 times, most recently from 9849922 to 06ba5e0 Compare November 10, 2023 17:32
inancgumus
inancgumus previously approved these changes Nov 13, 2023
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! LGTM. Added some weakly held suggestions. Feel free to skip.

tests/page_test.go Show resolved Hide resolved
tests/page_test.go Outdated Show resolved Hide resolved
tests/page_test.go Show resolved Hide resolved
browser/mapping_test.go Outdated Show resolved Hide resolved
Base automatically changed from add/network-throttle to main November 13, 2023 13:26
@ankur22 ankur22 dismissed inancgumus’s stale review November 13, 2023 13:26

The base branch was changed.

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.

This adds the bare bones implementation which includes all the changes
to get it to work with the mapping layer and a new CPUProfile type.
This is added to the frame session. Each page can contain multiple
frame sessions, and each frame session contains a single session. The
CDP throttle cpu API works at the session level, whereas the new API
we're adding works at the page scope. This implementation is required
here before wiring it into the page.
This wires page up with its frame sessions, calling each frame session
to action on the call to throttle the CPU.
This update adds the use of throttleCPU into the example test script.
@ankur22 ankur22 merged commit e37d96f into main Nov 14, 2023
17 checks passed
@ankur22 ankur22 deleted the add/cpu-throttle branch November 14, 2023 16:06
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