Skip to content

Commit

Permalink
Fix: Performance issues in chrome PSAT extension. (#762)
Browse files Browse the repository at this point in the history
* Memoise and do not update state if no diff is found.

* Dont update url for cookiestore.

* Oly update library detection if data is changed.

* Keep request reponse header as http
Add deep-object-diff to library-detection.

* Add promise queue to library detection. Use onCommitted listener instead of onUpdated listener.

* Remove promise queue.
use oncompleted listener

* Fix the commands.

* Rename function.

* Fix lint issue in the report.

* Remove p-queue

* Set loading to false when user aborts page load.

* Remove extra listener.

---------

Co-authored-by: sayedtaqui <sayedwp@gmail.com>
  • Loading branch information
amovar18 and mohdsayed authored Jul 22, 2024
1 parent 3875007 commit b2c4d58
Show file tree
Hide file tree
Showing 12 changed files with 139 additions and 75 deletions.
7 changes: 7 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"build-storybook": "storybook build",
"build:all": "npm-run-all **:build ",
"tsc-packages:build": "npm run i18n:build && npm run common:build && npm run library-detection:build && npm run design-system:build && npm run analysis-utils:build",
"tsc-packages:dev": "npm run i18n:dev && npm run common:dev && npm run library-detection:dev && npm run design-system:dev && npm run analysis-utils:dev",
"tsc-packages:dev": "run-p i18n:dev common:dev library-detection:dev design-system:dev analysis-utils:dev",
"build-cli": "npm run tsc-packages:build && npm run cli-dashboard:build && npm run cli:build",
"publish:all:local": "npm run build:all && npm run publish:local --workspaces",
"unpublish:all:local": "npm run unpublish:local --workspaces",
Expand All @@ -27,7 +27,8 @@
"analysis-utils:prebuild": "npm run build:remove -w @google-psat/analysis-utils",
"analysis-utils:dev": "npm run dev -w @google-psat/analysis-utils",
"analysis-utils:build": "npm run analysis-utils:prebuild && npm run build -w @google-psat/analysis-utils",
"cli:dev": "npm run tsc-packages:dev && npm run dev -w @google-psat/cli",
"cli:predev": "npm run tsc-packages:dev && npm run dev -w @google-psat/cli",
"cli:dev": "run-p tsc-packages:dev cli:predev",
"cli:prebuild": "npm run build:remove -w @google-psat/cli",
"cli:build": "cross-env NODE_ENV=production npm run build -w @google-psat/cli",
"cli:start": "npm install && npm run cli:dev",
Expand All @@ -37,7 +38,8 @@
"cli-dashboard:prebuild": "npm run build:remove -w @google-psat/cli-dashboard",
"cli-dashboard:build": "npm run cli-dashboard:prebuild && cross-env NODE_ENV=production npm run build -w @google-psat/cli-dashboard",
"cookie-db:update": "node scripts/update-cookie-db.cjs",
"ext:dev": "npm run tsc-packages:dev && npm run dev -w @google-psat/extension",
"ext:predev":"npm run dev -w @google-psat/extension",
"ext:dev": "run-p tsc-packages:dev ext:predev",
"ext:prebuild": "npm run build:remove -w @google-psat/extension",
"ext:build": "npm run tsc-packages:build && npm run ext:prebuild && cross-env NODE_ENV=production npm run build -w @google-psat/extension",
"ext:start": "npm install && npm run ext:dev",
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/cookies.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export type CookieData = {
};
analytics?: CookieAnalytics | null;
url?: string;
headerType?: 'response' | 'request' | 'javascript';
headerType?: 'response' | 'request' | 'javascript' | 'http';
isFirstParty?: boolean | null;
frameIdList?: Array<number | string>;
blockedReasons?: BlockedReason[];
Expand Down
1 change: 1 addition & 0 deletions packages/extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"@google-psat/i18n": "*",
"@google-psat/library-detection": "*",
"classnames": "^2.3.2",
"deep-object-diff": "^1.1.9",
"fast-xml-parser": "^4.3.2",
"file-saver": "^2.0.5",
"p-queue": "^7.3.4",
Expand Down
22 changes: 21 additions & 1 deletion packages/extension/src/store/synchnorousCookieStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -547,11 +547,31 @@ class SynchnorousCookieStore {
) {
sentMessageAnyWhere = true;

const newCookieData: {
[cookieKey: string]: CookieData;
} = {};

Object.keys(this.tabsData[tabId]).forEach((key) => {
newCookieData[key] = {
...this.tabsData[tabId][key],
networkEvents: {
requestEvents: [],
responseEvents: [],
},
url: '',
headerType: ['request', 'response'].includes(
this.tabsData[tabId][key]?.headerType ?? ''
)
? 'http'
: 'javascript',
};
});

await chrome.runtime.sendMessage({
type: NEW_COOKIE_DATA,
payload: {
tabId,
cookieData: this.tabsData[tabId],
cookieData: newCookieData,
extraData: {
extraFrameData: this.tabs[tabId].frameIDURLSet,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
/**
* External dependencies
*/
import React, { useMemo } from 'react';
import React, { memo, useMemo } from 'react';
import {
LibraryDetection,
useLibraryDetectionContext,
Expand Down Expand Up @@ -123,4 +123,4 @@ const AssembledCookiesLanding = () => {
</>
);
};
export default AssembledCookiesLanding;
export default memo(AssembledCookiesLanding);
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,9 @@ const useCookieListing = (domainsInAllowList: Set<string>) => {
case I18n.getMessage('jS'):
return value === 'javascript';
case I18n.getMessage('http'):
return value === 'request' || value === 'response';
return (
value === 'request' || value === 'response' || value === 'http'
);
default:
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
/**
* External dependencies.
*/
import React from 'react';
import React, { memo } from 'react';
import {
Button,
CookiesLanding,
Expand Down Expand Up @@ -105,4 +105,4 @@ const Cookies = ({ setFilteredCookies }: CookiesProps) => {
);
};

export default Cookies;
export default memo(Cookies);
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { useSettings } from '../settings';
import { getTab } from '../../../../utils/getTab';
import getFramesForCurrentTab from '../../../../utils/getFramesForCurrentTab';
import Context, { type CookieStoreContext } from './context';
import { diff } from 'deep-object-diff';

const Provider = ({ children }: PropsWithChildren) => {
const [loading, setLoading] = useState<boolean>(true);
Expand Down Expand Up @@ -91,15 +92,22 @@ const Provider = ({ children }: PropsWithChildren) => {

const currentTargets = await chrome.debugger.getTargets();

setTabFrames((prevState) =>
getFramesForCurrentTab(
setTabFrames((prevState) => {
const updatedTabFrames = getFramesForCurrentTab(
prevState,
currentTabFrames,
currentTargets,
extraFrameData ?? {},
isUsingCDP
)
);
);
const isThereDiff = diff(prevState ?? {}, updatedTabFrames);

if (Object.keys(isThereDiff).length === 0) {
return prevState;
}

return updatedTabFrames;
});
},
[isUsingCDP]
);
Expand Down Expand Up @@ -297,7 +305,16 @@ const Provider = ({ children }: PropsWithChildren) => {
if (tabId.toString() === message.payload.tabId.toString()) {
if (isCurrentTabBeingListenedToRef.current) {
setTabToRead(tabId.toString());
setTabCookies(Object.keys(data).length > 0 ? data : null);
setTabCookies((prevState) => {
if (Object.keys(data).length > 0) {
const isThereDiff = diff(prevState ?? {}, data);
if (Object.keys(isThereDiff).length === 0) {
return prevState;
}
return data;
}
return null;
});
await getAllFramesForCurrentTab(frameData);
} else {
setTabFrames(null);
Expand Down Expand Up @@ -414,36 +431,48 @@ const Provider = ({ children }: PropsWithChildren) => {
};
}, []);

return (
<Context.Provider
value={{
state: {
tabCookies,
tabUrl,
tabFrames,
loading,
selectedFrame,
isCurrentTabBeingListenedTo: isCurrentTabBeingListenedToRef.current,
returningToSingleTab,
contextInvalidated,
isInspecting,
canStartInspecting,
tabToRead,
frameHasCookies,
},
actions: {
setSelectedFrame,
changeListeningToThisTab,
getCookiesSetByJavascript,
setIsInspecting,
setContextInvalidated,
setCanStartInspecting,
},
}}
>
{children}
</Context.Provider>
);
const memoisedValue = useMemo(() => {
return {
state: {
tabCookies,
tabUrl,
tabFrames,
loading,
selectedFrame,
isCurrentTabBeingListenedTo: isCurrentTabBeingListenedToRef.current,
returningToSingleTab,
contextInvalidated,
isInspecting,
canStartInspecting,
tabToRead,
frameHasCookies,
},
actions: {
setSelectedFrame,
changeListeningToThisTab,
getCookiesSetByJavascript,
setIsInspecting,
setContextInvalidated,
setCanStartInspecting,
},
};
}, [
canStartInspecting,
changeListeningToThisTab,
contextInvalidated,
frameHasCookies,
getCookiesSetByJavascript,
isInspecting,
loading,
returningToSingleTab,
selectedFrame,
tabCookies,
tabFrames,
tabToRead,
tabUrl,
]);

return <Context.Provider value={memoisedValue}>{children}</Context.Provider>;
};

export default Provider;
1 change: 1 addition & 0 deletions packages/library-detection/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"@google-psat/design-system": "*",
"@google-psat/i18n": "*",
"classnames": "^2.5.1",
"deep-object-diff": "^1.1.9",
"escape-string-regexp": "^4.0.0",
"react": "^18.2.0",
"use-context-selector": "^1.4.1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
executeTaskInDevToolWorker,
LIBRARY_DETECTION_WORKER_TASK,
} from '@google-psat/common';

import { diff } from 'deep-object-diff';
/**
* Internal dependencies.
*/
Expand Down Expand Up @@ -91,8 +91,13 @@ const useLibraryDetection = () => {
matches,
realtimeComputationResult
);
const diffedLibraryMatches = diff(data, matches);

if (Object.keys(diffedLibraryMatches).length > 0) {
return data;
}

return data;
return matches;
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,24 +63,30 @@ const LibraryDetectionProvider = ({ children }: PropsWithChildren) => {
error !== 'net::ERR_ABORTED'
) {
setErrorOccured(true);
} else if (
frameId === 0 &&
_tabId === chrome.devtools.inspectedWindow.tabId &&
error === 'net::ERR_ABORTED'
) {
setIsCurrentTabLoading(false);
}
},
[]
);

// It is attached, next time the tab is updated or reloaded.
const onTabUpdate = useCallback(
(changingTabId: number, changeInfo: chrome.tabs.TabChangeInfo) => {
if (Number(changingTabId) === Number(tabId)) {
if (changeInfo.status === 'complete') {
setIsCurrentTabLoading(false);
} else if (changeInfo.status === 'loading') {
setLibraryMatches(initialLibraryMatches);
setIsCurrentTabLoading(true);
setShowLoader(true);
setLoadedBeforeState(false);
setIsInitialDataUpdated(false);
}
const onCompletedListener = useCallback(
({
frameId,
frameType,
tabId: changingTabId,
}: chrome.webNavigation.WebNavigationFramedCallbackDetails) => {
if (
frameId === 0 &&
frameType === 'outermost_frame' &&
Number(changingTabId) === Number(tabId)
) {
setIsCurrentTabLoading(false);
setErrorOccured(false);
}
},
[tabId]
Expand All @@ -102,28 +108,19 @@ const LibraryDetectionProvider = ({ children }: PropsWithChildren) => {
[tabId]
);

const onCompleted = useCallback(
({ frameId }: chrome.webNavigation.WebNavigationFramedCallbackDetails) => {
if (frameId === 0) {
setErrorOccured(false);
}
},
[]
);

useEffect(() => {
chrome.tabs.onUpdated.removeListener(onTabUpdate);
chrome.tabs.onUpdated.addListener(onTabUpdate);
chrome.webNavigation.onCompleted.addListener(onCompletedListener);
chrome.webNavigation.onErrorOccurred.addListener(onErrorOccuredListener);
chrome.webNavigation.onBeforeNavigate.addListener(onNavigatedListener);
chrome.webNavigation.onCompleted.addListener(onCompleted);

return () => {
chrome.tabs.onUpdated.removeListener(onTabUpdate);
chrome.webNavigation.onCompleted.removeListener(onCompletedListener);
chrome.webNavigation.onBeforeNavigate.removeListener(onNavigatedListener);
chrome.webNavigation.onCompleted.removeListener(onCompleted);
chrome.webNavigation.onErrorOccurred.removeListener(
onErrorOccuredListener
);
};
}, [onTabUpdate, onErrorOccuredListener, onNavigatedListener, onCompleted]);
}, [onErrorOccuredListener, onNavigatedListener, onCompletedListener]);

return (
<Context.Provider
Expand Down

0 comments on commit b2c4d58

Please sign in to comment.