[devtools] Remove a conditional that possibly exposes an ordering bug

This CL removes a conditional that defends against a message reordering
bug, but introduces a target confusion, and hence message loss, in the
DevTools front-end for certain navigations (the test
oopif-elements-navigate-out-extra-info.js
exposes the bug).

We expect these tests to flake and expose the reordering bug, so we can
fix that one next.

Bug: chromium:1030329
Fixed: 1030329
Change-Id: I0f14d6dcfe4047ff252fa1b4dd956f4258e4e45b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948898
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723433}

content/browser/devtools/devtools_instrumentation.cc

@@ -72,15 +72,7 @@ FrameTreeNode* GetFtnForNetworkRequest(int process_id, int routing_id) {
   // the id is set to 0. In these situations, the routing_id is the frame tree
   // node id, and can be used directly.
   if (process_id == 0) {
-    FrameTreeNode* ftn = FrameTreeNode::GloballyFindByID(routing_id);
-    if (ftn == nullptr)
-      return nullptr;
-    // If this is a navigation request (process_id == 0) of a child frame
-    // (ftn->parent()), then requestWillBeSent and responseReceived are
-    // delivered to the parent frame instead of the child because we don't know
-    // if the child will become an OOPIF with a separate target yet or not. Do
-    // the same for requestWillBeSentExtraInfo and responseReceivedExtraInfo.
-    return ftn->parent() ? ftn->parent() : ftn;
+    return FrameTreeNode::GloballyFindByID(routing_id);
   }
   return FrameTreeNode::GloballyFindByID(
       RenderFrameHost::GetFrameTreeNodeIdForRoutingId(process_id, routing_id));

third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-in2in-expected.txt

+Verifies that navigating from a in-process iframe to in-process iframe sets the right sessionId.
+
+Events received: [onRequestWillBeSent, onRequestWillBeSentExtraInfo, onResponseReceived, onResponseReceivedExtraInfo]
+Number of request ids: 4, unique ids 1
+Number of session ids: 4, unique ids 1
+

third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-in2in.js

+(async function(testRunner) {
+  const {page, session, dp} = await testRunner.startBlank(
+      `Verifies that navigating from a in-process iframe to in-process iframe sets the right sessionId.\n`);
+
+  await dp.Page.enable();
+  await dp.Page.setLifecycleEventsEnabled({ enabled: true });
+  let numberOfLoads = 0;
+  dp.Page.onLifecycleEvent(onLifecycleEvent);
+
+  await dp.Network.clearBrowserCache();
+  await dp.Network.setCacheDisabled({cacheDisabled: true});
+  await dp.Network.enable();
+
+  const helper = (await testRunner.loadScript('resources/extra-info-helper.js'))(dp, session);
+
+  await session.navigate('resources/page-in.html');
+
+  async function onLifecycleEvent(event) {
+    if (event.params.name != "load") return;
+    numberOfLoads++;
+    if (numberOfLoads != 2) return;
+    const events = [];
+    // The two in-process iframes from page-in.html have loaded.
+    dp.Network.onRequestWillBeSent(() => events.push("onRequestWillBeSent"));
+    dp.Network.onRequestWillBeSentExtraInfo(() => events.push("onRequestWillBeSentExtraInfo"));
+    dp.Network.onResponseReceivedExtraInfo(() => events.push("onResponseReceivedExtraInfo"));
+    dp.Network.onResponseReceived(() => events.push("onResponseReceived"));
+
+    const [request, requestExtraInfo, responseExtraInfo, response] =
+      await helper.jsNavigateIFrameWithExtraInfo('page-iframe', '/devtools/oopif/resources/inner-iframe.html');
+
+    testRunner.log(`Events received: [${events.sort().join(", ")}]`);
+    const requests = [request, requestExtraInfo, responseExtraInfo, response];
+    const requestIds = requests.map(x => x.params.requestId);
+    const sessionIds = requests.map(x => x.sessionId);
+
+    testRunner.log(`Number of request ids: ${requestIds.length}, unique ids ${new Set(requestIds).size}`);
+    testRunner.log(`Number of session ids: ${sessionIds.length}, unique ids ${new Set(sessionIds).size}`);
+
+    testRunner.completeTest();
+  }
+})

third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-in2out-expected.txt

+Verifies that navigating from a OOPIF to in-process iframe sets the right sessionId.
+
+Loaded page-out with OOPIF, setting iframe src to in-process URL.
+Events received on iframe: []
+Events received on main frame: [onRequestWillBeSent, onRequestWillBeSentExtraInfo, onResponseReceived, onResponseReceivedExtraInfo]
+

third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-in2out.js

+(async function(testRunner) {
+  const {page, session, dp} = await testRunner.startBlank(
+      `Verifies that navigating from a OOPIF to in-process iframe sets the right sessionId.\n`);
+
+  await dp.Page.enable();
+  await dp.Page.setLifecycleEventsEnabled({ enabled: true });
+  let numberOfLoads = 0;
+  dp.Page.onLifecycleEvent(onLifecycleEvent);
+
+  await dp.Network.clearBrowserCache();
+  await dp.Network.setCacheDisabled({cacheDisabled: true});
+  await dp.Network.enable();
+
+  await dp.Target.setAutoAttach({autoAttach: true, waitForDebuggerOnStart: true, flatten: true});
+
+  const iFrameEvents = [];
+  const mainEvents = [];
+
+  function hook(network, events) {
+    network.onRequestWillBeSent(() => events.push("onRequestWillBeSent"));
+    network.onRequestWillBeSentExtraInfo(() => events.push("onRequestWillBeSentExtraInfo"));
+    network.onResponseReceivedExtraInfo(() => events.push("onResponseReceivedExtraInfo"));
+    network.onResponseReceived(() => events.push("onResponseReceived"));
+  }
+
+  dp.Target.onAttachedToTarget(async event => {
+   const dp2 = session.createChild(event.params.sessionId).protocol;
+   await dp2.Page.enable();
+   await dp2.Page.setLifecycleEventsEnabled({ enabled: true });
+   dp2.Page.onLifecycleEvent(onLifecycleEvent);
+   await dp2.Network.clearBrowserCache();
+   await dp2.Network.setCacheDisabled({cacheDisabled: true});
+   await dp2.Network.enable();
+   // None of these should fire.
+   hook(dp2.Network, iFrameEvents);
+   await dp2.Runtime.runIfWaitingForDebugger();
+  });
+
+  await session.navigate('resources/page-in.html');
+
+  async function onLifecycleEvent(event) {
+    if (event.params.name != "load") return;
+    numberOfLoads++;
+    if (numberOfLoads == 4) {
+      // There are two load events fired, one for the OOPIF frame, and one for page-out after
+      // setting the src property on the iframe.
+      testRunner.log(`Events received on iframe: [${iFrameEvents.sort().join(", ")}]`);
+      testRunner.log(`Events received on main frame: [${mainEvents.sort().join(", ")}]`);
+      testRunner.completeTest();
+    }
+    if (numberOfLoads == 2) {
+      testRunner.log("Loaded page-out with OOPIF, setting iframe src to in-process URL.");
+
+      hook(dp.Network, mainEvents);
+
+      const iFrameId = 'page-iframe';
+      const url = 'http://devtools.oopif.test:8000/inspector-protocol/network/resources/inner-iframe.html';
+      await session.evaluate(`document.getElementById('${iFrameId}').src = '${url}'`);
+    }
+  }
+})

third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-out2in-expected.txt

+Verifies that navigating from a OOPIF to in-process iframe sets the right sessionId.
+
+Loaded page-out with OOPIF, setting iframe src to in-process URL.
+Events received on iframe: [onRequestWillBeSent,onRequestWillBeSentExtraInfo,onResponseReceived,onResponseReceivedExtraInfo]
+Events received on main frame: []
+

third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-out2in.js

+(async function(testRunner) {
+  const {page, session, dp} = await testRunner.startBlank(
+      `Verifies that navigating from a OOPIF to in-process iframe sets the right sessionId.\n`);
+
+  await dp.Page.enable();
+  await dp.Page.setLifecycleEventsEnabled({ enabled: true });
+  let numberOfLoads = 0;
+  dp.Page.onLifecycleEvent(onLifecycleEvent);
+
+  await dp.Network.clearBrowserCache();
+  await dp.Network.setCacheDisabled({cacheDisabled: true});
+  await dp.Network.enable();
+
+  await dp.Target.setAutoAttach({autoAttach: true, waitForDebuggerOnStart: true, flatten: true});
+
+  const iFrameEvents = [];
+  const mainEvents = [];
+
+  function hook(network, events) {
+    network.onRequestWillBeSent(() => events.push("onRequestWillBeSent"));
+    network.onRequestWillBeSentExtraInfo(() => events.push("onRequestWillBeSentExtraInfo"));
+    network.onResponseReceivedExtraInfo(() => events.push("onResponseReceivedExtraInfo"));
+    network.onResponseReceived(() => events.push("onResponseReceived"));
+  }
+
+  dp.Target.onAttachedToTarget(async event => {
+   const dp2 = session.createChild(event.params.sessionId).protocol;
+   await dp2.Page.enable();
+   await dp2.Page.setLifecycleEventsEnabled({ enabled: true });
+   dp2.Page.onLifecycleEvent(onLifecycleEvent);
+   await dp2.Network.clearBrowserCache();
+   await dp2.Network.setCacheDisabled({cacheDisabled: true});
+   await dp2.Network.enable();
+   // None of these should fire.
+   hook(dp2.Network, iFrameEvents);
+   await dp2.Runtime.runIfWaitingForDebugger();
+  });
+
+  await session.navigate('resources/page-out.html');
+
+  async function onLifecycleEvent(event) {
+    if (event.params.name != "load") return;
+    numberOfLoads++;
+    if (numberOfLoads == 4) {
+      // There are two load events fired, one for the OOPIF frame, and one for page-out after
+      // setting the src property on the iframe.
+      testRunner.log(`Events received on iframe: [${iFrameEvents.sort()}]`);
+      testRunner.log(`Events received on main frame: [${mainEvents.sort()}]`);
+      testRunner.completeTest();
+    }
+    if (numberOfLoads == 2) {
+      testRunner.log("Loaded page-out with OOPIF, setting iframe src to in-process URL.");
+
+      hook(dp.Network, mainEvents);
+
+      const iFrameId = 'page-iframe';
+      const url = 'http://127.0.0.1:8000/inspector-protocol/network/resources/inner-iframe.html';
+      await session.evaluate(`document.getElementById('${iFrameId}').src = '${url}'`);
+    }
+  }
+})

third_party/blink/web_tests/http/tests/inspector-protocol/network/resources/extra-info-helper.js

@@ -31,6 +31,12 @@
       const responseExtraInfo = await responseExtraInfoPromise;
       return {requestExtraInfo, responseExtraInfo};
     }
+
+    async jsNavigateIFrameWithExtraInfo(iFrameId, url) {
+      const promises = [this._dp.Network.onceRequestWillBeSent(), this._dp.Network.onceRequestWillBeSentExtraInfo(), this._dp.Network.onceResponseReceivedExtraInfo(), this._dp.Network.onceResponseReceived()];
+      await this._session.evaluate(`document.getElementById('${iFrameId}').src = '${url}'`);
+      return Promise.all(promises);
+    }
   };
 
   return (dp, session) => {

third_party/blink/web_tests/http/tests/inspector-protocol/network/resources/inner-iframe.html

+<body>IFRAME</body>

third_party/blink/web_tests/http/tests/inspector-protocol/network/resources/page-in.html

+<body>
+    <div>
+        <iframe id="page-iframe" src="/inspector-protocol/network/resources/inner-iframe.html"></iframe>
+    </div>
+</body>

third_party/blink/web_tests/http/tests/inspector-protocol/network/resources/page-out.html

+<body>
+    <div>
+        <iframe id="page-iframe" src="http://devtools.oopif.test:8000/inspector-protocol/network/resources/inner-iframe.html"></iframe>
+    </div>
+</body>