Webium product: fix frame id in API test [chromium/src : main]
0 views
Skip to first unread message
Tom Lukaszewicz (Gerrit)
Mar 10, 2026, 3:28:22 PM (2 days ago) Mar 10
to Mingyu Lei, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Mingyu Lei
Tom Lukaszewicz added 1 comment![]( "Open in Gerrit")
File chrome/test/data/extensions/api_test/webnavigation/fencedFrames/test_fencedFrame.js
Line 182, Patchset 3 (Latest):
chrome.webNavigation.getAllFrames({tabId: tab.id}, function (details) {
var documentIds = [];
var nextDocumentId = 1;
details.forEach(element => {
// Since processIds are randomly assigned we remove them for the
// assertEq.
delete element.processId;
if ('parentDocumentId' in element) {
if (documentIds[element.parentDocumentId] === undefined) {
documentIds[element.parentDocumentId] = nextDocumentId++;
}
element.parentDocumentId = documentIds[element.parentDocumentId];
}
if (documentIds[element.documentId] === undefined) {
documentIds[element.documentId] = nextDocumentId++;
}
element.documentId = documentIds[element.documentId];
});Tom Lukaszewicz . unresolved
This looks reasonable - though I suppose we could run into similar problems with other features that instantiate a browser-associated WebUI (bsoton team is already talking about embedding two independent web contents for the toolbar as a possibility for e.g.)
It might be more to capture the frame ids dynamically and compare them on a relative basis (documentId looks like its already doing this).
e.g. the test block becomes
```
function testGetAllFrames() {
chrome.webNavigation.getAllFrames({tabId: tab.id}, function (details) {
var documentIds = [];
var nextDocumentId = 1;
var frameIds = [];
var nextFrameId = 1;
details.forEach(element => {
// Since processIds are randomly assigned we remove them for the
// assertEq.
delete element.processId;
if ('parentDocumentId' in element) {
if (documentIds[element.parentDocumentId] === undefined) {
documentIds[element.parentDocumentId] = nextDocumentId++;
}
element.parentDocumentId = documentIds[element.parentDocumentId];
}
if (documentIds[element.documentId] === undefined) {
documentIds[element.documentId] = nextDocumentId++;
}
element.documentId = documentIds[element.documentId];
if ('parentFrameId' in element && element.parentFrameId > 0) {
if (frameIds[element.parentFrameId] === undefined) {
frameIds[element.parentFrameId] = nextFrameId++;
}
element.parentFrameId = frameIds[element.parentFrameId];
}
if ('frameId' in element && element.frameId > 0) {
if (frameIds[element.frameId] === undefined) {
frameIds[element.frameId] = nextFrameId++;
}
element.frameId = frameIds[element.frameId];
}
});
```Then since the frameIds are relative starting from 1 the expectations only need to change to be
```
chrome.test.assertEq(
[{errorOccurred: false,
documentId: 1,
documentLifecycle: "active",
frameId: 0,
frameType: "outermost_frame",
parentFrameId: -1,
url: URL_MAIN},
{errorOccurred: false,
documentId: 2,
documentLifecycle: "active",
frameId: 1,
frameType: "sub_frame",
parentDocumentId: 1,
parentFrameId: 0,
url: URL_INTERMEDIATE_IFRAME},
{errorOccurred: false,
documentId: 3,
documentLifecycle: "active",
frameId: 2,
frameType: "fenced_frame",
parentDocumentId: 2,
parentFrameId: 1,
url: URL_FENCED_FRAME}],
details);
chrome.test.succeed();
});
```
This was I believe we also don't need to push through the experiment params - wdyt?
Related details
Attention is currently required from:
- Mingyu Lei
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mingyu Lei (Gerrit)
Mar 11, 2026, 3:34:06 AM (yesterday) Mar 11
to Tom Lukaszewicz, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Tom Lukaszewicz
Mingyu Lei voted and added 1 comment![]( "Open in Gerrit")
Votes added by Mingyu Lei
| Commit-Queue | +1 |
1 comment
File chrome/test/data/extensions/api_test/webnavigation/fencedFrames/test_fencedFrame.js
Line 182, Patchset 3:
chrome.webNavigation.getAllFrames({tabId: tab.id}, function (details) {
var documentIds = [];
var nextDocumentId = 1;
details.forEach(element => {
// Since processIds are randomly assigned we remove them for the
// assertEq.
delete element.processId;
if ('parentDocumentId' in element) {
if (documentIds[element.parentDocumentId] === undefined) {
documentIds[element.parentDocumentId] = nextDocumentId++;
}
element.parentDocumentId = documentIds[element.parentDocumentId];
}
if (documentIds[element.documentId] === undefined) {
documentIds[element.documentId] = nextDocumentId++;
}
element.documentId = documentIds[element.documentId];
});Tom Lukaszewicz . resolved
Mingyu Lei
Thanks! That sounds better, I have update the test.
Also for the other test, since there is meaningless to normalize the only frame id and check if it equals to 1, I just changed it to only ensuring the frame id is not undefined like the document id.
Related details
Attention is currently required from:
- Tom Lukaszewicz
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Lukaszewicz (Gerrit)
Mar 11, 2026, 4:49:29 PM (19 hours ago) Mar 11
to Mingyu Lei, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Mingyu Lei
Tom Lukaszewicz voted and added 2 comments![]( "Open in Gerrit")
Votes added by Tom Lukaszewicz
| Code-Review | +1 |
2 comments
Patchset-level comments
File chrome/test/data/extensions/api_test/messaging/connect_fenced_frames/test.js
Line 41, Patchset 6 (Latest):
// documentId / frameId are unique IDs so we can't assume anything about it,
// it's also possible that they are changed just
// that it is provided.Tom Lukaszewicz . unresolved
nit: Could we wrap this to 80 cols?
Related details
Attention is currently required from:
- Mingyu Lei
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mingyu Lei (Gerrit)
2:01 AM (10 hours ago) 2:01 AM
to Tom Lukaszewicz, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Mingyu Lei voted and added 1 comment![]( "Open in Gerrit")
Votes added by Mingyu Lei
| Commit-Queue | +1 |
1 comment
File chrome/test/data/extensions/api_test/messaging/connect_fenced_frames/test.js
Line 41, Patchset 6:
// documentId / frameId are unique IDs so we can't assume anything about it,
// it's also possible that they are changed just
// that it is provided.Tom Lukaszewicz . resolved
nit: Could we wrap this to 80 cols?
Mingyu Lei
Done, but sorry I set up `jj` and it somehow reformat the whole `fencedFrameNavigation()` including those unmodified lines in the latest patch.
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mingyu Lei (Gerrit)
3:03 AM (9 hours ago) 3:03 AM
to Tom Lukaszewicz, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Mingyu Lei voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
3:06 AM (9 hours ago) 3:06 AM
to Mingyu Lei, Tom Lukaszewicz, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
6 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: chrome/test/data/extensions/api_test/webnavigation/fencedFrames/test_fencedFrame.js
Insertions: 199, Deletions: 149.
@@ -21,155 +21,205 @@
// an iframe which contains a fenced frame.
// Tests that the frameId/parentFrameId are populated correctly.
function fencedFrameNavigation() {
- expect([
- { label: 'main-onBeforeNavigate',
- event: 'onBeforeNavigate',
- details: { documentLifecycle: "active",
- frameId: 0,
- frameType: "outermost_frame",
- parentFrameId: -1,
- processId: -1,
- tabId: 0,
- timeStamp: 0,
- url: URL_MAIN }},
- { label: 'main-onCommitted',
- event: 'onCommitted',
- details: { documentId: 1,
- documentLifecycle: "active",
- frameId: 0,
- frameType: "outermost_frame",
- parentFrameId: -1,
- processId: 0,
- tabId: 0,
- timeStamp: 0,
- transitionQualifiers: [],
- transitionType: 'link',
- url: URL_MAIN }},
- { label: 'main-onDOMContentLoaded',
- event: 'onDOMContentLoaded',
- details: { documentId: 1,
- documentLifecycle: "active",
- frameId: 0,
- frameType: "outermost_frame",
- parentFrameId: -1,
- processId: 0,
- tabId: 0,
- timeStamp: 0,
- url: URL_MAIN }},
- { label: 'main-onCompleted',
- event: 'onCompleted',
- details: { documentId: 1,
- documentLifecycle: "active",
- frameId: 0,
- frameType: "outermost_frame",
- parentFrameId: -1,
- processId: 0,
- tabId: 0,
- timeStamp: 0,
- url: URL_MAIN }},
- { label: 'intermediate-onBeforeNavigate',
- event: 'onBeforeNavigate',
- details: { documentLifecycle: "active",
- frameId: 1,
- frameType: "sub_frame",
- parentDocumentId: 1,
- parentFrameId: 0,
- processId: -1,
- tabId: 0,
- timeStamp: 0,
- url: URL_INTERMEDIATE_IFRAME }},
- { label: 'intermediate-onCommitted',
- event: 'onCommitted',
- details: { documentId: 2,
- documentLifecycle: "active",
- frameId: 1,
- frameType: "sub_frame",
- parentDocumentId: 1,
- parentFrameId: 0,
- processId: 0,
- tabId: 0,
- timeStamp: 0,
- transitionQualifiers: [],
- transitionType: 'auto_subframe',
- url: URL_INTERMEDIATE_IFRAME }},
- { label: 'intermediate-onDOMContentLoaded',
- event: 'onDOMContentLoaded',
- details: { documentId: 2,
- documentLifecycle: "active",
- frameId: 1,
- frameType: "sub_frame",
- parentDocumentId: 1,
- parentFrameId: 0,
- processId: 0,
- tabId: 0,
- timeStamp: 0,
- url: URL_INTERMEDIATE_IFRAME }},
- { label: 'intermediate-onCompleted',
- event: 'onCompleted',
- details: { documentId: 2,
- documentLifecycle: "active",
- frameId: 1,
- frameType: "sub_frame",
- parentDocumentId: 1,
- parentFrameId: 0,
- processId: 0,
- tabId: 0,
- timeStamp: 0,
- url: URL_INTERMEDIATE_IFRAME }},
- { label: 'a.test-onBeforeNavigate',
- event: 'onBeforeNavigate',
- details: { documentLifecycle: "active",
- frameId: 2,
- frameType: "fenced_frame",
- parentDocumentId: 2,
- parentFrameId: 1,
- processId: -1,
- tabId: 0,
- timeStamp: 0,
- url: URL_FENCED_FRAME }},
- { label: 'a.test-onCommitted',
- event: 'onCommitted',
- details: { documentId: 3,
- documentLifecycle: "active",
- frameId: 2,
- frameType: "fenced_frame",
- parentDocumentId: 2,
- parentFrameId: 1,
- processId: 1,
- tabId: 0,
- timeStamp: 0,
- transitionQualifiers: [],
- transitionType: 'auto_subframe',
- url: URL_FENCED_FRAME }},
- { label: 'a.test-onDOMContentLoaded',
- event: 'onDOMContentLoaded',
- details: { documentId: 3,
- documentLifecycle: "active",
- frameId: 2,
- frameType: "fenced_frame",
- parentDocumentId: 2,
- parentFrameId: 1,
- processId: 1,
- tabId: 0,
- timeStamp: 0,
- url: URL_FENCED_FRAME }},
- { label: 'a.test-onCompleted',
- event: 'onCompleted',
- details: { documentId: 3,
- documentLifecycle: "active",
- frameId: 2,
- frameType: "fenced_frame",
- parentDocumentId: 2,
- parentFrameId: 1,
- processId: 1,
- tabId: 0,
- timeStamp: 0,
- url: URL_FENCED_FRAME }}],
- [
- navigationOrder('main-'),
- navigationOrder('intermediate-'),
- navigationOrder('a.test-'),
- ]);
+ expect(
+ [
+ {
+ label: 'main-onBeforeNavigate',
+ event: 'onBeforeNavigate',
+ details: {
+ documentLifecycle: 'active',
+ frameId: 0,
+ frameType: 'outermost_frame',
+ parentFrameId: -1,
+ processId: -1,
+ tabId: 0,
+ timeStamp: 0,
+ url: URL_MAIN
+ }
+ },
+ {
+ label: 'main-onCommitted',
+ event: 'onCommitted',
+ details: {
+ documentId: 1,
+ documentLifecycle: 'active',
+ frameId: 0,
+ frameType: 'outermost_frame',
+ parentFrameId: -1,
+ processId: 0,
+ tabId: 0,
+ timeStamp: 0,
+ transitionQualifiers: [],
+ transitionType: 'link',
+ url: URL_MAIN
+ }
+ },
+ {
+ label: 'main-onDOMContentLoaded',
+ event: 'onDOMContentLoaded',
+ details: {
+ documentId: 1,
+ documentLifecycle: 'active',
+ frameId: 0,
+ frameType: 'outermost_frame',
+ parentFrameId: -1,
+ processId: 0,
+ tabId: 0,
+ timeStamp: 0,
+ url: URL_MAIN
+ }
+ },
+ {
+ label: 'main-onCompleted',
+ event: 'onCompleted',
+ details: {
+ documentId: 1,
+ documentLifecycle: 'active',
+ frameId: 0,
+ frameType: 'outermost_frame',
+ parentFrameId: -1,
+ processId: 0,
+ tabId: 0,
+ timeStamp: 0,
+ url: URL_MAIN
+ }
+ },
+ {
+ label: 'intermediate-onBeforeNavigate',
+ event: 'onBeforeNavigate',
+ details: {
+ documentLifecycle: 'active',
+ frameId: 1,
+ frameType: 'sub_frame',
+ parentDocumentId: 1,
+ parentFrameId: 0,
+ processId: -1,
+ tabId: 0,
+ timeStamp: 0,
+ url: URL_INTERMEDIATE_IFRAME
+ }
+ },
+ {
+ label: 'intermediate-onCommitted',
+ event: 'onCommitted',
+ details: {
+ documentId: 2,
+ documentLifecycle: 'active',
+ frameId: 1,
+ frameType: 'sub_frame',
+ parentDocumentId: 1,
+ parentFrameId: 0,
+ processId: 0,
+ tabId: 0,
+ timeStamp: 0,
+ transitionQualifiers: [],
+ transitionType: 'auto_subframe',
+ url: URL_INTERMEDIATE_IFRAME
+ }
+ },
+ {
+ label: 'intermediate-onDOMContentLoaded',
+ event: 'onDOMContentLoaded',
+ details: {
+ documentId: 2,
+ documentLifecycle: 'active',
+ frameId: 1,
+ frameType: 'sub_frame',
+ parentDocumentId: 1,
+ parentFrameId: 0,
+ processId: 0,
+ tabId: 0,
+ timeStamp: 0,
+ url: URL_INTERMEDIATE_IFRAME
+ }
+ },
+ {
+ label: 'intermediate-onCompleted',
+ event: 'onCompleted',
+ details: {
+ documentId: 2,
+ documentLifecycle: 'active',
+ frameId: 1,
+ frameType: 'sub_frame',
+ parentDocumentId: 1,
+ parentFrameId: 0,
+ processId: 0,
+ tabId: 0,
+ timeStamp: 0,
+ url: URL_INTERMEDIATE_IFRAME
+ }
+ },
+ {
+ label: 'a.test-onBeforeNavigate',
+ event: 'onBeforeNavigate',
+ details: {
+ documentLifecycle: 'active',
+ frameId: 2,
+ frameType: 'fenced_frame',
+ parentDocumentId: 2,
+ parentFrameId: 1,
+ processId: -1,
+ tabId: 0,
+ timeStamp: 0,
+ url: URL_FENCED_FRAME
+ }
+ },
+ {
+ label: 'a.test-onCommitted',
+ event: 'onCommitted',
+ details: {
+ documentId: 3,
+ documentLifecycle: 'active',
+ frameId: 2,
+ frameType: 'fenced_frame',
+ parentDocumentId: 2,
+ parentFrameId: 1,
+ processId: 1,
+ tabId: 0,
+ timeStamp: 0,
+ transitionQualifiers: [],
+ transitionType: 'auto_subframe',
+ url: URL_FENCED_FRAME
+ }
+ },
+ {
+ label: 'a.test-onDOMContentLoaded',
+ event: 'onDOMContentLoaded',
+ details: {
+ documentId: 3,
+ documentLifecycle: 'active',
+ frameId: 2,
+ frameType: 'fenced_frame',
+ parentDocumentId: 2,
+ parentFrameId: 1,
+ processId: 1,
+ tabId: 0,
+ timeStamp: 0,
+ url: URL_FENCED_FRAME
+ }
+ },
+ {
+ label: 'a.test-onCompleted',
+ event: 'onCompleted',
+ details: {
+ documentId: 3,
+ documentLifecycle: 'active',
+ frameId: 2,
+ frameType: 'fenced_frame',
+ parentDocumentId: 2,
+ parentFrameId: 1,
+ processId: 1,
+ tabId: 0,
+ timeStamp: 0,
+ url: URL_FENCED_FRAME
+ }
+ }
+ ],
+ [
+ navigationOrder('main-'),
+ navigationOrder('intermediate-'),
+ navigationOrder('a.test-'),
+ ]);
chrome.tabs.update(tab.id, {url: URL_MAIN});
},
```
```
The name of the file: chrome/test/data/extensions/api_test/messaging/connect_fenced_frames/test.js
Insertions: 11, Deletions: 12.
@@ -22,13 +22,13 @@
// the tab and wait for that message.
let actualSender;
let messagePromise = new Promise((resolve) => {
- chrome.runtime.onMessage.addListener(function messageListener(message,
- sender) {
- chrome.test.assertEq(message.connected, true);
- chrome.runtime.onMessage.removeListener(messageListener);
- actualSender = sender;
- resolve();
- });
+ chrome.runtime.onMessage.addListener(
+ function messageListener(message, sender) {
+ chrome.test.assertEq(message.connected, true);
+ chrome.runtime.onMessage.removeListener(messageListener);
+ actualSender = sender;
+ resolve();
+ });
});
// This tab will be used for the other tests as well.
@@ -38,9 +38,8 @@
});
});
await messagePromise;
- // documentId / frameId are unique IDs so we can't assume anything about it,
- // it's also possible that they are changed just
- // that it is provided.
+ // documentId / frameId are unique IDs so we can't assume anything about
+ // it, it's also possible that they are changed just that it is provided.
chrome.test.assertNe(undefined, actualSender.documentId);
chrome.test.assertNe(undefined, actualSender.frameId);
chrome.test.assertEq('active', actualSender.documentLifecycle);
@@ -69,7 +68,7 @@
chrome.test.getConfig(async (config) => {
serverOrigin = `http://localhost:${config.testServer.port}`;
- serverURL = serverOrigin + '/extensions/api_test/messaging/'
- + 'connect_fenced_frames/';
+ serverURL = serverOrigin + '/extensions/api_test/messaging/' +
+ 'connect_fenced_frames/';
chrome.test.runTests(tests);
});
```
Change information
Commit message:
Webium product: fix frame id in API test
When WebUIReloadButtonEnabled is enabled, there will be a web UI loaded
in top chrome, and it's causing the frame id to have off-by-one error.
This CL fixes that by normalizing the frame id (like the document id)
for
chrome/test/data/extensions/api_test/webnavigation/fencedFrames/test_fencedFrame.js
and skip checking the exact id for
chrome/test/data/extensions/api_test/messaging/connect_fenced_frames/test.js
Bug: 444358999
Change-Id: I8990bdda66b827ed24eb1234132188bea7ecaf39
Reviewed-by: Tom Lukaszewicz <tl...@chromium.org>
Commit-Queue: Mingyu Lei <le...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1598248}
Files:
- M chrome/test/data/extensions/api_test/messaging/connect_fenced_frames/test.js
- M chrome/test/data/extensions/api_test/webnavigation/fencedFrames/test_fencedFrame.js
Change size: L
Delta: 2 files changed, 276 insertions(+), 216 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Tom Lukaszewicz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages