core: use target type instead of session for oopif (#15006)

GoogleChrome · May 2, 2023 · 6603f36 · 6603f36
1 parent be9d2bb
commit 6603f36
Show file tree

Hide file tree

Showing 28 changed files with 1,024 additions and 853 deletions.
diff --git a/cli/test/fixtures/oopif-scripts-timespan.html b/cli/test/fixtures/oopif-scripts-timespan.html
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+  <body style="height: 200vh">
+    <h1>Hello frames</h1>
+    <iframe id="iframe-1" src="http://localhost:10200/oopif-simple-page.html?defer" style="position: fixed"></iframe>
+    <iframe id="iframe-2" src="http://localhost:10503/oopif-simple-page.html?defer" style="position: fixed; top: 50vh"></iframe>
+  </body>
+</html>
diff --git a/cli/test/fixtures/oopif-simple-page.html b/cli/test/fixtures/oopif-simple-page.html
@@ -5,16 +5,27 @@
   <meta http-equiv="X-UA-Compatible" content="IE=edge">
   <meta name="viewport" content="width=device-width, initial-scale=1.0">
   <script src="simple-script.js"></script>
-  <script>
-    new Worker('simple-worker.mjs', { type: 'module' });
-    new Worker('simple-worker.js');
-
-    window.addEventListener('load', () => {
-      fetch('simple-script.js').then(r => r.text()).then(eval);
-    });
-  </script>
 </head>
 <body>
   <h1>Just a simple page</h1>
+  <script>
+    const shouldDeferStuff = location.search === '?defer';
+    if (shouldDeferStuff) {
+      const el = document.createElement('button');
+      el.textContent = 'Click to load scripts';
+      el.addEventListener('click', () => {
+        new Worker('simple-worker.mjs', { type: 'module' });
+        new Worker('simple-worker.js');
+        fetch('simple-script.js').then(r => r.text()).then(eval);
+      });
+      document.body.append(el);
+    } else {
+      new Worker('simple-worker.mjs', { type: 'module' });
+      new Worker('simple-worker.js');
+      window.addEventListener('load', () => {
+        fetch('simple-script.js').then(r => r.text()).then(eval);
+      });
+    }
+  </script>
 </body>
 </html>
diff --git a/core/audits/network-requests.js b/core/audits/network-requests.js
@@ -63,6 +63,7 @@ class NetworkRequests extends Audit {
 
       return {
         url: UrlUtils.elideDataURI(record.url),
+        sessionTargetType: record.sessionTargetType,
         protocol: record.protocol,
         rendererStartTime: normalizeTime(record.rendererStartTime),
         networkRequestTime: normalizeTime(record.networkRequestTime),

diff --git a/core/gather/driver/target-manager.js b/core/gather/driver/target-manager.js
@@ -119,7 +119,7 @@ class TargetManager extends ProtocolEventEmitter {
       const targetName = target.targetInfo.url || target.targetInfo.targetId;
       log.verbose('target-manager', `target ${targetName} attached`);
 
-      const trueProtocolListener = this._getProtocolEventListener(newSession.id());
+      const trueProtocolListener = this._getProtocolEventListener(targetType, newSession.id());
       /** @type {(event: unknown) => void} */
       // @ts-expect-error - pptr currently typed only for single arg emits.
       const protocolListener = trueProtocolListener;
@@ -156,17 +156,19 @@ class TargetManager extends ProtocolEventEmitter {
   /**
    * Returns a listener for all protocol events from session, and augments the
    * event with the sessionId.
+   * @param {LH.Protocol.TargetType} targetType
    * @param {string} sessionId
    */
-  _getProtocolEventListener(sessionId) {
+  _getProtocolEventListener(targetType, sessionId) {
     /**
      * @template {keyof LH.Protocol.RawEventMessageRecord} EventName
      * @param {EventName} method
      * @param {LH.Protocol.RawEventMessageRecord[EventName]['params']} params
      */
     const onProtocolEvent = (method, params) => {
       // Cast because tsc 4.7 still can't quite track the dependent parameters.
-      const payload = /** @type {LH.Protocol.RawEventMessage} */ ({method, params, sessionId});
+      const payload = /** @type {LH.Protocol.RawEventMessage} */ (
+        {method, params, targetType, sessionId});
       this.emit('protocolevent', payload);
     };
 

diff --git a/core/gather/gatherers/dobetterweb/optimized-images.js b/core/gather/gatherers/dobetterweb/optimized-images.js
@@ -54,8 +54,8 @@ class OptimizedImages extends FRGatherer {
     /** @type {Set<string>} */
     const seenUrls = new Set();
     return networkRecords.reduce((prev, record) => {
-      // Skip records that we've seen before, never finished, or came from child targets (OOPIFS).
-      if (seenUrls.has(record.url) || !record.finished || record.sessionId) {
+      // Skip records that we've seen before, never finished, or came from OOPIFs.
+      if (seenUrls.has(record.url) || !record.finished || record.isOutOfProcessIframe) {
         return prev;
       }
 

diff --git a/core/gather/gatherers/dobetterweb/response-compression.js b/core/gather/gatherers/dobetterweb/response-compression.js
@@ -55,8 +55,7 @@ class ResponseCompression extends FRGatherer {
     const unoptimizedResponses = [];
 
     networkRecords.forEach(record => {
-      // Ignore records from child targets (OOPIFS).
-      if (record.sessionId) return;
+      if (record.isOutOfProcessIframe) return;
 
       const mimeType = record.mimeType;
       const resourceType = record.resourceType || NetworkRequest.TYPES.Other;

diff --git a/core/gather/gatherers/script-elements.js b/core/gather/gatherers/script-elements.js
@@ -65,8 +65,7 @@ class ScriptElements extends FRGatherer {
 
     const scriptRecords = networkRecords
       .filter(record => record.resourceType === NetworkRequest.TYPES.Script)
-      // Ignore records from OOPIFs
-      .filter(record => !record.sessionId);
+      .filter(record => !record.isOutOfProcessIframe);
 
     for (let i = 0; i < scriptRecords.length; i++) {
       const record = scriptRecords[i];

diff --git a/core/gather/gatherers/scripts.js b/core/gather/gatherers/scripts.js
@@ -59,9 +59,6 @@ class Scripts extends FRGatherer {
   /** @type {Array<string | undefined>} */
   _scriptContents = [];
 
-  /** @type {string|null|undefined} */
-  _mainSessionId = null;
-
   constructor() {
     super();
     this.onScriptParsed = this.onScriptParsed.bind(this);

diff --git a/core/legacy/gather/connections/connection.js b/core/legacy/gather/connections/connection.js
@@ -25,6 +25,8 @@ class Connection {
     this._lastCommandId = 0;
     /** @type {Map<number, CommandCallback>} */
     this._callbacks = new Map();
+    /** @type {Map<string, LH.Protocol.TargetType>} */
+    this._sessionIdToTargetType = new Map();
 
     this._eventEmitter = /** @type {?CrdpEventMessageEmitter} */ (new EventEmitter());
   }
@@ -129,6 +131,18 @@ class Connection {
     if (!('id' in object)) {
       log.formatProtocol('<= event',
           {method: object.method, params: object.params}, 'verbose');
+      if (object.method === 'Target.attachedToTarget') {
+        const type = object.params.targetInfo.type;
+        if (type === 'page' || type === 'iframe') {
+          this._sessionIdToTargetType.set(object.params.sessionId, type);
+        }
+      }
+      if (object.sessionId) {
+        const type = this._sessionIdToTargetType.get(object.sessionId);
+        if (type) {
+          object.targetType = type;
+        }
+      }
       this.emitProtocolEvent(object);
       return;
     }
@@ -175,6 +189,7 @@ class Connection {
       this._eventEmitter.removeAllListeners();
       this._eventEmitter = null;
     }
+    this._sessionIdToTargetType.clear();
   }
 }
 

diff --git a/core/lib/network-recorder.js b/core/lib/network-recorder.js
@@ -31,8 +31,6 @@ class NetworkRecorder extends RequestEventEmitter {
     this._records = [];
     /** @type {Map<string, NetworkRequest>} */
     this._recordsById = new Map();
-    /** @type {string|null|undefined} */
-    this._mainSessionId = null;
   }
 
   /**
@@ -71,11 +69,12 @@ class NetworkRecorder extends RequestEventEmitter {
   // DevTools SDK network layer.
 
   /**
-   * @param {{params: LH.Crdp.Network.RequestWillBeSentEvent, sessionId?: string}} event
+   * @param {{params: LH.Crdp.Network.RequestWillBeSentEvent, targetType: LH.Protocol.TargetType, sessionId?: string}} event
    */
   onRequestWillBeSent(event) {
     const data = event.params;
-    const originalRequest = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
+    const originalRequest = this._findRealRequestAndSetSession(
+      data.requestId, event.targetType, event.sessionId);
     // This is a simple new request, create the NetworkRequest object and finish.
     if (!originalRequest) {
       const request = new NetworkRequest();
@@ -114,68 +113,74 @@ class NetworkRecorder extends RequestEventEmitter {
   }
 
   /**
-   * @param {{params: LH.Crdp.Network.RequestServedFromCacheEvent, sessionId?: string}} event
+   * @param {{params: LH.Crdp.Network.RequestServedFromCacheEvent, targetType: LH.Protocol.TargetType, sessionId?: string}} event
    */
   onRequestServedFromCache(event) {
     const data = event.params;
-    const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
+    const request = this._findRealRequestAndSetSession(
+      data.requestId, event.targetType, event.sessionId);
     if (!request) return;
     log.verbose('network', `${request.url} served from cache`);
     request.onRequestServedFromCache();
   }
 
   /**
-   * @param {{params: LH.Crdp.Network.ResponseReceivedEvent, sessionId?: string}} event
+   * @param {{params: LH.Crdp.Network.ResponseReceivedEvent, targetType: LH.Protocol.TargetType, sessionId?: string}} event
    */
   onResponseReceived(event) {
     const data = event.params;
-    const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
+    const request = this._findRealRequestAndSetSession(
+      data.requestId, event.targetType, event.sessionId);
     if (!request) return;
     log.verbose('network', `${request.url} response received`);
     request.onResponseReceived(data);
   }
 
   /**
-   * @param {{params: LH.Crdp.Network.DataReceivedEvent, sessionId?: string}} event
+   * @param {{params: LH.Crdp.Network.DataReceivedEvent, targetType: LH.Protocol.TargetType, sessionId?: string}} event
    */
   onDataReceived(event) {
     const data = event.params;
-    const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
+    const request = this._findRealRequestAndSetSession(
+      data.requestId, event.targetType, event.sessionId);
     if (!request) return;
     log.verbose('network', `${request.url} data received`);
     request.onDataReceived(data);
   }
 
   /**
-   * @param {{params: LH.Crdp.Network.LoadingFinishedEvent, sessionId?: string}} event
+   * @param {{params: LH.Crdp.Network.LoadingFinishedEvent, targetType: LH.Protocol.TargetType, sessionId?: string}} event
    */
   onLoadingFinished(event) {
     const data = event.params;
-    const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
+    const request = this._findRealRequestAndSetSession(
+      data.requestId, event.targetType, event.sessionId);
     if (!request) return;
     log.verbose('network', `${request.url} loading finished`);
     request.onLoadingFinished(data);
     this.onRequestFinished(request);
   }
 
   /**
-   * @param {{params: LH.Crdp.Network.LoadingFailedEvent, sessionId?: string}} event
+   * @param {{params: LH.Crdp.Network.LoadingFailedEvent, targetType: LH.Protocol.TargetType, sessionId?: string}} event
    */
   onLoadingFailed(event) {
     const data = event.params;
-    const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
+    const request = this._findRealRequestAndSetSession(
+      data.requestId, event.targetType, event.sessionId);
     if (!request) return;
     log.verbose('network', `${request.url} loading failed`);
     request.onLoadingFailed(data);
     this.onRequestFinished(request);
   }
 
   /**
-   * @param {{params: LH.Crdp.Network.ResourceChangedPriorityEvent, sessionId?: string}} event
+   * @param {{params: LH.Crdp.Network.ResourceChangedPriorityEvent, targetType: LH.Protocol.TargetType, sessionId?: string}} event
    */
   onResourceChangedPriority(event) {
     const data = event.params;
-    const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
+    const request = this._findRealRequestAndSetSession(
+      data.requestId, event.targetType, event.sessionId);
     if (!request) return;
     request.onResourceChangedPriority(data);
   }
@@ -204,24 +209,11 @@ class NetworkRecorder extends RequestEventEmitter {
    * message is referring.
    *
    * @param {string} requestId
+   * @param {LH.Protocol.TargetType} targetType
    * @param {string|undefined} sessionId
    * @return {NetworkRequest|undefined}
    */
-  _findRealRequestAndSetSession(requestId, sessionId) {
-    // The very first sessionId processed is always the main sessionId. In all but DevTools,
-    // this sessionId is undefined. However, in DevTools the main Lighthouse protocol connection
-    // does send events with sessionId set to a string, because of how DevTools routes the protocol
-    // to Lighthouse.
-    // Many places in Lighthouse use `record.sessionId === undefined` to mean that the session is not
-    // an OOPIF. To maintain this property, we intercept sessionId here and set it to undefined if
-    // it matches the first value seen.
-    if (this._mainSessionId === null) {
-      this._mainSessionId = sessionId;
-    }
-    if (this._mainSessionId === sessionId) {
-      sessionId = undefined;
-    }
-
+  _findRealRequestAndSetSession(requestId, targetType, sessionId) {
     let request = this._recordsById.get(requestId);
     if (!request || !request.isValid) return undefined;
 
@@ -230,6 +222,7 @@ class NetworkRecorder extends RequestEventEmitter {
     }
 
     request.setSession(sessionId);
+    request.sessionTargetType = targetType;
 
     return request;
   }

diff --git a/core/lib/network-request.js b/core/lib/network-request.js
@@ -178,12 +178,10 @@ class NetworkRequest {
     this.fetchedViaServiceWorker = false;
     /** @type {string|undefined} */
     this.frameId = '';
-    /**
-     * @type {string|undefined}
-     * Only set for child targets (OOPIFs). This is the sessionId of the protocol connection on
-     * which this request was discovered. `undefined` means it came from the root.
-     */
+    /** @type {string|undefined} */
     this.sessionId = undefined;
+    /** @type {LH.Protocol.TargetType|undefined} */
+    this.sessionTargetType = undefined;
     this.isLinkPreload = false;
   }
 
@@ -320,12 +318,16 @@ class NetworkRequest {
   }
 
   /**
-   * @param {string=} sessionId
+   * @param {string|undefined} sessionId
    */
   setSession(sessionId) {
     this.sessionId = sessionId;
   }
 
+  get isOutOfProcessIframe() {
+    return this.sessionTargetType === 'iframe';
+  }
+
   /**
    * @param {LH.Crdp.Network.Response} response
    * @param {number} timestamp in seconds