fix(route): correctly remove expired handlers (#28385)

* Check if handler is still in the route list before calling it
* Check if the handler is still in the list before removing it after
`times` expiration

packages/playwright-core/src/client/browserContext.ts

@@ -197,8 +197,11 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
     for (const routeHandler of routeHandlers) {
       if (!routeHandler.matches(route.request().url()))
         continue;
+      const index = this._routes.indexOf(routeHandler);
+      if (index === -1)
+        continue;
       if (routeHandler.willExpire())
-        this._routes.splice(this._routes.indexOf(routeHandler), 1);
+        this._routes.splice(index, 1);
       const handled = await routeHandler.handle(route);
       if (!this._routes.length)
         this._wrapApiCall(() => this._updateInterceptionPatterns(), true).catch(() => {});

packages/playwright-core/src/client/page.ts

@@ -177,8 +177,11 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     for (const routeHandler of routeHandlers) {
       if (!routeHandler.matches(route.request().url()))
         continue;
+      const index = this._routes.indexOf(routeHandler);
+      if (index === -1)
+        continue;
       if (routeHandler.willExpire())
-        this._routes.splice(this._routes.indexOf(routeHandler), 1);
+        this._routes.splice(index, 1);
       const handled = await routeHandler.handle(route);
       if (!this._routes.length)
         this._wrapApiCall(() => this._updateInterceptionPatterns(), true).catch(() => {});

tests/library/browsercontext-route.spec.ts

@@ -208,6 +208,25 @@ it('should support the times parameter with route matching', async ({ context, p
   expect(intercepted).toHaveLength(1);
 });
 
+it('should work if handler with times parameter was removed from another handler', async ({ context, page, server }) => {
+  const intercepted = [];
+  const handler = async route => {
+    intercepted.push('first');
+    void route.continue();
+  };
+  await context.route('**/*', handler, { times: 1 });
+  await context.route('**/*', async route => {
+    intercepted.push('second');
+    await context.unroute('**/*', handler);
+    await route.fallback();
+  });
+  await page.goto(server.EMPTY_PAGE);
+  expect(intercepted).toEqual(['second']);
+  intercepted.length = 0;
+  await page.goto(server.EMPTY_PAGE);
+  expect(intercepted).toEqual(['second']);
+});
+
 it('should support async handler w/ times', async ({ context, page, server }) => {
   await context.route('**/empty.html', async route => {
     await new Promise(f => setTimeout(f, 100));

tests/page/page-route.spec.ts

@@ -902,6 +902,25 @@ it('should support the times parameter with route matching', async ({ page, serv
   expect(intercepted).toHaveLength(1);
 });
 
+it('should work if handler with times parameter was removed from another handler', async ({ page, server }) => {
+  const intercepted = [];
+  const handler = async route => {
+    intercepted.push('first');
+    void route.continue();
+  };
+  await page.route('**/*', handler, { times: 1 });
+  await page.route('**/*', async route => {
+    intercepted.push('second');
+    await page.unroute('**/*', handler);
+    await route.fallback();
+  });
+  await page.goto(server.EMPTY_PAGE);
+  expect(intercepted).toEqual(['second']);
+  intercepted.length = 0;
+  await page.goto(server.EMPTY_PAGE);
+  expect(intercepted).toEqual(['second']);
+});
+
 it('should support async handler w/ times', async ({ page, server }) => {
   await page.route('**/empty.html', async route => {
     await new Promise(f => setTimeout(f, 100));