From 5bcabdd3f06af91de9401ba8e2a03a221d9171ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 10 Oct 2024 13:21:44 +0200 Subject: [PATCH 01/14] Add the mechanisms to retrieve the favicons via WebChannel --- src/app-logic/browser-connection.js | 16 ++++++++++++++++ src/app-logic/web-channel.js | 23 +++++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/app-logic/browser-connection.js b/src/app-logic/browser-connection.js index 04ede410ad..c544291b80 100644 --- a/src/app-logic/browser-connection.js +++ b/src/app-logic/browser-connection.js @@ -11,6 +11,7 @@ import { getSymbolTableViaWebChannel, queryWebChannelVersionViaWebChannel, querySymbolicationApiViaWebChannel, + getPageFaviconsViaWebChannel, } from './web-channel'; import type { Milliseconds } from 'firefox-profiler/types'; @@ -68,6 +69,8 @@ export interface BrowserConnection { debugName: string, breakpadId: string ): Promise; + + getPageFavicons(pageUrls: Array): Promise>; } /** @@ -81,12 +84,14 @@ class BrowserConnectionImpl implements BrowserConnection { _webChannelSupportsGetProfileAndSymbolication: boolean; _webChannelSupportsGetExternalPowerTracks: boolean; _webChannelSupportsGetExternalMarkers: boolean; + _webChannelSupportsGetPageFavicons: boolean; _geckoProfiler: $GeckoProfiler | void; constructor(webChannelVersion: number) { this._webChannelSupportsGetProfileAndSymbolication = webChannelVersion >= 1; this._webChannelSupportsGetExternalPowerTracks = webChannelVersion >= 2; this._webChannelSupportsGetExternalMarkers = webChannelVersion >= 3; + this._webChannelSupportsGetPageFavicons = webChannelVersion >= 4; } // Only called when we must obtain the profile from the browser, i.e. if we @@ -181,6 +186,17 @@ class BrowserConnectionImpl implements BrowserConnection { 'Cannot obtain a symbol table: have neither WebChannel nor a GeckoProfiler object' ); } + + async getPageFavicons( + pageUrls: Array + ): Promise> { + // This is added in Firefox 133. + if (this._webChannelSupportsGetPageFavicons) { + return getPageFaviconsViaWebChannel(pageUrls); + } + + return []; + } } // Should work with: diff --git a/src/app-logic/web-channel.js b/src/app-logic/web-channel.js index a8d733e56a..389e5c3058 100644 --- a/src/app-logic/web-channel.js +++ b/src/app-logic/web-channel.js @@ -27,7 +27,8 @@ export type Request = | GetExternalMarkersRequest | GetExternalPowerTracksRequest | GetSymbolTableRequest - | QuerySymbolicationApiRequest; + | QuerySymbolicationApiRequest + | GetPageFaviconsRequest; type StatusQueryRequest = {| type: 'STATUS_QUERY' |}; type EnableMenuButtonRequest = {| type: 'ENABLE_MENU_BUTTON' |}; @@ -52,6 +53,10 @@ type QuerySymbolicationApiRequest = {| path: string, requestJson: string, |}; +type GetPageFaviconsRequest = {| + type: 'GET_PAGE_FAVICONS', + pageUrls: Array, +|}; export type MessageFromBrowser = | OutOfBandErrorMessageFromBrowser @@ -82,7 +87,8 @@ export type ResponseFromBrowser = | GetExternalMarkersResponse | GetExternalPowerTracksResponse | GetSymbolTableResponse - | QuerySymbolicationApiResponse; + | QuerySymbolicationApiResponse + | GetPageFaviconsResponse; type StatusQueryResponse = {| menuButtonIsEnabled: boolean, @@ -114,6 +120,7 @@ type GetExternalMarkersResponse = ExternalMarkersData; type GetExternalPowerTracksResponse = MixedObject[]; type GetSymbolTableResponse = SymbolTableAsTuple; type QuerySymbolicationApiResponse = string; +type GetPageFaviconsResponse = Array; // Manually declare all pairs of request + response for Flow. /* eslint-disable no-redeclare */ @@ -138,6 +145,9 @@ declare function _sendMessageWithResponse( declare function _sendMessageWithResponse( QuerySymbolicationApiRequest ): Promise; +declare function _sendMessageWithResponse( + GetPageFaviconsRequest +): Promise; /* eslint-enable no-redeclare */ /** @@ -226,6 +236,15 @@ export async function querySymbolicationApiViaWebChannel( }); } +export async function getPageFaviconsViaWebChannel( + pageUrls: Array +): Promise { + return _sendMessageWithResponse({ + type: 'GET_PAGE_FAVICONS', + pageUrls, + }); +} + /** * ----------------------------------------------------------------------------- * From 7200193d20cc7097df59ac709a491144a30261a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Sat, 12 Oct 2024 14:35:01 +0200 Subject: [PATCH 02/14] Retrieve favicons from the browser on the initial load --- src/actions/receive-profile.js | 35 ++++++++++++++++++++++++++++++++++ src/reducers/profile-view.js | 10 ++++++++++ src/types/actions.js | 7 ++++++- src/types/profile.js | 5 +++++ 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/actions/receive-profile.js b/src/actions/receive-profile.js index bbaf8101fe..f10fafab42 100644 --- a/src/actions/receive-profile.js +++ b/src/actions/receive-profile.js @@ -90,6 +90,7 @@ import type { InnerWindowID, Pid, OriginsTimelineRoot, + PageList, } from 'firefox-profiler/types'; import type { @@ -279,6 +280,10 @@ export function finalizeProfileView( await doSymbolicateProfile(dispatch, profile, symbolStore); } } + + if (browserConnection && pages && pages.length > 0) { + await retrievePageFaviconsFromBrowser(dispatch, pages, browserConnection); + } }; } @@ -1017,6 +1022,36 @@ export async function doSymbolicateProfile( dispatch(doneSymbolicating()); } +export async function retrievePageFaviconsFromBrowser( + dispatch: Dispatch, + pages: PageList, + browserConnection: BrowserConnection +) { + const newPages = [...pages]; + + await browserConnection + .getPageFavicons(newPages.map((p) => p.url)) + .then((favicons) => { + if (newPages.length !== favicons.length) { + // It appears that an error occurred since the pages and favicons arrays + // have different lengths. Return early without doing anything. + return; + } + + for (let index = 0; index < favicons.length; index++) { + newPages[index] = { + ...newPages[index], + favicon: favicons[index], + }; + } + }); + + dispatch({ + type: 'UPDATE_PAGES', + newPages, + }); +} + // From a BrowserConnectionStatus, this unwraps the included browserConnection // when possible. export function unwrapBrowserConnection( diff --git a/src/reducers/profile-view.js b/src/reducers/profile-view.js index 5a10ca65b7..0f49ed7c91 100644 --- a/src/reducers/profile-view.js +++ b/src/reducers/profile-view.js @@ -72,6 +72,16 @@ const profile: Reducer = (state = null, action) => { }, }; } + case 'UPDATE_PAGES': { + if (state === null) { + throw new Error( + `Assumed that a profile would be loaded by the time for the pages update` + ); + } + + const { newPages } = action; + return { ...state, pages: newPages }; + } default: return state; } diff --git a/src/types/actions.js b/src/types/actions.js index cc25d06f05..cdf051fb28 100644 --- a/src/types/actions.js +++ b/src/types/actions.js @@ -13,6 +13,7 @@ import type { TabID, IndexIntoCategoryList, IndexIntoLibs, + PageList, } from './profile'; import type { CallNodePath, @@ -442,7 +443,11 @@ type ReceiveProfileAction = | {| +type: 'WAITING_FOR_PROFILE_FROM_BROWSER' |} | {| +type: 'WAITING_FOR_PROFILE_FROM_STORE' |} | {| +type: 'WAITING_FOR_PROFILE_FROM_URL', +profileUrl: ?string |} - | {| +type: 'TRIGGER_LOADING_FROM_URL', +profileUrl: string |}; + | {| +type: 'TRIGGER_LOADING_FROM_URL', +profileUrl: string |} + | {| + +type: 'UPDATE_PAGES', + +newPages: PageList, + |}; type UrlEnhancerAction = | {| +type: 'START_FETCHING_PROFILES' |} diff --git a/src/types/profile.js b/src/types/profile.js index 07463b12b7..bbffdb559d 100644 --- a/src/types/profile.js +++ b/src/types/profile.js @@ -479,6 +479,11 @@ export type Page = {| // capturing was disabled when a private browsing window was open. // The property is always present in Firefox 98+. isPrivateBrowsing?: boolean, + // Favicon data of the page if it was successfully retrieved from Firefox. + // It's a base64 encoded URI string when available. + // It's null when Firefox can't get the favicon. + // This is added in Firefox 133, earlier profiles will not have it. + favicon?: string | null, |}; export type PageList = Array; From b087655964c87630adb0842ceeb6ac4be2e917fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 15 Oct 2024 20:18:03 +0200 Subject: [PATCH 03/14] Sanitize the favicon data while sanitizing the urls --- src/profile-logic/sanitize.js | 2 ++ src/test/unit/sanitize.test.js | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/profile-logic/sanitize.js b/src/profile-logic/sanitize.js index 7b8afdd562..309088cba0 100644 --- a/src/profile-logic/sanitize.js +++ b/src/profile-logic/sanitize.js @@ -120,6 +120,8 @@ export function sanitizePII( pages = pages.map((page, pageIndex) => ({ ...page, url: removeURLs(page.url, ``), + // Remove the favicon data as it could reveal the url. + favicon: null, })); } } diff --git a/src/test/unit/sanitize.test.js b/src/test/unit/sanitize.test.js index e2b1f04451..801427f86a 100644 --- a/src/test/unit/sanitize.test.js +++ b/src/test/unit/sanitize.test.js @@ -448,6 +448,32 @@ describe('sanitizePII', function () { expect(includesChromeUrl).toBe(true); }); + it('should sanitize the favicons in the pages information', function () { + const profile = processGeckoProfile(createGeckoProfile()); + // Add some favicons to check later + ensureExists(profile.pages)[1].favicon = + 'data:image/png;base64,mock-base64-image-data'; + + const { originalProfile, sanitizedProfile } = setup( + { shouldRemoveUrls: true }, + profile + ); + + // Checking to make sure that we have favicons in the original profile pages array. + const pageUrl = ensureExists(originalProfile.pages).find( + (page) => page.favicon + ); + if (pageUrl === undefined) { + throw new Error( + "There should be a favicon in the 'pages' array in this profile." + ); + } + + for (const page of ensureExists(sanitizedProfile.pages)) { + expect(page.favicon).toBe(null); + } + }); + it('should sanitize all the URLs inside network markers', function () { const { sanitizedProfile } = setup({ shouldRemoveUrls: true, From b92ad9a702d29afa8cd3fb4dbdbac14df78392e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 15 Oct 2024 21:47:45 +0200 Subject: [PATCH 04/14] Provide window.crypto in jest In the following commit, we are going to be using the sha1 code that we have that relies on window.crypto. We have to add it to jest as well to make sure that we don't break the tests. --- src/test/setup.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/setup.js b/src/test/setup.js index 8ce6d61d6b..83a808f6dc 100644 --- a/src/test/setup.js +++ b/src/test/setup.js @@ -10,6 +10,7 @@ import '@testing-library/jest-dom'; import fetchMock from 'fetch-mock-jest'; import { Headers, Request, Response } from 'node-fetch'; import { TextDecoder, TextEncoder } from 'util'; +import crypto from 'crypto'; jest.mock('../utils/worker-factory'); import * as WorkerFactory from '../utils/worker-factory'; @@ -83,3 +84,10 @@ expect.extend({ }; }, }); + +Object.defineProperty(global.self, 'crypto', { + value: { + // $FlowExpectError This flow version doesn't know about webcrypto + subtle: crypto.webcrypto.subtle, + }, +}); From f643aab6f7337780cf2c32f234079a10d7506987 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 15 Oct 2024 21:06:00 +0200 Subject: [PATCH 05/14] Make it possible to pass data urls to the Icon component --- src/actions/icons.js | 33 ++++++++++++++++++--------- src/components/app/ProfileViewer.js | 6 ++--- src/reducers/icons.js | 4 ++-- src/selectors/icons.js | 28 +++++++---------------- src/test/store/icons.test.js | 35 ++++++++++++++--------------- src/types/actions.js | 2 +- src/types/state.js | 10 ++++----- 7 files changed, 59 insertions(+), 59 deletions(-) diff --git a/src/actions/icons.js b/src/actions/icons.js index d0b94ffb90..85488134b9 100644 --- a/src/actions/icons.js +++ b/src/actions/icons.js @@ -4,11 +4,12 @@ // @flow import type { Action, ThunkAction } from 'firefox-profiler/types'; +import sha1 from 'firefox-profiler/utils/sha1'; -export function iconHasLoaded(icon: string): Action { +export function iconHasLoaded(iconWithClassName: [string, string]): Action { return { type: 'ICON_HAS_LOADED', - icon, + iconWithClassName, }; } @@ -21,24 +22,27 @@ export function iconIsInError(icon: string): Action { const icons: Set = new Set(); -type IconRequestResult = 'loaded' | 'error' | 'cached'; +type IconRequestResult = + | {| type: 'error' | 'cached' |} + | {| type: 'loaded', iconWithClassName: [string, string] |}; -function _getIcon(icon: string): Promise { +async function _getIcon(icon: string): Promise { if (icons.has(icon)) { - return Promise.resolve('cached'); + return Promise.resolve({ type: 'cached' }); } icons.add(icon); + const className = await _classNameFromUrl(icon); const result = new Promise((resolve) => { const image = new Image(); image.src = icon; image.referrerPolicy = 'no-referrer'; image.onload = () => { - resolve('loaded'); + resolve({ type: 'loaded', iconWithClassName: [icon, className] }); }; image.onerror = () => { - resolve('error'); + resolve({ type: 'error' }); }; }); @@ -48,9 +52,9 @@ function _getIcon(icon: string): Promise { export function iconStartLoading(icon: string): ThunkAction> { return (dispatch) => { return _getIcon(icon).then((result) => { - switch (result) { + switch (result.type) { case 'loaded': - dispatch(iconHasLoaded(icon)); + dispatch(iconHasLoaded(result.iconWithClassName)); break; case 'error': dispatch(iconIsInError(icon)); @@ -59,8 +63,17 @@ export function iconStartLoading(icon: string): ThunkAction> { // nothing to do break; default: - throw new Error(`Unknown icon load result ${result}`); + throw new Error(`Unknown icon load result ${result.type}`); } }); }; } + +/** + * Transforms a URL into a valid CSS class name. + */ +async function _classNameFromUrl(url): Promise { + return url.startsWith('data:image/') + ? 'dataUrl' + (await sha1(url)) + : url.replace(/[/:.+>< ~()#,]/g, '_'); +} diff --git a/src/components/app/ProfileViewer.js b/src/components/app/ProfileViewer.js index 69b0ae1d34..1248f7fc50 100644 --- a/src/components/app/ProfileViewer.js +++ b/src/components/app/ProfileViewer.js @@ -38,7 +38,7 @@ import { BackgroundImageStyleDef } from 'firefox-profiler/components/shared/Styl import classNames from 'classnames'; import { DebugWarning } from 'firefox-profiler/components/app/DebugWarning'; -import type { CssPixels, IconWithClassName } from 'firefox-profiler/types'; +import type { CssPixels, IconsWithClassNames } from 'firefox-profiler/types'; import type { ConnectedProps } from 'firefox-profiler/utils/connect'; import './ProfileViewer.css'; @@ -50,7 +50,7 @@ type StateProps = {| +isUploading: boolean, +isHidingStaleProfile: boolean, +hasSanitizedProfile: boolean, - +icons: IconWithClassName[], + +icons: IconsWithClassNames, +isBottomBoxOpen: boolean, |}; @@ -83,7 +83,7 @@ class ProfileViewerImpl extends PureComponent { profileViewerWrapperBackground: hasSanitizedProfile, })} > - {icons.map(({ className, icon }) => ( + {[...icons].map(([icon, className]) => ( > = (state = new Set(), action) => { +const favicons: Reducer> = (state = new Map(), action) => { switch (action.type) { case 'ICON_HAS_LOADED': - return new Set([...state, action.icon]); + return new Map([...state.entries(), action.iconWithClassName]); case 'ICON_IN_ERROR': // nothing to do default: return state; diff --git a/src/selectors/icons.js b/src/selectors/icons.js index 435c83d128..56f0c7e2f7 100644 --- a/src/selectors/icons.js +++ b/src/selectors/icons.js @@ -3,9 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ // @flow -import { createSelector } from 'reselect'; import type { - IconWithClassName, IconState, Selector, DangerousSelectorWithArguments, @@ -13,8 +11,10 @@ import type { /** * A simple selector into the icon state. + * It returns a map that matches icon to the icon class name. */ -export const getIcons: Selector = (state) => state.icons; +export const getIconsWithClassNames: Selector = (state) => + state.icons; /** * In order to load icons without multiple requests, icons are created through @@ -26,21 +26,9 @@ export const getIconClassName: DangerousSelectorWithArguments< string, string | null, > = (state, icon) => { - const icons = getIcons(state); - return icon !== null && icons.has(icon) ? _classNameFromUrl(icon) : ''; + if (icon === null) { + return ''; + } + const icons = getIconsWithClassNames(state); + return icons.get(icon) ?? ''; }; - -/** - * This functions returns an object with both the icon URL and the class name. - */ -export const getIconsWithClassNames: Selector = - createSelector(getIcons, (icons) => - [...icons].map((icon) => ({ icon, className: _classNameFromUrl(icon) })) - ); - -/** - * Transforms a URL into a valid CSS class name. - */ -function _classNameFromUrl(url): string { - return url.replace(/[/:.+>< ~()#,]/g, '_'); -} diff --git a/src/test/store/icons.test.js b/src/test/store/icons.test.js index ff78a1397e..c577031a6b 100644 --- a/src/test/store/icons.test.js +++ b/src/test/store/icons.test.js @@ -56,9 +56,10 @@ describe('actions/icons', function () { return blankStore().getState(); } - it('getIcons return an empty set', function () { - const initialState = iconsAccessors.getIcons(getInitialState()); - expect(initialState).toBeInstanceOf(Set); + it('getIconsWithClassNames return an empty map', function () { + const initialState = + iconsAccessors.getIconsWithClassNames(getInitialState()); + expect(initialState).toBeInstanceOf(Map); expect(initialState.size).toEqual(0); }); @@ -69,11 +70,6 @@ describe('actions/icons', function () { ); expect(subject).toBe(''); }); - - it('getIconsWithClassNames returns an empty array', function () { - const subject = iconsAccessors.getIconsWithClassNames(getInitialState()); - expect(subject).toEqual([]); - }); }); describe('Requesting an existing icon', function () { @@ -87,6 +83,11 @@ describe('actions/icons', function () { dispatch(iconsActions.iconStartLoading(validIcons[1])), ]; + // Let the event loop go around for iconStartLoading. Note that we don't + // want to await for the promises yet, since we would like to mock them. + // We will await them later. + await new Promise((res) => requestAnimationFrame(res)); + // Only 2 requests because only 2 different icons expect(imageInstances.length).toBe(2); imageInstances.forEach((instance, i) => { @@ -97,12 +98,9 @@ describe('actions/icons', function () { await Promise.all(promises); const state = getState(); - let subject = iconsAccessors.getIcons(state); - expect([...subject]).toEqual(validIcons); - - subject = iconsAccessors.getIconsWithClassNames(state); - expect(subject).toEqual( - validIcons.map((icon, i) => ({ icon, className: expectedClasses[i] })) + let subject = iconsAccessors.getIconsWithClassNames(state); + expect([...subject]).toEqual( + validIcons.map((icon, i) => [icon, expectedClasses[i]]) ); validIcons.forEach((icon, i) => { @@ -121,13 +119,17 @@ describe('actions/icons', function () { const actionPromise = dispatch( iconsActions.iconStartLoading(invalidIcon) ); + // Let the event loop go around for iconStartLoading. Note that we don't + // want to await for the promises yet, since we would like to mock them. + // We will await them later. + await new Promise((res) => requestAnimationFrame(res)); expect(imageInstances.length).toBe(1); (imageInstances[0]: any).onerror(); await actionPromise; const state = getState(); - let subject = iconsAccessors.getIcons(state); + let subject = iconsAccessors.getIconsWithClassNames(state); expect([...subject]).toEqual([]); subject = iconsAccessors.getIconClassName( @@ -135,9 +137,6 @@ describe('actions/icons', function () { _createCallNodeWithIcon(invalidIcon).icon ); expect(subject).toBe(''); - - subject = iconsAccessors.getIconsWithClassNames(state); - expect(subject).toEqual([]); }); }); }); diff --git a/src/types/actions.js b/src/types/actions.js index cdf051fb28..5a1b4dfb8f 100644 --- a/src/types/actions.js +++ b/src/types/actions.js @@ -577,7 +577,7 @@ type UrlStateAction = |}; type IconsAction = - | {| +type: 'ICON_HAS_LOADED', +icon: string |} + | {| +type: 'ICON_HAS_LOADED', +iconWithClassName: [string, string] |} | {| +type: 'ICON_IN_ERROR', +icon: string |}; type SidebarAction = {| diff --git a/src/types/state.js b/src/types/state.js index 67a8384a77..aaf3481da2 100644 --- a/src/types/state.js +++ b/src/types/state.js @@ -423,7 +423,7 @@ export type L10nState = {| +direction: 'ltr' | 'rtl', |}; -export type IconState = Set; +export type IconState = Map; export type CodeState = {| +sourceCodeCache: Map, @@ -441,7 +441,7 @@ export type State = {| +code: CodeState, |}; -export type IconWithClassName = {| - +icon: string, - +className: string, -|}; +/** + * Map of icons to their class names + */ +export type IconsWithClassNames = Map; From fff37cb20a96ae0134369bb24eacf1ab01e6bb54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Sat, 12 Oct 2024 16:00:47 +0200 Subject: [PATCH 06/14] Show the favicons in the tab selector and profile filter navigator --- src/components/shared/TabSelectorMenu.css | 4 ++++ src/components/shared/TabSelectorMenu.js | 6 ++++++ src/profile-logic/profile-data.js | 14 ++++---------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/components/shared/TabSelectorMenu.css b/src/components/shared/TabSelectorMenu.css index 5da4b4a96a..92c6ad16b3 100644 --- a/src/components/shared/TabSelectorMenu.css +++ b/src/components/shared/TabSelectorMenu.css @@ -17,3 +17,7 @@ /* Move the checkmark to inline-start instead of right, as it's logically better. */ inset-inline: 8px 0; } + +.tabSelectorMenuItem .nodeIcon { + margin-inline-end: 10px; +} diff --git a/src/components/shared/TabSelectorMenu.js b/src/components/shared/TabSelectorMenu.js index d50c78d764..7ca5a663bc 100644 --- a/src/components/shared/TabSelectorMenu.js +++ b/src/components/shared/TabSelectorMenu.js @@ -13,6 +13,7 @@ import explicitConnect from 'firefox-profiler/utils/connect'; import { changeTabFilter } from 'firefox-profiler/actions/receive-profile'; import { getTabFilter } from '../../selectors/url-state'; import { getProfileFilterSortedPageData } from 'firefox-profiler/selectors/profile'; +import { Icon } from 'firefox-profiler/components/shared/Icon'; import type { TabID, SortedTabPageData } from 'firefox-profiler/types'; import type { ConnectedProps } from 'firefox-profiler/utils/connect'; @@ -41,6 +42,10 @@ class TabSelectorMenuImpl extends React.PureComponent { return null; } + const hasSomeIcons = sortedPageData.some( + ({ pageData }) => !!pageData.favicon + ); + return ( <> { 'aria-checked': tabFilter === tabID ? 'false' : 'true', }} > + {hasSomeIcons ? : null} {pageData.hostname} ))} diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 6107bc2a0d..58156f265e 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -2963,7 +2963,8 @@ export function extractProfileFilterPageData( } // The last page is the one we care about. - const pageUrl = topMostPages[topMostPages.length - 1].url; + const currentPage = topMostPages[topMostPages.length - 1]; + const pageUrl = currentPage.url; if (pageUrl.startsWith('about:')) { // If we only have an `about:*` page, we should return early with a friendly // origin and hostname. Otherwise the try block will always fail. @@ -2986,7 +2987,7 @@ export function extractProfileFilterPageData( const pageData: ProfileFilterPageData = { origin: '', hostname: '', - favicon: null, + favicon: currentPage.favicon ?? null, }; try { @@ -3005,15 +3006,8 @@ export function extractProfileFilterPageData( ) ?? '') : page.hostname; - // FIXME(Bug 1620546): This is not ideal and we should get the favicon - // either during profile capture or profile pre-process. pageData.origin = page.origin; - const favicon = new URL('/favicon.ico', page.origin); - if (favicon.protocol === 'http:') { - // Upgrade http requests. - favicon.protocol = 'https:'; - } - pageData.favicon = favicon.href; + pageData.favicon = currentPage.favicon ?? null; } catch (e) { console.warn( 'Error while extracing the hostname and favicon from the page url', From 0c4a2c5d2eddf88d1c3b952a17c97234791d9f07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 15 Oct 2024 21:31:48 +0200 Subject: [PATCH 07/14] Update tests after the favicon changes --- src/test/components/TabSelectorMenu.test.js | 16 +++++++++++++ .../FilterNavigatorBar.test.js.snap | 3 --- .../TabSelectorMenu.test.js.snap | 6 +++++ src/test/unit/profile-data.test.js | 24 ++++++++++++++----- 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/test/components/TabSelectorMenu.test.js b/src/test/components/TabSelectorMenu.test.js index ecc3b8682b..ea1501f5d9 100644 --- a/src/test/components/TabSelectorMenu.test.js +++ b/src/test/components/TabSelectorMenu.test.js @@ -18,12 +18,28 @@ import { import { storeWithProfile } from '../fixtures/stores'; import { fireFullClick } from '../fixtures/utils'; import { getTabFilter } from '../../selectors/url-state'; +import { createImageMock } from '../fixtures/mocks/image'; +import { ensureExists } from 'firefox-profiler/utils/flow'; describe('app/TabSelectorMenu', () => { + beforeAll(() => { + const mock = createImageMock(); + (window: any).Image = mock.Image; + }); + + afterAll(() => { + // Let all the async favicons finish creating the images. + requestAnimationFrame(() => { + delete (window: any).Image; + }); + }); + function setup() { const { profile, ...extraPageData } = addActiveTabInformationToProfile( getProfileWithNiceTracks() ); + ensureExists(profile.pages)[3].favicon = + 'data:image/png;base64,test-png-favicon-data-for-profiler.firefox.com'; // This is needed for the thread activity score calculation. profile.meta.sampleUnits = { diff --git a/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap b/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap index 4b996c5b3f..df3e698b35 100644 --- a/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap +++ b/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap @@ -89,9 +89,6 @@ exports[`app/ProfileFilterNavigator renders the site hostname as its first eleme -
diff --git a/src/test/components/__snapshots__/TabSelectorMenu.test.js.snap b/src/test/components/__snapshots__/TabSelectorMenu.test.js.snap index 65bebfdc66..ed77224990 100644 --- a/src/test/components/__snapshots__/TabSelectorMenu.test.js.snap +++ b/src/test/components/__snapshots__/TabSelectorMenu.test.js.snap @@ -40,6 +40,9 @@ exports[`app/TabSelectorMenu should render properly 1`] = ` role="menuitem" tabindex="-1" > +
profiler.firefox.com
+
mozilla.org
diff --git a/src/test/unit/profile-data.test.js b/src/test/unit/profile-data.test.js index 1ed154632f..bc71e670b6 100644 --- a/src/test/unit/profile-data.test.js +++ b/src/test/unit/profile-data.test.js @@ -1137,18 +1137,22 @@ describe('extractProfileFilterPageData', function () { innerWindowID: 1, url: 'https://www.mozilla.org', embedderInnerWindowID: 0, + favicon: 'data:image/png;base64,test-png-favicon-data-for-mozilla.org', }, aboutBlank: { tabID: 2222, innerWindowID: 2, url: 'about:blank', embedderInnerWindowID: 0, + favicon: null, }, profiler: { tabID: 2222, innerWindowID: 3, url: 'https://profiler.firefox.com/public/xyz', embedderInnerWindowID: 0, + favicon: + 'data:image/png;base64,test-png-favicon-data-for-profiler.firefox.com', }, exampleSubFrame: { tabID: 2222, @@ -1156,12 +1160,14 @@ describe('extractProfileFilterPageData', function () { url: 'https://example.com/subframe', // This is a subframe of the page above. embedderInnerWindowID: 3, + favicon: 'data:image/png;base64,test-png-favicon-data-for-example.com', }, exampleTopFrame: { tabID: 2222, innerWindowID: 5, url: 'https://example.com', embedderInnerWindowID: 0, + favicon: 'data:image/png;base64,test-png-favicon-data-for-example.com', }, }; @@ -1175,7 +1181,8 @@ describe('extractProfileFilterPageData', function () { { origin: 'https://www.mozilla.org', hostname: 'www.mozilla.org', - favicon: 'https://www.mozilla.org/favicon.ico', + favicon: + 'data:image/png;base64,test-png-favicon-data-for-mozilla.org', }, ], ]); @@ -1193,7 +1200,8 @@ describe('extractProfileFilterPageData', function () { { origin: 'https://profiler.firefox.com', hostname: 'profiler.firefox.com', - favicon: 'https://profiler.firefox.com/favicon.ico', + favicon: + 'data:image/png;base64,test-png-favicon-data-for-profiler.firefox.com', }, ], ]); @@ -1211,7 +1219,8 @@ describe('extractProfileFilterPageData', function () { { origin: 'https://profiler.firefox.com', hostname: 'profiler.firefox.com', - favicon: 'https://profiler.firefox.com/favicon.ico', + favicon: + 'data:image/png;base64,test-png-favicon-data-for-profiler.firefox.com', }, ], ]); @@ -1257,7 +1266,8 @@ describe('extractProfileFilterPageData', function () { { origin: 'https://example.com', hostname: 'example.com', - favicon: 'https://example.com/favicon.ico', + favicon: + 'data:image/png;base64,test-png-favicon-data-for-example.com', }, ], ]); @@ -1275,7 +1285,8 @@ describe('extractProfileFilterPageData', function () { { origin: 'https://www.mozilla.org', hostname: 'www.mozilla.org', - favicon: 'https://www.mozilla.org/favicon.ico', + favicon: + 'data:image/png;base64,test-png-favicon-data-for-mozilla.org', }, ], [ @@ -1283,7 +1294,8 @@ describe('extractProfileFilterPageData', function () { { origin: 'https://profiler.firefox.com', hostname: 'profiler.firefox.com', - favicon: 'https://profiler.firefox.com/favicon.ico', + favicon: + 'data:image/png;base64,test-png-favicon-data-for-profiler.firefox.com', }, ], ]); From 40e30684a7a8cc230335500b37e7243ce753e36c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Wed, 16 Oct 2024 00:50:09 +0200 Subject: [PATCH 08/14] [DO NOT LAND] Enable the tab selector --- src/components/app/ProfileFilterNavigator.js | 3 +-- .../FilterNavigatorBar.test.js.snap | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/components/app/ProfileFilterNavigator.js b/src/components/app/ProfileFilterNavigator.js index baf2b6b0d9..f4e1d1bed6 100644 --- a/src/components/app/ProfileFilterNavigator.js +++ b/src/components/app/ProfileFilterNavigator.js @@ -106,8 +106,7 @@ class ProfileFilterNavigatorBarImpl extends React.PureComponent { // profile or when the page information is empty. This could happen for // older profiles and profiles from external importers that don't have // this information. - // eslint-disable-next-line no-constant-condition - if (false && pageDataByTabID && pageDataByTabID.size > 0) { + if (pageDataByTabID && pageDataByTabID.size > 0) { const pageData = tabFilter !== null ? pageDataByTabID.get(tabFilter) : null; diff --git a/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap b/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap index df3e698b35..1a9dc79bb6 100644 --- a/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap +++ b/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap @@ -10,7 +10,12 @@ exports[`app/ProfileFilterNavigator renders ProfileFilterNavigator properly 1`] - Full Range (⁨51ms⁩) + @@ -27,7 +32,11 @@ exports[`app/ProfileFilterNavigator renders ProfileFilterNavigator properly 2`] class="filterNavigatorBarItemContent" type="button" > - Full Range (⁨51ms⁩) + + Full Range (⁨51ms⁩) +
  • - Full Range (⁨51ms⁩) + + Full Range (⁨51ms⁩) +
  • Date: Sun, 20 Oct 2024 18:10:13 +0200 Subject: [PATCH 09/14] Address some small review comments --- src/actions/receive-profile.js | 32 ++++++++++++++++--------------- src/profile-logic/profile-data.js | 1 - src/reducers/profile-view.js | 2 +- src/test/store/icons.test.js | 15 ++++++--------- 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/actions/receive-profile.js b/src/actions/receive-profile.js index f10fafab42..7bac82ec74 100644 --- a/src/actions/receive-profile.js +++ b/src/actions/receive-profile.js @@ -1029,22 +1029,24 @@ export async function retrievePageFaviconsFromBrowser( ) { const newPages = [...pages]; - await browserConnection - .getPageFavicons(newPages.map((p) => p.url)) - .then((favicons) => { - if (newPages.length !== favicons.length) { - // It appears that an error occurred since the pages and favicons arrays - // have different lengths. Return early without doing anything. - return; - } + const favicons = await browserConnection.getPageFavicons( + newPages.map((p) => p.url) + ); - for (let index = 0; index < favicons.length; index++) { - newPages[index] = { - ...newPages[index], - favicon: favicons[index], - }; - } - }); + if (newPages.length !== favicons.length) { + // It appears that an error occurred since the pages and favicons arrays + // have different lengths. Return early without doing anything. + return; + } + + for (let index = 0; index < favicons.length; index++) { + if (favicons[index]) { + newPages[index] = { + ...newPages[index], + favicon: favicons[index], + }; + } + } dispatch({ type: 'UPDATE_PAGES', diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 58156f265e..96c06bf563 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -3007,7 +3007,6 @@ export function extractProfileFilterPageData( : page.hostname; pageData.origin = page.origin; - pageData.favicon = currentPage.favicon ?? null; } catch (e) { console.warn( 'Error while extracing the hostname and favicon from the page url', diff --git a/src/reducers/profile-view.js b/src/reducers/profile-view.js index 0f49ed7c91..46116902b4 100644 --- a/src/reducers/profile-view.js +++ b/src/reducers/profile-view.js @@ -75,7 +75,7 @@ const profile: Reducer = (state = null, action) => { case 'UPDATE_PAGES': { if (state === null) { throw new Error( - `Assumed that a profile would be loaded by the time for the pages update` + `We tried to update the pages information for a non-existent profile.` ); } diff --git a/src/test/store/icons.test.js b/src/test/store/icons.test.js index c577031a6b..2a581d7ae1 100644 --- a/src/test/store/icons.test.js +++ b/src/test/store/icons.test.js @@ -8,6 +8,7 @@ import { blankStore } from '../fixtures/stores'; import * as iconsAccessors from '../../selectors/icons'; import * as iconsActions from '../../actions/icons'; import type { CallNodeDisplayData } from 'firefox-profiler/types'; +import { waitFor } from 'firefox-profiler/test/fixtures/testing-library'; describe('actions/icons', function () { const validIcons = [ @@ -56,7 +57,7 @@ describe('actions/icons', function () { return blankStore().getState(); } - it('getIconsWithClassNames return an empty map', function () { + it('getIconsWithClassNames returns an empty map', function () { const initialState = iconsAccessors.getIconsWithClassNames(getInitialState()); expect(initialState).toBeInstanceOf(Map); @@ -83,10 +84,8 @@ describe('actions/icons', function () { dispatch(iconsActions.iconStartLoading(validIcons[1])), ]; - // Let the event loop go around for iconStartLoading. Note that we don't - // want to await for the promises yet, since we would like to mock them. - // We will await them later. - await new Promise((res) => requestAnimationFrame(res)); + // Wait until we have 2 image instances after calling iconStartLoading. + await waitFor(() => expect(imageInstances.length).toBe(2)); // Only 2 requests because only 2 different icons expect(imageInstances.length).toBe(2); @@ -119,10 +118,8 @@ describe('actions/icons', function () { const actionPromise = dispatch( iconsActions.iconStartLoading(invalidIcon) ); - // Let the event loop go around for iconStartLoading. Note that we don't - // want to await for the promises yet, since we would like to mock them. - // We will await them later. - await new Promise((res) => requestAnimationFrame(res)); + // Wait until we have 2 image instances after calling iconStartLoading. + await waitFor(() => expect(imageInstances.length).toBe(1)); expect(imageInstances.length).toBe(1); (imageInstances[0]: any).onerror(); From 21944bee859771d1cb4f44d248e5a81feaac7861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 21 Oct 2024 14:00:09 +0200 Subject: [PATCH 10/14] Use IconsWithClassNames everywhere instead of IconState --- src/reducers/icons.js | 4 ++-- src/selectors/icons.js | 4 ++-- src/types/state.js | 12 +++++------- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/reducers/icons.js b/src/reducers/icons.js index 0457011439..75f189f265 100644 --- a/src/reducers/icons.js +++ b/src/reducers/icons.js @@ -3,9 +3,9 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ // @flow -import type { Reducer } from 'firefox-profiler/types'; +import type { Reducer, IconsWithClassNames } from 'firefox-profiler/types'; -const favicons: Reducer> = (state = new Map(), action) => { +const favicons: Reducer = (state = new Map(), action) => { switch (action.type) { case 'ICON_HAS_LOADED': return new Map([...state.entries(), action.iconWithClassName]); diff --git a/src/selectors/icons.js b/src/selectors/icons.js index 56f0c7e2f7..05ffdf75a7 100644 --- a/src/selectors/icons.js +++ b/src/selectors/icons.js @@ -4,7 +4,7 @@ // @flow import type { - IconState, + IconsWithClassNames, Selector, DangerousSelectorWithArguments, } from 'firefox-profiler/types'; @@ -13,7 +13,7 @@ import type { * A simple selector into the icon state. * It returns a map that matches icon to the icon class name. */ -export const getIconsWithClassNames: Selector = (state) => +export const getIconsWithClassNames: Selector = (state) => state.icons; /** diff --git a/src/types/state.js b/src/types/state.js index aaf3481da2..332a5459ac 100644 --- a/src/types/state.js +++ b/src/types/state.js @@ -423,7 +423,10 @@ export type L10nState = {| +direction: 'ltr' | 'rtl', |}; -export type IconState = Map; +/** + * Map of icons to their class names + */ +export type IconsWithClassNames = Map; export type CodeState = {| +sourceCodeCache: Map, @@ -434,14 +437,9 @@ export type State = {| +app: AppState, +profileView: ProfileViewState, +urlState: UrlState, - +icons: IconState, + +icons: IconsWithClassNames, +zippedProfiles: ZippedProfilesState, +publish: PublishState, +l10n: L10nState, +code: CodeState, |}; - -/** - * Map of icons to their class names - */ -export type IconsWithClassNames = Map; From e603c2656d81503000e78129659bb5ec2c52a18e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 21 Oct 2024 14:14:36 +0200 Subject: [PATCH 11/14] Convert the iconWithClassName array into an object --- src/actions/icons.js | 18 ++++++++++++++---- src/reducers/icons.js | 6 ++++-- src/types/actions.js | 6 +++++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/actions/icons.js b/src/actions/icons.js index 85488134b9..417baface6 100644 --- a/src/actions/icons.js +++ b/src/actions/icons.js @@ -3,10 +3,17 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ // @flow -import type { Action, ThunkAction } from 'firefox-profiler/types'; +import type { + Action, + ThunkAction, + IconWithClassName, +} from 'firefox-profiler/types'; import sha1 from 'firefox-profiler/utils/sha1'; -export function iconHasLoaded(iconWithClassName: [string, string]): Action { +export function iconHasLoaded(iconWithClassName: {| + +icon: string, + +className: string, +|}): Action { return { type: 'ICON_HAS_LOADED', iconWithClassName, @@ -24,7 +31,10 @@ const icons: Set = new Set(); type IconRequestResult = | {| type: 'error' | 'cached' |} - | {| type: 'loaded', iconWithClassName: [string, string] |}; + | {| + type: 'loaded', + iconWithClassName: IconWithClassName, + |}; async function _getIcon(icon: string): Promise { if (icons.has(icon)) { @@ -39,7 +49,7 @@ async function _getIcon(icon: string): Promise { image.src = icon; image.referrerPolicy = 'no-referrer'; image.onload = () => { - resolve({ type: 'loaded', iconWithClassName: [icon, className] }); + resolve({ type: 'loaded', iconWithClassName: { icon, className } }); }; image.onerror = () => { resolve({ type: 'error' }); diff --git a/src/reducers/icons.js b/src/reducers/icons.js index 75f189f265..7103171304 100644 --- a/src/reducers/icons.js +++ b/src/reducers/icons.js @@ -7,8 +7,10 @@ import type { Reducer, IconsWithClassNames } from 'firefox-profiler/types'; const favicons: Reducer = (state = new Map(), action) => { switch (action.type) { - case 'ICON_HAS_LOADED': - return new Map([...state.entries(), action.iconWithClassName]); + case 'ICON_HAS_LOADED': { + const { icon, className } = action.iconWithClassName; + return new Map([...state.entries(), [icon, className]]); + } case 'ICON_IN_ERROR': // nothing to do default: return state; diff --git a/src/types/actions.js b/src/types/actions.js index 5a1b4dfb8f..e4788fe43d 100644 --- a/src/types/actions.js +++ b/src/types/actions.js @@ -576,8 +576,12 @@ type UrlStateAction = +selectedTab: TabSlug, |}; +export type IconWithClassName = {| +icon: string, +className: string |}; type IconsAction = - | {| +type: 'ICON_HAS_LOADED', +iconWithClassName: [string, string] |} + | {| + +type: 'ICON_HAS_LOADED', + +iconWithClassName: IconWithClassName, + |} | {| +type: 'ICON_IN_ERROR', +icon: string |}; type SidebarAction = {| From 61f7417349028320fcedcf8bacb32527e545885b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 21 Oct 2024 14:53:43 +0200 Subject: [PATCH 12/14] Use a static icon counter for favicon classes --- src/actions/icons.js | 15 ++++++++------- src/test/store/icons.test.js | 6 ++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/actions/icons.js b/src/actions/icons.js index 417baface6..9759326f8a 100644 --- a/src/actions/icons.js +++ b/src/actions/icons.js @@ -8,7 +8,6 @@ import type { ThunkAction, IconWithClassName, } from 'firefox-profiler/types'; -import sha1 from 'firefox-profiler/utils/sha1'; export function iconHasLoaded(iconWithClassName: {| +icon: string, @@ -28,6 +27,7 @@ export function iconIsInError(icon: string): Action { } const icons: Set = new Set(); +let iconCounter = 0; type IconRequestResult = | {| type: 'error' | 'cached' |} @@ -42,7 +42,10 @@ async function _getIcon(icon: string): Promise { } icons.add(icon); - const className = await _classNameFromUrl(icon); + + // New class name for an icon. They are guaranteed to be unique, that's why + // just increment the icon counter and return that string. + const className = `favicon-${++iconCounter}`; const result = new Promise((resolve) => { const image = new Image(); @@ -80,10 +83,8 @@ export function iconStartLoading(icon: string): ThunkAction> { } /** - * Transforms a URL into a valid CSS class name. + * Only use it in tests! */ -async function _classNameFromUrl(url): Promise { - return url.startsWith('data:image/') - ? 'dataUrl' + (await sha1(url)) - : url.replace(/[/:.+>< ~()#,]/g, '_'); +export function _resetIconCounter() { + iconCounter = 0; } diff --git a/src/test/store/icons.test.js b/src/test/store/icons.test.js index 2a581d7ae1..cf91fe8691 100644 --- a/src/test/store/icons.test.js +++ b/src/test/store/icons.test.js @@ -15,10 +15,7 @@ describe('actions/icons', function () { 'https://valid.icon1.example.org/favicon.ico', 'https://valid.icon2.example.org/favicon.ico', ]; - const expectedClasses = [ - 'https___valid_icon1_example_org_favicon_ico', - 'https___valid_icon2_example_org_favicon_ico', - ]; + const expectedClasses = ['favicon-1', 'favicon-2']; const invalidIcon = 'https://invalid.icon.example.org/favicon.ico'; let imageInstances: Image[] = []; @@ -32,6 +29,7 @@ describe('actions/icons', function () { afterEach(() => { delete (window: any).Image; imageInstances = []; + iconsActions._resetIconCounter(); }); function _createCallNodeWithIcon(icon: string): CallNodeDisplayData { From d3b9734586558e0e1e6646b9a839866d4f636264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 21 Oct 2024 15:28:23 +0200 Subject: [PATCH 13/14] Get the binary favicon data directly and batch the icon loading for data urls --- src/actions/icons.js | 31 +++++++++++++++++++++++++++++ src/actions/receive-profile.js | 20 +++++++++++++++++-- src/app-logic/browser-connection.js | 6 +++--- src/app-logic/web-channel.js | 3 ++- src/reducers/icons.js | 8 ++++++++ src/types/actions.js | 3 ++- src/types/profile-derived.js | 8 ++++++++ src/utils/base64.js | 28 ++++++++++++++++++++++++++ 8 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 src/utils/base64.js diff --git a/src/actions/icons.js b/src/actions/icons.js index 9759326f8a..f22730980e 100644 --- a/src/actions/icons.js +++ b/src/actions/icons.js @@ -82,6 +82,37 @@ export function iconStartLoading(icon: string): ThunkAction> { }; } +/** + * Batch load the data url icons. + * + * We don't need to check if they are valid images or not, so we can omit doing + * this extra work for these icons. Just add them directly to our cache and state. + */ +export function batchLoadDataUrlIcons( + iconsToAdd: Array +): ThunkAction { + return (dispatch) => { + const newIcons = []; + for (const icon of iconsToAdd) { + if (!icon || icons.has(icon)) { + continue; + } + + icons.add(icon); + + // New class name for an icon. They are guaranteed to be unique, that's why + // just increment the icon counter and return that string. + const className = `favicon-${++iconCounter}`; + newIcons.push({ icon, className }); + } + + dispatch({ + type: 'ICON_BATCH_ADD', + icons: newIcons, + }); + }; +} + /** * Only use it in tests! */ diff --git a/src/actions/receive-profile.js b/src/actions/receive-profile.js index 7bac82ec74..9b22a918be 100644 --- a/src/actions/receive-profile.js +++ b/src/actions/receive-profile.js @@ -69,6 +69,7 @@ import { import { computeActiveTabTracks } from 'firefox-profiler/profile-logic/active-tab'; import { setDataSource } from './profile-view'; import { fatalError } from './errors'; +import { batchLoadDataUrlIcons } from './icons'; import { GOOGLE_STORAGE_BUCKET } from 'firefox-profiler/app-logic/constants'; import { determineTimelineType, @@ -98,6 +99,7 @@ import type { SymbolicationStepInfo, } from '../profile-logic/symbolication'; import { assertExhaustiveCheck, ensureExists } from '../utils/flow'; +import { bytesToBase64DataUrl } from 'firefox-profiler/utils/base64'; import type { BrowserConnection, BrowserConnectionStatus, @@ -1039,15 +1041,29 @@ export async function retrievePageFaviconsFromBrowser( return; } + // Convert binary favicon data into data urls. + const faviconDataStringPromises: Array> = favicons.map( + (faviconData) => { + if (!faviconData) { + return Promise.resolve(null); + } + return bytesToBase64DataUrl(faviconData.data, faviconData.mimeType); + } + ); + + const faviconDataUrls = await Promise.all(faviconDataStringPromises); + for (let index = 0; index < favicons.length; index++) { - if (favicons[index]) { + if (faviconDataUrls[index]) { newPages[index] = { ...newPages[index], - favicon: favicons[index], + favicon: faviconDataUrls[index], }; } } + // Once we update the pages, we can also start loading the data urls. + dispatch(batchLoadDataUrlIcons(faviconDataUrls)); dispatch({ type: 'UPDATE_PAGES', newPages, diff --git a/src/app-logic/browser-connection.js b/src/app-logic/browser-connection.js index c544291b80..a44e91a3bb 100644 --- a/src/app-logic/browser-connection.js +++ b/src/app-logic/browser-connection.js @@ -13,7 +13,7 @@ import { querySymbolicationApiViaWebChannel, getPageFaviconsViaWebChannel, } from './web-channel'; -import type { Milliseconds } from 'firefox-profiler/types'; +import type { Milliseconds, FaviconData } from 'firefox-profiler/types'; /** * This file manages the communication between the profiler and the browser. @@ -70,7 +70,7 @@ export interface BrowserConnection { breakpadId: string ): Promise; - getPageFavicons(pageUrls: Array): Promise>; + getPageFavicons(pageUrls: Array): Promise>; } /** @@ -189,7 +189,7 @@ class BrowserConnectionImpl implements BrowserConnection { async getPageFavicons( pageUrls: Array - ): Promise> { + ): Promise> { // This is added in Firefox 133. if (this._webChannelSupportsGetPageFavicons) { return getPageFaviconsViaWebChannel(pageUrls); diff --git a/src/app-logic/web-channel.js b/src/app-logic/web-channel.js index 389e5c3058..aca79d7b88 100644 --- a/src/app-logic/web-channel.js +++ b/src/app-logic/web-channel.js @@ -8,6 +8,7 @@ import type { Milliseconds, MixedObject, ExternalMarkersData, + FaviconData, } from 'firefox-profiler/types'; /** @@ -120,7 +121,7 @@ type GetExternalMarkersResponse = ExternalMarkersData; type GetExternalPowerTracksResponse = MixedObject[]; type GetSymbolTableResponse = SymbolTableAsTuple; type QuerySymbolicationApiResponse = string; -type GetPageFaviconsResponse = Array; +type GetPageFaviconsResponse = Array; // Manually declare all pairs of request + response for Flow. /* eslint-disable no-redeclare */ diff --git a/src/reducers/icons.js b/src/reducers/icons.js index 7103171304..1c77ae3c69 100644 --- a/src/reducers/icons.js +++ b/src/reducers/icons.js @@ -11,6 +11,14 @@ const favicons: Reducer = (state = new Map(), action) => { const { icon, className } = action.iconWithClassName; return new Map([...state.entries(), [icon, className]]); } + case 'ICON_BATCH_ADD': { + const newState = new Map([...state.entries()]); + for (const { icon, className } of action.icons) { + newState.set(icon, className); + } + + return newState; + } case 'ICON_IN_ERROR': // nothing to do default: return state; diff --git a/src/types/actions.js b/src/types/actions.js index e4788fe43d..99ca82a98f 100644 --- a/src/types/actions.js +++ b/src/types/actions.js @@ -582,7 +582,8 @@ type IconsAction = +type: 'ICON_HAS_LOADED', +iconWithClassName: IconWithClassName, |} - | {| +type: 'ICON_IN_ERROR', +icon: string |}; + | {| +type: 'ICON_IN_ERROR', +icon: string |} + | {| +type: 'ICON_BATCH_ADD', icons: IconWithClassName[] |}; type SidebarAction = {| +type: 'CHANGE_SIDEBAR_OPEN_STATE', diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index 92ca20f7cc..df6e9fb524 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -740,3 +740,11 @@ export type BottomBoxInfo = {| sourceFile: string | null, nativeSymbols: NativeSymbolInfo[], |}; + +/** + * Favicon data that is retrieved from the browser connection. + */ +export type FaviconData = {| + +data: Uint8Array, + +mimeType: string, +|}; diff --git a/src/utils/base64.js b/src/utils/base64.js new file mode 100644 index 0000000000..484e7b2007 --- /dev/null +++ b/src/utils/base64.js @@ -0,0 +1,28 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +// @flow + +/** + * Encode the bytes Uint8Array into a base64 data url. + */ +export async function bytesToBase64DataUrl( + bytes: Uint8Array, + type: string = 'application/octet-stream' +): Promise { + return new Promise((resolve, reject) => { + const reader = Object.assign(new FileReader(), { + onload: () => resolve((reader.result: any)), + onerror: () => reject(reader.error), + }); + reader.readAsDataURL(new File([bytes], '', { type })); + }); +} + +/** + * Decode the encoded base64 data url into bytes array. + */ +export async function dataUrlToBytes(dataUrl: string): Promise { + const res = await fetch(dataUrl); + return new Uint8Array(await res.arrayBuffer()); +} From 269d3ffa01db7272533bfe72337f816e3ac57abd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 22 Oct 2024 00:56:25 +0200 Subject: [PATCH 14/14] Remove the unneeded image mock --- src/test/components/TabSelectorMenu.test.js | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/test/components/TabSelectorMenu.test.js b/src/test/components/TabSelectorMenu.test.js index ea1501f5d9..43b533e845 100644 --- a/src/test/components/TabSelectorMenu.test.js +++ b/src/test/components/TabSelectorMenu.test.js @@ -18,22 +18,9 @@ import { import { storeWithProfile } from '../fixtures/stores'; import { fireFullClick } from '../fixtures/utils'; import { getTabFilter } from '../../selectors/url-state'; -import { createImageMock } from '../fixtures/mocks/image'; import { ensureExists } from 'firefox-profiler/utils/flow'; describe('app/TabSelectorMenu', () => { - beforeAll(() => { - const mock = createImageMock(); - (window: any).Image = mock.Image; - }); - - afterAll(() => { - // Let all the async favicons finish creating the images. - requestAnimationFrame(() => { - delete (window: any).Image; - }); - }); - function setup() { const { profile, ...extraPageData } = addActiveTabInformationToProfile( getProfileWithNiceTracks()