core(driver): fix protocol timeout being ignored for isolated eval (#…

…15826)

core/gather/driver/execution-context.js

@@ -80,15 +80,10 @@ class ExecutionContext {
    * page without isolation.
    * @param {string} expression
    * @param {number|undefined} contextId
+   * @param {number} timeout
    * @return {Promise<*>}
    */
-  async _evaluateInContext(expression, contextId) {
-    // Use a higher than default timeout if the user hasn't specified a specific timeout.
-    // Otherwise, use whatever was requested.
-    const timeout = this._session.hasNextProtocolTimeout() ?
-      this._session.getNextProtocolTimeout() :
-      60000;
-
+  async _evaluateInContext(expression, contextId, timeout) {
     // `__lighthouseExecutionContextUniqueIdentifier` is only used by the FullPageScreenshot gatherer.
     // See `getNodeDetails` in page-functions.
     const uniqueExecutionContextIdentifier = contextId === undefined ?
@@ -166,17 +161,22 @@ class ExecutionContext {
    * @return {Promise<*>}
    */
   async evaluateAsync(expression, options = {}) {
+    // Use a higher than default timeout if the user hasn't specified a specific timeout.
+    // Otherwise, use whatever was requested.
+    const timeout = this._session.hasNextProtocolTimeout() ?
+      this._session.getNextProtocolTimeout() :
+      60000;
     const contextId = options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined;
 
     try {
       // `await` is not redundant here because we want to `catch` the async errors
-      return await this._evaluateInContext(expression, contextId);
+      return await this._evaluateInContext(expression, contextId, timeout);
     } catch (err) {
       // If we were using isolation and the context disappeared on us, retry one more time.
       if (contextId && err.message.includes('Cannot find context')) {
         this.clearContextId();
         const freshContextId = await this._getOrCreateIsolatedContextId();
-        return this._evaluateInContext(expression, freshContextId);
+        return this._evaluateInContext(expression, freshContextId, timeout);
       }
 
       throw err;

core/test/gather/driver/execution-context-test.js

@@ -120,6 +120,27 @@ describe('.evaluateAsync', () => {
     await expect(evaluatePromise).rejects.toBeTruthy();
   });
 
+  it('uses the specific timeout given (isolation)', async () => {
+    const expectedTimeout = 5000;
+    const setNextProtocolTimeout = sessionMock.setNextProtocolTimeout = fnAny();
+    sessionMock.hasNextProtocolTimeout = fnAny().mockReturnValue(true);
+    sessionMock.getNextProtocolTimeout = fnAny().mockReturnValue(expectedTimeout);
+    sessionMock.sendCommand
+      .mockResponse('Page.enable')
+      .mockResponse('Runtime.enable')
+      .mockResponse('Page.getFrameTree', {frameTree: {frame: {id: '1337'}}})
+      .mockResponse('Page.createIsolatedWorld', {executionContextId: 1});
+
+    const evaluatePromise = makePromiseInspectable(executionContext.evaluateAsync('1 + 1', {
+      useIsolation: true,
+    }));
+
+    await flushAllTimersAndMicrotasks();
+    expect(setNextProtocolTimeout).toHaveBeenCalledWith(expectedTimeout);
+    expect(evaluatePromise).toBeDone();
+    await expect(evaluatePromise).rejects.toBeTruthy();
+  });
+
   it('evaluates an expression in isolation', async () => {
     sessionMock.sendCommand
       .mockResponse('Page.enable')