Page.frameNavigated event from OOPIF is missing frame.name property |
||||||
Issue descriptionIn Chrome 65, Page.frameNavigated event from OOPIF always had a frame.name property, even though its value could be an empty string. In Chrome 66, however, this property is missing, and is causing ChromeDriver to throw an error. I am checking in a workaround in ChromeDriver (http://crrev.com/c/969788) to keep it working, but I'd like to confirm whether the missing frame.name property is intentional or accidental. An example event in Chrome 65: [1521512839.191][DEBUG]: DEVTOOLS EVENT Page.frameNavigated { "frame": { "id": "403A823448CE22C0539B279BB5B5C210", "loaderId": "22F6486391B1550F47C033FD4AC23AB9", "mimeType": "text/html", "name": "", "parentId": "52725EF1F26BBD2D5513982CA7C213D4", "securityOrigin": "http://localhost:46403", "url": "http://localhost:46403/chromedriver/inner.html" } } An example event in Chrome 66 and 67 (note that "name" is missing): [1521511084.681][DEBUG]: DEVTOOLS EVENT Page.frameNavigated { "frame": { "id": "794723890EB2408E0A2FFE527748F04F", "loaderId": "5D3C9FA4C8EBC891C4E4C171105C0C14", "mimeType": "text/html", "parentId": "7B012D43394633D6AD227E0D34383342", "securityOrigin": "http://localhost:51779", "url": "http://localhost:51779/chromedriver/outer.html" } }
,
Mar 20 2018
Yes, I'm curious if this is a DevTools change. It does sound quite similar to dcheng's r537735 for issue 803620, which changed behavior around frames with empty names (see comment 35 of that bug). That said, that change landed in M65, so it doesn't match the window you describe, and I'm not sure if it would have this effect on ChromeDriver.
,
Mar 20 2018
This is something controlled by devtools: https://chromium.googlesource.com/chromium/src/+blame/d260e9cf660f062b203692601cf9b6ccb27f3d1e/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp#941 I guess what's changed is there's more remote frames now, so we never enter that block (and thus don't set the name). Maybe devtools should be using FrameTree::GetName() here, since that's also updated when window.name changes? Or perhaps devtools intentoinally wants to use the name attribute on the iframe element since it's more stable?
,
Mar 21 2018
,
Apr 3 2018
I see the workaround landed in r544272. Just wanted to confirm-- is there still a functional bug here in Chrome or ChromeDriver, or is this about cleaning up the frame.name behavior so the workaround can be removed?
,
Apr 3 2018
The main purpose of this bug is to confirm whether the missing frame.name property is by design or a bug. * If it is by design, please simply close this bug, and we'll keep ChromeDriver unchanged. * If it is a bug, then probably the right thing to do is fix the bug, and then remove the workaround from ChromeDriver.
,
Apr 3 2018
John, first of all, this wasn't intentional -- this is a side effect of OOPIF as Daniel correctly states. The code there relies on having access to the frame owner element, which we don't really need for the frame name itself, but do need for the fall-back case where we use element's id attribute if name is empty. I'm thinking of landing a quick fix that will take the name from the frame tree node, but there's no quick fix for the "id" fall-back -- we don't propagate it between the renderers, and I'm not sure whether we should. The question is, do you think this is something that may affect WebDriver? Please let me know if there's a use case that relies on that.
,
Apr 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cae175987c1f842d1f232f9f23d8615611ba4747 commit cae175987c1f842d1f232f9f23d8615611ba4747 Author: Andrey Kosyakov <caseq@chromium.org> Date: Tue Apr 03 23:02:51 2018 DevTools: get frame name from frame tree, not owner element Bug: 823579 Change-Id: I05cc69fc17b4195a1404583ca6635b9fd0888eb7 Reviewed-on: https://chromium-review.googlesource.com/993972 Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Commit-Queue: Andrey Kosyakov <caseq@chromium.org> Cr-Commit-Position: refs/heads/master@{#547858} [modify] https://crrev.com/cae175987c1f842d1f232f9f23d8615611ba4747/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp
,
Apr 4 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by lukasza@chromium.org
, Mar 20 2018Components: Platform>DevTools