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

Feature: Complete migration to CDP #539

Merged
merged 82 commits into from
May 29, 2024
Merged

Conversation

amovar18
Copy link
Collaborator

@amovar18 amovar18 commented Mar 1, 2024

Description

This PR aims to use CDP completely rather than using the chrome.webRequest API along with CDP.

Relevant Technical Choices

Frame Tree Creation and Tab Identification:

  • FrameIds received in the requestWillBeSent and responseReceived are sometimes not available in the chrome.debugger.getTargets().
  • We use 'Page.frameAttached` to create a frameTree of the frames being added in the Tab.
  • The source.TabId is sometimes undefined instead source.targetId is defined which we use to look into the frameTree of the tab if that target exists then we get the TabId of the tab and use it to update.
  • PSAT also tracks additional frames that are recorded in the serviceWorker for a Tab. These frames along with frameIds are sent to the DevTools so that the extra frameIds can be appended to the frameIds being used in DevTools.

Cookie analysis:

  • requestWillBeSent and responseReceived events provide frameId and urls.
  • responseReceivedExtraInfo and requestWillBeSentExtraInfo provide information about cookies.
  • extraInfo is not processed until their respective requestWillBeSent or responseReceived has not arrived.
  • If the extraInfo arrives before their counterpart, they are stored in a map. Once the counterpart arrives they are processed.
  • If extraInfo arrives after the counterpart, they are processed immediately.
  • Along with the above the resource URLs of the PSAT analysed are kept in a set where all the resource URLs belonging to a frameId are stored.
  • These resource URLs are used to supplement data from Network.getCookies every 5 seconds to avoid any mismatch of the data.

Testing Instructions

  • Clone this branch.
  • Launch a new browser instance with the PSAT installed.
  • Open any site then open DevTools then navigate to the PSAT tab in DevTools.
  • Click on Enable CDP then click on Reload.
  • Once CDP is enabled you can click on analyse this tab.
  • Now you should see cookies data being loaded.

Additional Information:

Screenshot/Screencast


Checklist

  • I have thoroughly tested this code to the best of my abilities.
  • I have reviewed the code myself before requesting a review.
  • This code is covered by unit tests to verify that it works as intended.
  • The QA of this PR is done by a member of the QA team (to be checked by QA).

@amovar18 amovar18 self-assigned this Mar 1, 2024
@mohdsayed mohdsayed added this to the v0.7.0 milestone Mar 12, 2024
@mohdsayed mohdsayed changed the title Feature: use CDP to parse cookies. Feature: Complete migration to CDP Apr 5, 2024
@mohdsayed mohdsayed modified the milestones: v0.7.0, v1.0.0 Apr 5, 2024

if (
preSetSettings?.allowedNumberOfTabs &&
Object.keys(preSetSettings).includes('isUsingCDP')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional Chaining can help

Suggested change
Object.keys(preSetSettings).includes('isUsingCDP')
preSetSettings?.isUsingCDP

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cant do here because it is a boolean variable so we might not know whether or not the variable exists or it is false.

synchnorousCookieStore.tabMode = sessionStorage.allowedNumberOfTabs;
}

if (Object.keys(sessionStorage).includes('isUsingCDP')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional Chaining

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cant do here because it is a boolean variable so we might not know whether or not the variable exists or it is false.

synchnorousCookieStore.tabMode = storage.allowedNumberOfTabs;
}

if (Object.keys(storage).includes('isUsingCDP')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional Chaining

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cant do here because it is a boolean variable so we might not know whether or not the variable exists or it is false.

@amovar18 amovar18 requested a review from mayan-000 May 28, 2024 05:25
Comment on lines +63 to +65
targets.map(async ({ id, url }) => {
if (url.startsWith('http')) {
await attachCDP({ targetId: id });
Copy link
Collaborator

@mohdsayed mohdsayed May 28, 2024

Choose a reason for hiding this comment

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

Are we attaching CDP to all targets on all ALLOWED_EVENTS every time? Can it have any side effects when the same target is attached CDP more than once, like it happens with event listeners in JavaScript?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If debugger is again attached to a target it errors out with an error saying Debugger already attached to targetId: xyz.
Screenshot 2024-05-29 at 10 57 00

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a comment for it.

Copy link
Collaborator

@mohdsayed mohdsayed left a comment

Choose a reason for hiding this comment

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

Looks good. Great work, thanks.

@mohdsayed mohdsayed merged commit ed655a3 into develop May 29, 2024
4 checks passed
@mohdsayed mohdsayed deleted the feat/use-cdp-completely branch May 29, 2024 07:24
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.

None yet

3 participants