From d426f2fd4e44b0a132c78a834b5952c34bc8856e Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 4 Oct 2023 11:18:06 -0700 Subject: [PATCH] fix(chromium): continue requests paused for the second time (#27429) Sometimes Chromium restarts requests. This leads to multiple `Fetch.requestPaused` for a single `Network.requestWillBeSent`. Fixes #27294. --- .../src/server/chromium/crNetworkManager.ts | 54 +++++++++++------- .../server/webkit/wkInterceptableRequest.ts | 7 --- .../ct-react-vite/playwright.config.ts | 3 + .../ct-react-vite/src/assets/iconfont.woff2 | Bin 0 -> 2656 bytes .../ct-react-vite/src/assets/index.css | 9 +++ .../src/components/TitleWithFont.css | 3 + .../src/components/TitleWithFont.tsx | 5 ++ .../ct-react-vite/tests/route.spec.tsx | 22 +++++++ 8 files changed, 75 insertions(+), 28 deletions(-) create mode 100644 tests/components/ct-react-vite/src/assets/iconfont.woff2 create mode 100644 tests/components/ct-react-vite/src/components/TitleWithFont.css create mode 100644 tests/components/ct-react-vite/src/components/TitleWithFont.tsx create mode 100644 tests/components/ct-react-vite/tests/route.spec.tsx diff --git a/packages/playwright-core/src/server/chromium/crNetworkManager.ts b/packages/playwright-core/src/server/chromium/crNetworkManager.ts index 6d731cdbe7528..197c1bf5082b9 100644 --- a/packages/playwright-core/src/server/chromium/crNetworkManager.ts +++ b/packages/playwright-core/src/server/chromium/crNetworkManager.ts @@ -206,6 +206,21 @@ export class CRNetworkManager { this._onRequest(sessionInfo, requestWillBeSentEvent, event); this._requestIdToRequestWillBeSentEvent.delete(requestId); } else { + const existingRequest = this._requestIdToRequest.get(requestId); + const alreadyContinuedParams = existingRequest?._route?._alreadyContinuedParams; + if (alreadyContinuedParams) { + // Sometimes Chromium network stack restarts the request internally. + // For example, when no-cors request hits a "less public address space", it should be resent with cors. + // There are some more examples here: https://source.chromium.org/chromium/chromium/src/+/main:services/network/url_loader.cc;l=1205-1234;drc=d5dd931e0ad3d9ffe74888ec62a3cc106efd7ea6 + // There are probably even more cases deep inside the network stack. + // + // Anyway, in this case, continue the request in the same way as before, and it should go through. + this._session._sendMayFail('Fetch.continueRequest', { + ...alreadyContinuedParams, + requestId: event.requestId, + }); + return; + } this._requestIdToRequestPausedEvent.set(requestId, event); } } @@ -368,14 +383,20 @@ export class CRNetworkManager { return response; } + _deleteRequest(request: InterceptableRequest) { + this._requestIdToRequest.delete(request._requestId); + if (request._route) + request._route._alreadyContinuedParams = undefined; + if (request._interceptionId) + this._attemptedAuthentications.delete(request._interceptionId); + } + _handleRequestRedirect(request: InterceptableRequest, responsePayload: Protocol.Network.Response, timestamp: number, hasExtraInfo: boolean) { const response = this._createResponse(request, responsePayload, hasExtraInfo); response.setTransferSize(null); response.setEncodedBodySize(null); response._requestFinished((timestamp - request._timestamp) * 1000); - this._requestIdToRequest.delete(request._requestId); - if (request._interceptionId) - this._attemptedAuthentications.delete(request._interceptionId); + this._deleteRequest(request); (this._page?._frameManager || this._serviceWorker)!.requestReceivedResponse(response); (this._page?._frameManager || this._serviceWorker)!.reportRequestFinished(request.request, response); } @@ -423,9 +444,7 @@ export class CRNetworkManager { response.responseHeadersSize().then(size => response.setEncodedBodySize(event.encodedDataLength - size)); response._requestFinished(helper.secondsToRoundishMillis(event.timestamp - request._timestamp)); } - this._requestIdToRequest.delete(request._requestId); - if (request._interceptionId) - this._attemptedAuthentications.delete(request._interceptionId); + this._deleteRequest(request); (this._page?._frameManager || this._serviceWorker)!.reportRequestFinished(request.request, response); } @@ -458,9 +477,7 @@ export class CRNetworkManager { response.setEncodedBodySize(null); response._requestFinished(helper.secondsToRoundishMillis(event.timestamp - request._timestamp)); } - this._requestIdToRequest.delete(request._requestId); - if (request._interceptionId) - this._attemptedAuthentications.delete(request._interceptionId); + this._deleteRequest(request); request.request._setFailureText(event.errorText); (this._page?._frameManager || this._serviceWorker)!.requestFailed(request.request, !!event.canceled); } @@ -491,7 +508,7 @@ class InterceptableRequest { readonly _documentId: string | undefined; readonly _timestamp: number; readonly _wallTime: number; - private _route: RouteImpl | null; + readonly _route: RouteImpl | null; private _redirectedFrom: InterceptableRequest | null; session: CRSession; @@ -529,18 +546,12 @@ class InterceptableRequest { this.request = new network.Request(context, frame, serviceWorker, redirectedFrom?.request || null, documentId, url, type, method, postDataBuffer, headersObjectToArray(headers)); } - - _routeForRedirectChain(): RouteImpl | null { - let request: InterceptableRequest = this; - while (request._redirectedFrom) - request = request._redirectedFrom; - return request._route; - } } class RouteImpl implements network.RouteDelegate { private readonly _session: CRSession; private _interceptionId: string; + _alreadyContinuedParams: Protocol.Fetch.continueRequestParameters | undefined; constructor(session: CRSession, interceptionId: string) { this._session = session; @@ -548,15 +559,16 @@ class RouteImpl implements network.RouteDelegate { } async continue(request: network.Request, overrides: types.NormalizedContinueOverrides): Promise { - // In certain cases, protocol will return error if the request was already canceled - // or the page was closed. We should tolerate these errors. - await this._session._sendMayFail('Fetch.continueRequest', { + this._alreadyContinuedParams = { requestId: this._interceptionId!, url: overrides.url, headers: overrides.headers, method: overrides.method, postData: overrides.postData ? overrides.postData.toString('base64') : undefined - }); + }; + // In certain cases, protocol will return error if the request was already canceled + // or the page was closed. We should tolerate these errors. + await this._session._sendMayFail('Fetch.continueRequest', this._alreadyContinuedParams); } async fulfill(response: types.NormalizedFulfillResponse) { diff --git a/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts b/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts index 390d33423d02b..38ed0c4bcdb8c 100644 --- a/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts +++ b/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts @@ -64,13 +64,6 @@ export class WKInterceptableRequest { resourceType, event.request.method, postDataBuffer, headersObjectToArray(event.request.headers)); } - _routeForRedirectChain(): WKRouteImpl | null { - let request: WKInterceptableRequest = this; - while (request._redirectedFrom) - request = request._redirectedFrom; - return request._route; - } - createResponse(responsePayload: Protocol.Network.Response): network.Response { const getResponseBody = async () => { const response = await this._session.send('Network.getResponseBody', { requestId: this._requestId }); diff --git a/tests/components/ct-react-vite/playwright.config.ts b/tests/components/ct-react-vite/playwright.config.ts index bb45af7150ffc..63d909ae2717b 100644 --- a/tests/components/ct-react-vite/playwright.config.ts +++ b/tests/components/ct-react-vite/playwright.config.ts @@ -25,6 +25,9 @@ export default defineConfig({ use: { trace: 'on-first-retry', ctViteConfig: { + build: { + assetsInlineLimit: 0, + }, resolve: { alias: { '@': resolve(__dirname, './src'), diff --git a/tests/components/ct-react-vite/src/assets/iconfont.woff2 b/tests/components/ct-react-vite/src/assets/iconfont.woff2 new file mode 100644 index 0000000000000000000000000000000000000000..ceba03549a59bd18dea3d36df96a2438540c9c50 GIT binary patch literal 2656 zcmV-m3ZM0NPew8T0RR910199L4*&oF02hz|0162J0RR9100000000000000000000 z0000SR0d!Gg9Zo=37iZO2nvJ%gb51>00A}vBm)ctAO(d@2R00W92eRJdEd>ugcKqIOq} z9QL2)O8eg=DUndb!lN*CZf>%(-9O18f+~SW1xS*>wMGE>6S7LP^*x4L9>c#?8co@( zWFJvKBm#J8Wf&D*v|Mz#U<$-c`@ZLo86I)#t$7 z;g^@cX%htsOB0X#n(2j;d`*CmUN=9>7X2pA0g+%uU_m2DCSG=kImYk==V-jK7{WH* zNKga_&CkQops~i|Oo7!*O=DJ`<}{5KPK&#@o= zN^~yM_*OO=H@>30{&CBYwAc2#Z1P6k+tw1zb$FyJk*TDoVBO?7Ac~|9I&8;FkT1Y;+uraB5sS$`;4T3UDuLQENQM* z6i1cScgL=ZBuGv+R?~Cd3^h9csSsOhzhkPmsl0LZT~3yL7LkX9MAszxIIb3O0S_-s z^t3U#-?+!32`pArYlF0rgsU0wKw5|_u74Y832qaSl=-n`uIY`um^>50MLh3~hJ4+P z$j{ipy(JnD<3)?npEcBU49l%iiL*?kwvx~;^yk|jOXj-K)NTNJrIe*g^G?2Hj!Ey) zo8+DOeBKAL(Nxb7lX)J<0g@!L`3!E8$hMftnZrGDvIDsoyESrC{Wj7b{h3@Ajk|Ox zxnvKMhvy$sGo5)pNoLo!0{KgMZ`1SlPPFrmmA`ZW8Z)zxp}gYuNRH&j3`BWs*%Uc9 zM@kdg-=rqOf*~p6ZQW5$j88a+oJ5_uN2Aj=||1}UoiDp}Yr)7$oJi!oN73LYBCtzal}CoZ4Jfg&`D3xVRH z5GoOhN`X?e@Lbd`MRL&7zlDE7TLe6hbGqB ze@D&1HnL+C^N{?*QP8bwsDa`IDEv^=LQw}rJroU4G(yn?MKd%v4E4rJE!7JpNi&=E zX6>0HiEwuE#^WRBI_cB$TXp>Y+b_JEz!vIdSi+m&c*HY*g1bU9bh z@2H!;i*q}n8(OD#VH=EcbhzCkQ4htsE#-Fvh(55}tuqwWJSyE#ua)jeK>UURJ?IYf zfnK?6glpON+J~m3Jf%)6u!!=f-Tnds`MDOYkmsy|XS2~*j!-gbtUOOy}X18la48Ur>amti)R*Q`g0 z6ijsl!dVykO0>&i3UXku8CH>mRV7(}d-cSjSiGXp64V~KdJ~4-k^~z0hVYm=_2wOF zg^^C-h8u1B+_0y2TLLZEYe&~7%9y!Fj6|y8gZ=>vPTIJ^_AQCP)HUEZFDYhvy@)+okd#f4+195ab)xEZG?3JsA&5l-OS{O^&HT|m5 z%PJWb%dL#FzNuGdw|D&=UxeoE^9ZFm{}ZbYtg54~!uIX$w)0&j-e(*k5EhujHSQ9O<{t^BMNCH>v5q-H#}gUa zAx_vsIB5_0rzRMs(-1^(Msw+`BXlm2;a%apJ&X(XFfQ7|xTJwDJ3?0+;j1ul1lQ~l zT(?JX!yds+4Rp&9y6s5ro#4B^JGE6(cKN$Tjqe}x*Ob@9KtkD&P&@|)3rPpu23SMvS#n*g3=H7Wj#3xr(@p`x1-tcQU z>*B`WX5HER$E-`6evdhLX=3iIFc*2|Hmq~&sC(A+;@kDI_I0LtS(&+IxtWh24=!5u z`q|mxiT7G*VfKhtaEGEpQ#+kiJTqzPiL=}C%4ON^_{N$ z#om}ZI?r8Pzke41^H24cx)EP(WC^&%ja)=4kJ6rtUa@gwyJsvBBT9=(;&Hh=aXd6C z0e)bvwUreMB0b#=^%bp^t$u&L-=F8RD*}-oTNGG|87!9=u}l)tTH=W41X}dkKyy)H zTe!I^(CJhcHWf9M7P{?{00Id7|6(z*GkfqKceb7b_-jq=J478V%`r3&1I9lk;26gYnBsb1I~&?7CtsC zvT?I>k(U6;d;7@eF&Z%*V3wD9i-a7>UZlv9YS!R{*nl&%$f8JAvJK9}A}^cd$UZ*m z<(Gf*oif(s{OR3|Pe$t(WliL5(CUeK^@L>*^cR%}o#0SEOPSFYZe8s5rn8zyjwjoX z9-V%!F?^9Q_+2#m8J=#3dThoxz(G!Nm>H7n<|q!0jM0QcJFQu9I}YO{-Qj@3ooCoj zg1tB-IL1+SGx-SN2;phu{S2P6Bo-ea%tSHrJ{GUDl9L>>cA4O?pyzqsG*-L!W>|;s z&_x{UrU*L OX|-JQJ!{e?8U+Ay83p73 literal 0 HcmV?d00001 diff --git a/tests/components/ct-react-vite/src/assets/index.css b/tests/components/ct-react-vite/src/assets/index.css index 97495c44b8634..dd2a6fa726c4a 100644 --- a/tests/components/ct-react-vite/src/assets/index.css +++ b/tests/components/ct-react-vite/src/assets/index.css @@ -18,3 +18,12 @@ code { background-color: #1b1b1d; } } + +@font-face { + font-family: 'pwtest-iconfont'; + /* See tests/assets/webfont/README.md */ + src: url('./iconfont.woff2') format('woff2'); + font-weight: normal; + font-style: normal; + font-display: swap; +} diff --git a/tests/components/ct-react-vite/src/components/TitleWithFont.css b/tests/components/ct-react-vite/src/components/TitleWithFont.css new file mode 100644 index 0000000000000..9fc7e0a7c056e --- /dev/null +++ b/tests/components/ct-react-vite/src/components/TitleWithFont.css @@ -0,0 +1,3 @@ +.title-with-font { + font-family: pwtest-iconfont, sans-serif; +} diff --git a/tests/components/ct-react-vite/src/components/TitleWithFont.tsx b/tests/components/ct-react-vite/src/components/TitleWithFont.tsx new file mode 100644 index 0000000000000..ad783205c3c19 --- /dev/null +++ b/tests/components/ct-react-vite/src/components/TitleWithFont.tsx @@ -0,0 +1,5 @@ +import './TitleWithFont.css'; + +export default function TitleWithFont() { + return
+-
+} diff --git a/tests/components/ct-react-vite/tests/route.spec.tsx b/tests/components/ct-react-vite/tests/route.spec.tsx new file mode 100644 index 0000000000000..657b0a70073a9 --- /dev/null +++ b/tests/components/ct-react-vite/tests/route.spec.tsx @@ -0,0 +1,22 @@ +import { test, expect } from '@playwright/experimental-ct-react'; +import TitleWithFont from '@/components/TitleWithFont'; + +test('should load font without routes', async ({ mount, page }) => { + const promise = page.waitForEvent('requestfinished', request => request.url().includes('iconfont')); + await mount(); + const request = await promise; + const response = await request.response(); + const body = await response!.body(); + expect(body.length).toBe(2656); +}); + +test('should load font with routes', async ({ mount, page }) => { + test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/27294' }); + await page.route('**/*.json', r => r.continue()); + const promise = page.waitForEvent('requestfinished', request => request.url().includes('iconfont')); + await mount(); + const request = await promise; + const response = await request.response(); + const body = await response!.body(); + expect(body.length).toBe(2656); +});