Use FrameTreeNode ID as frameId in extension APIs

chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.js

Index: chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.js
diff --git a/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.js b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.js
new file mode 100644
index 0000000000000000000000000000000000000000..dac9e9cf1855670a7b857c7bc366c3d2f0136bcc
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.js
@@ -0,0 +1,211 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+onload = function() {
+  debug = true;
+  var getURL = chrome.extension.getURL;
+  var URL_MAIN = getURL("main.html");
+  var URL_FRAME1 = "http://a.com:PORT/extensions/api_test/webnavigation/" +
+      "crossProcessIframe/frame.html";
+  var URL_FRAME2 = "http://b.com:PORT/extensions/api_test/webnavigation/" +
+      "crossProcessIframe/frame.html";
+  var URL_FRAME3 = "http://c.com:PORT/extensions/api_test/webnavigation/" +
+      "crossProcessIframe/frame.html";
+  chrome.tabs.create({"url": "about:blank"}, function(tab) {
+    var tabId = tab.id;
+    chrome.test.getConfig(function(config) {
+      var fixPort = function(url) {
+        return url.replace(/PORT/g, config.testServer.port);
+      };
+      URL_FRAME1 = fixPort(URL_FRAME1);
+      URL_FRAME2 = fixPort(URL_FRAME2);
+      URL_FRAME3 = fixPort(URL_FRAME3);
+
+      chrome.test.runTests([
+        // Navigates from an extension page to a HTTP page which causes a
+        // process switch. The extension page embeds a same-process iframe which
+        // embeds another frame navigates three times (cross-process):
+        // c. Loaded by the parent frame.
+        // d. Navigated by the parent frame.
+        // e. Navigated by the child frame.
+        // Tests whether the frameId stays constant across navigations.
+        function crossProcessIframe() {
+          expect([
+            { label: "a-onBeforeNavigate",

[nasko, 2015/11/06 23:28:27]

Shouldn't we use "main-" prefix on these, as we already use a.com as a domain
for the subframe?

+              event: "onBeforeNavigate",
+              details: { frameId: 0,
+                         parentFrameId: -1,
+                         processId: 0,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_MAIN }},
+            { label: "a-onCommitted",
+              event: "onCommitted",
+              details: { frameId: 0,
+                         processId: 0,
+                         tabId: 0,
+                         timeStamp: 0,
+                         transitionQualifiers: [],
+                         transitionType: "link",
+                         url: URL_MAIN }},
+            { label: "a-onDOMContentLoaded",
+              event: "onDOMContentLoaded",
+              details: { frameId: 0,
+                         processId: 0,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_MAIN }},
+            { label: "a-onCompleted",
+              event: "onCompleted",
+              details: { frameId: 0,
+                         processId: 0,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_MAIN }},
+            // Process swap.

[nasko, 2015/11/06 23:28:27]

Here I would expect to see the load of the iframe with a.com, which includes
onBeforeNavigate, onCommitted, onDOMContentLoaded, though not sure about getting
an onCompleted. I think because of how the code is structured, you will get
onErrorOccurred, since we navigate the frame before finishing the onload event
execution.

+            { label: "pre-b-onBeforeNavigate",
+              event: "onBeforeNavigate",
+              details: { frameId: 1,
+                         parentFrameId: 0,
+                         processId: 0,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME1 }},
+            { label: "pre-b-onErrorOccurred",
+              event: "onErrorOccurred",
+              details: { error: "net::ERR_ABORTED",
+                         frameId: 1,
+                         processId: 0,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME1 }},
+            { label: "b-onBeforeNavigate",
+              event: "onBeforeNavigate",
+              details: { frameId: 1,
+                         parentFrameId: 0,
+                         processId: 1,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME1 }},
+            { label: "b-onCommitted",
+              event: "onCommitted",
+              details: { frameId: 1,
+                         processId: 1,
+                         tabId: 0,
+                         timeStamp: 0,
+                         transitionQualifiers: [],
+                         transitionType: "auto_subframe",

[nasko, 2015/11/06 23:28:27]

Hmm, this isn't quite expected for me. auto_subframe should be only in the case
where the parent hasn't fired and completed its onload event. For the navigation
to b.com, the parent should've completed the onload event handler, as main.js
uses setTimeout in the handler to defer the navigation to the next iteration of
the event loop.

+                         url: URL_FRAME1 }},
+            { label: "b-onDOMContentLoaded",
+              event: "onDOMContentLoaded",
+              details: { frameId: 1,
+                         processId: 1,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME1 }},
+            { label: "b-onCompleted",
+              event: "onCompleted",
+              details: { frameId: 1,
+                         processId: 1,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME1 }},
+            // TODO(robwu): Why is there no onBeforeNavigate and onErrorOccurred
+            // like above before the process swap?

[nasko, 2015/11/06 23:28:27]

When you are on b.com, it registers an onload event handler, which has a
setTimeout() call. This will result on the onload event to complete successfully
and we will fire the onCompleted for b.com, which is the event right before your
comment. Because of the setTimeout() handling, the function that will navigate
the frame to c.com will be queued up on the event loop.

[robwu, 2015/11/07 00:23:40]

By "like above", I'm referring to the pair of onBeforeNavigate+onErrorOccurred
events that occur at the other navigation. But I think that I've found a
plausible explanation: https://codereview.chromium.org/1413543005/#msg20

+            { label: "c-onBeforeNavigate",
+              event: "onBeforeNavigate",
+              details: { frameId: 1,
+                         parentFrameId: 0,
+                         processId: 2,

[nasko, 2015/11/06 23:28:27]

Does this pass? I would expect the onBeforeNavigate to come from process 1, the
browser process to do a transfer of the navigation from one process to another,
then the commit to come from process 2. Though this is complex enough that I
can't reason about it entirely in my head and need to probably patch in the CL
and look at the debugger.

[robwu, 2015/11/07 00:23:40]

The test passes. My TODO comment a few lines back refers to the absence of the
sequence of events that you're describing. See also my reply ("plausible
explanation") that I mentioned in my codereview comment above.

+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME2 }},
+            { label: "c-onCommitted",
+              event: "onCommitted",
+              details: { frameId: 1,
+                         processId: 2,
+                         tabId: 0,
+                         timeStamp: 0,
+                         transitionQualifiers: [],
+                         transitionType: "manual_subframe",
+                         url: URL_FRAME2 }},
+            { label: "c-onDOMContentLoaded",
+              event: "onDOMContentLoaded",
+              details: { frameId: 1,
+                         processId: 2,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME2 }},
+            { label: "c-onCompleted",
+              event: "onCompleted",
+              details: { frameId: 1,
+                         processId: 2,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME2 }},
+            // Process swap.
+            { label: "pre-d-onBeforeNavigate",

[nasko, 2015/11/06 23:28:27]

Urgh, I might have been misreading these steps because of the labelling : (.
Let's use the prefixes to be "main", "a-com", "b-com", "c-com", so they match
the navigations we perform in the JS files.

[robwu, 2015/12/07 23:44:22]

Done.

+              event: "onBeforeNavigate",
+              details: { frameId: 1,
+                         parentFrameId: 0,
+                         processId: 2,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME3 }},
+            { label: "pre-d-onErrorOccurred",
+              event: "onErrorOccurred",
+              details: { error: "net::ERR_ABORTED",
+                         frameId: 1,
+                         processId: 2,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME3 }},
+            { label: "d-onBeforeNavigate",
+              event: "onBeforeNavigate",
+              details: { frameId: 1,
+                         parentFrameId: 0,
+                         processId: 3,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME3 }},
+            { label: "d-onCommitted",
+              event: "onCommitted",
+              details: { frameId: 1,
+                         processId: 3,
+                         tabId: 0,
+                         timeStamp: 0,
+                         transitionQualifiers: [],
+                         transitionType: "manual_subframe",
+                         url: URL_FRAME3 }},
+            { label: "d-onDOMContentLoaded",
+              event: "onDOMContentLoaded",
+              details: { frameId: 1,
+                         processId: 3,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME3 }},
+            { label: "d-onCompleted",
+              event: "onCompleted",
+              details: { frameId: 1,
+                         processId: 3,
+                         tabId: 0,
+                         timeStamp: 0,
+                         url: URL_FRAME3 }}],
+            [
+              navigationOrder("a-"),
+              navigationOrder("b-"),
+              navigationOrder("c-"),
+              navigationOrder("d-"),
+              ["pre-b-onBeforeNavigate", "pre-b-onErrorOccurred"],
+              ["pre-d-onBeforeNavigate", "pre-d-onErrorOccurred"]]);
+
+          chrome.tabs.update(tabId, {
+            url: URL_MAIN + "?" + config.testServer.port
+          });
+        },
+
+      ]);
+    });
+  });
+};