Adding TE-NeedsTriageHelp label, as we don't have the setup to check the issue from our end.

Thanks!!

It's possible that we could add a mode to disable --isolate-extensions for testing, but that would mean the testing wouldn't be representative of how the extension's iframes will behave in practice.  It seems better to find a way for Selenium to support iframes running in a different process if possible.

I'm not familiar with how Selenium works, but the example given above makes it sound like it uses a synchronous API to walk into cross-origin subframes, which probably won't be easy to support.  We'll chat some more to see if there's a way forward.

nasko@ mentions that Selenium uses Chrome Driver, which uses the DevTools protocol.  I think there was a similar request from the Chrome Driver folks as well.

dgozman@, do you know who we would ask about the protocol and whether there's any chance of having it support some kind of interactions with OOPIFs?

Disable isolate extension, although I agree would not be ideal, would at
least allow us to test the frame contents as part of the extension. As
chrome handles the coms between the frames, the risk is relatively small in
terms of the test not being representative of the way the application will
work outside of the test environment. For example we did not have to change
our code when Out-of_Process frames was introduced.
Being able to disable --isolate-extensions might be a relatively quick and
easy fix.

On 24 July 2017 at 18:41, cr… via monorail <monorail+v2.2436768650@
chromium.org> wrote:

Out-of-process iframes is a big architecture change which unfortunately(?) leaks through Remote Debugging Protocol. Any client who wants to talk to OOPIFs has to update. In this case ChromeDriver should support talking to OOPIFs. Assigning to John for ChromeDriver triage.

Target domain [1] in our protocol is designed to communicate with entities which leave out of process. To automatically connect to all OOPIFs, send Target.setAttachToFrames({value: true}) and Target.setAutoAttach({autoAttach: true, waitForDebuggerOnStart: false}). After that, notification Target.attachedToTarget will be issued for every out-of-process iframe inside the page. The sessionId parameter can be used for further communication with iframe using Target.sendMessageToTarget and Target.receivedMessageFromTarget. Iframe implements the same remote debugging protocol as the page itself. For example, one can send DOM.getDocument to start working with it's DOM.

Here is an approximate traffic over protocol:
-> {"id": 1, "method": "Target.setAttachToFrames", "params": {"value": true}}
-> {"id": 2, "method": "Target.setAutoAttach", "params": {"autoAttach": true, "waitForDebuggerOnStart": false}}
<- {"method": "Target.attachedToTarget", "params": {"sessionId": "<id>", "targetInfo": {"type": "iframe", ...}, "waitingForDebugger": false}}
Connection is established now, sending DOM.enable to the iframe:
-> {"id": 3, "method": "Target.sendMessageToTarget", "params": {"sessionId": "<id>", "message": "{\"id\":1,\"method\":\"DOM.enable\"}"}}
Receiving response for DOM.enable:
<- {"method": "Target.receivedMessageFromTarget", "params": {"sessionId": "<id>", "message": "{\"id\":1,\"result\":{}}"}}

Please see DevTools frontend code in TargetManager.js [2] for reference implementation, and ask any questions here.

[1] https://chromedevtools.github.io/devtools-protocol/tot/Target/
[2] https://chromium.googlesource.com/chromium/src/+/c70a24f50c547d0f93eaa86bbcf4fb344b449561/third_party/WebKit/Source/devtools/front_end/sdk/TargetManager.js#539

Looks like no one is currently actively working on this, despite its importance to Google. I may dig in, but I'm not particularly familiar with the codebase.

pfeldman@: Can you own this to make sure we're making progress towards the fix?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40126a62123c1e8704b2f92a6ef54eb3e6ce52db

commit 40126a62123c1e8704b2f92a6ef54eb3e6ce52db
Author: Caleb Rouleau <crouleau@chromium.org>
Date: Tue Jan 16 21:23:09 2018

[ChromeDriver] Add failing test for OOPIF.

I disabled the test, but it can be run locally with a command like
python run_py_tests.py --chromedriver=/path/to/chromedriver
 --filter=*testCanClickOOPIF --chrome=/usr/bin/google-chrome-stable

Bug:  746266 
Change-Id: Iafce9da5cc58f71df2b4ffb40d84e61ab87f6601
Reviewed-on: https://chromium-review.googlesource.com/865648
Reviewed-by: John Chen <johnchen@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529502}
[modify] https://crrev.com/40126a62123c1e8704b2f92a6ef54eb3e6ce52db/chrome/test/chromedriver/test/run_py_tests.py
[add] https://crrev.com/40126a62123c1e8704b2f92a6ef54eb3e6ce52db/chrome/test/data/chromedriver/cross_domain_iframe.html

johnchen@ is making progress on this in https://chromium-review.googlesource.com/c/chromium/src/+/934145.  (Thanks!)

If I understand correctly, this doesn't need to target a particular milestone since ChromeDriver can be released independently.  I think all the necessary support on Chrome's site should be present in Chrome 65.  That said, we would treat this as a blocker to enabling Site Isolation on Beta or Stable channels, so hopefully we can get the support in place soon (since we're hoping to experiment with Site Isolation in M66 Beta in issue 810843).

The following change fixes this issue, and will be released in upcoming ChromeDriver 2.36:

commit 69001d48045f7470bc435ce015e438209d7c8cc9
Author: John Chen <johnchen@chromium.org>
Date:   Tue Feb 27 17:50:37 2018 +0000

    [ChromeDriver] Support OOPIF
    
    Add support of OOPIF (out-of-process iFrame) to ChromeDriver
    
    Change-Id: I0b936b21597d8970656029c3110bfe0f61ae9aa8
    Reviewed-on: https://chromium-review.googlesource.com/934145
    Commit-Queue: John Chen <johnchen@chromium.org>
    Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#539493}

Thanks!  Much appreciated-- I'll mark this as fixed.

Do you know when that version will become available for verifying the new OOPIF support?

Also, if I understand correctly from the CL, this means that ChromeDriver 2.36 will support OOPIFs when used with Chrome 65 as well, which is fantastic.

Yes, the tests passed on Chrome 65 and 66.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/901b6309c1dbdf2bddc59cc0e857fe399273dd91

commit 901b6309c1dbdf2bddc59cc0e857fe399273dd91
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Fri Mar 02 02:30:39 2018

[DevTools] Route input events to the correct RenderWidgetHost

With OOPIFs, we route input events sent to main frame's target
to the correct RenderWidgetHost inside.

We might consider doing the same for non-main frames as well, or
instead prohibit input domain on them.

This does not touch gestures yet, those will follow in next patch.

Bug:  746266 

Change-Id: I7ba87c6d227ea24a559ebb7b5ca6a3cf9764d0f0
Reviewed-on: https://chromium-review.googlesource.com/939540
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540419}
[modify] https://crrev.com/901b6309c1dbdf2bddc59cc0e857fe399273dd91/chrome/browser/devtools/devtools_sanity_browsertest.cc
[add] https://crrev.com/901b6309c1dbdf2bddc59cc0e857fe399273dd91/chrome/test/data/devtools/oopif-input-iframe.html
[add] https://crrev.com/901b6309c1dbdf2bddc59cc0e857fe399273dd91/chrome/test/data/devtools/oopif-input.html
[add] https://crrev.com/901b6309c1dbdf2bddc59cc0e857fe399273dd91/chrome/test/data/devtools/oopif-input.js
[modify] https://crrev.com/901b6309c1dbdf2bddc59cc0e857fe399273dd91/content/browser/devtools/protocol/input_handler.cc
[modify] https://crrev.com/901b6309c1dbdf2bddc59cc0e857fe399273dd91/content/browser/devtools/protocol/input_handler.h
[modify] https://crrev.com/901b6309c1dbdf2bddc59cc0e857fe399273dd91/testing/buildbot/filters/mash.browser_tests.filter
[modify] https://crrev.com/901b6309c1dbdf2bddc59cc0e857fe399273dd91/third_party/WebKit/Source/devtools/front_end/Tests.js

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f3a123fb6f21d4371f066940ca605643248fea7e

commit f3a123fb6f21d4371f066940ca605643248fea7e
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Mon Mar 05 19:13:12 2018

[chromedriver] Do not rebase element coordinates to the neareset OOPIF

This is not needed in Chrome 66 anymore, since we now dispatch
events to the correct OOPIF via DevTools Protocol.

Bug:  746266 
Change-Id: Ieb8ff79f70c9c03345a8805758a910d1d2c1026d
Reviewed-on: https://chromium-review.googlesource.com/947091
Reviewed-by: Jonathon Kereliuk <kereliuk@chromium.org>
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540890}
[modify] https://crrev.com/f3a123fb6f21d4371f066940ca605643248fea7e/chrome/test/chromedriver/chrome/web_view_impl.cc
[modify] https://crrev.com/f3a123fb6f21d4371f066940ca605643248fea7e/chrome/test/chromedriver/element_util.cc

When will ChromeDriver 2.36 be available?  I don't see it yet on https://sites.google.com/a/chromium.org/chromedriver/.

John is OOO for a bit. I will work with his team here to see if we can get a release.

Planning on getting the release out by the end of the week. +Artur is working on the release. Still need to make sure that it fixes users' problems, so I am working on that.

You can download a nightly build of Chromedriver here: https://sites.google.com/a/chromium.org/chromedriver/chromedriver-canary and then you can see if it fixes problems.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/690a89ee621cde0c72fad9b20f10af9b59a4e7eb

commit 690a89ee621cde0c72fad9b20f10af9b59a4e7eb
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Tue Mar 06 01:25:31 2018

[DevTools] Route synthetic gestures to OOPIF's widget hosts

This follows mouse and touch events in input handler.

Bug:  746266 
Change-Id: I10e08dbc6af952e240e6426a145bf1257ccd93ae
Reviewed-on: https://chromium-review.googlesource.com/947470
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541019}
[modify] https://crrev.com/690a89ee621cde0c72fad9b20f10af9b59a4e7eb/content/browser/devtools/protocol/input_handler.cc
[modify] https://crrev.com/690a89ee621cde0c72fad9b20f10af9b59a4e7eb/content/browser/devtools/protocol/input_handler.h

Requesting the merge of https://chromium.googlesource.com/chromium/src.git/+/901b6309c1dbdf2bddc59cc0e857fe399273dd91 (from #c23) and https://chromium.googlesource.com/chromium/src.git/+/690a89ee621cde0c72fad9b20f10af9b59a4e7eb (from #c28) to M66. This will make Chromedriver work with version 66.

Your change meets the bar and is auto-approved for M66. Please go ahead and merge the CL to branch 3359 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58b43c8ca2941ca6c5ece913b1841bc278e9bbf1

commit 58b43c8ca2941ca6c5ece913b1841bc278e9bbf1
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Wed Mar 07 18:53:48 2018

[DevTools] Route input events to the correct RenderWidgetHost

With OOPIFs, we route input events sent to main frame's target
to the correct RenderWidgetHost inside.

We might consider doing the same for non-main frames as well, or
instead prohibit input domain on them.

This does not touch gestures yet, those will follow in next patch.

Bug:  746266 

TBR=dgozman@chromium.org

(cherry picked from commit 901b6309c1dbdf2bddc59cc0e857fe399273dd91)

Change-Id: I7ba87c6d227ea24a559ebb7b5ca6a3cf9764d0f0
Reviewed-on: https://chromium-review.googlesource.com/939540
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#540419}
Reviewed-on: https://chromium-review.googlesource.com/953163
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#66}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/58b43c8ca2941ca6c5ece913b1841bc278e9bbf1/chrome/browser/devtools/devtools_sanity_browsertest.cc
[add] https://crrev.com/58b43c8ca2941ca6c5ece913b1841bc278e9bbf1/chrome/test/data/devtools/oopif-input-iframe.html
[add] https://crrev.com/58b43c8ca2941ca6c5ece913b1841bc278e9bbf1/chrome/test/data/devtools/oopif-input.html
[add] https://crrev.com/58b43c8ca2941ca6c5ece913b1841bc278e9bbf1/chrome/test/data/devtools/oopif-input.js
[modify] https://crrev.com/58b43c8ca2941ca6c5ece913b1841bc278e9bbf1/content/browser/devtools/protocol/input_handler.cc
[modify] https://crrev.com/58b43c8ca2941ca6c5ece913b1841bc278e9bbf1/content/browser/devtools/protocol/input_handler.h
[modify] https://crrev.com/58b43c8ca2941ca6c5ece913b1841bc278e9bbf1/testing/buildbot/filters/mash.browser_tests.filter
[modify] https://crrev.com/58b43c8ca2941ca6c5ece913b1841bc278e9bbf1/third_party/WebKit/Source/devtools/front_end/Tests.js

dogzman@: Thanks for the followup CLs!  It sounds like you're merging r541019 to M66 as well?

Just to check, if we wanted ChromeDriver to work with OOPIFs in M65, do any of these CLs have to be merged there as well?  Or does M65 work as is (which was my earlier impression)?

Also, I see r540890 from comment 24 says it's about M66 but it landed in M67.  Does that need to be merged to M66 as well, or are we functional without it?

> dogzman@: Thanks for the followup CLs!  It sounds like you're merging r541019 to M66 as well?
Yeah, I'll give it one more day in Canary and merge tomorrow.

> Just to check, if we wanted ChromeDriver to work with OOPIFs in M65, do any of these CLs have to be merged there as well?
> Or does M65 work as is (which was my earlier impression)?
M65 should work as is (to some extent), since I've left a special codepath for it in Chromedriver's code.

> Also, I see r540890 from comment 24 says it's about M66 but it landed in M67.
> Does that need to be merged to M66 as well, or are we functional without it?
That patch is Chromedriver-only, which has a separate release schedule, so no need to merge it anywhere.

Thanks, sounds perfect!

crouleau@ / khachatryan@: Thanks for the update on the ChromeDriver testing and release in comment 27!  Please also check whether this week's release needs to include r540890 from comment 24, per dgozman's comment 33.

 Issue chromedriver:1366  has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e889dff2f80b0cd6572d427db92e89219a9335f1

commit e889dff2f80b0cd6572d427db92e89219a9335f1
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Mon Mar 12 19:10:07 2018

[DevTools] Route synthetic gestures to OOPIF's widget hosts

This follows mouse and touch events in input handler.

TBR=dgozman@chromium.org

(cherry picked from commit 690a89ee621cde0c72fad9b20f10af9b59a4e7eb)

Bug:  746266 
Change-Id: I10e08dbc6af952e240e6426a145bf1257ccd93ae
Reviewed-on: https://chromium-review.googlesource.com/947470
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#541019}
Reviewed-on: https://chromium-review.googlesource.com/959357
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#173}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/e889dff2f80b0cd6572d427db92e89219a9335f1/content/browser/devtools/protocol/input_handler.cc
[modify] https://crrev.com/e889dff2f80b0cd6572d427db92e89219a9335f1/content/browser/devtools/protocol/input_handler.h

Unfortunately currently ChromeDriver does not use Chrome's release model. Instead, ChromeDriver simply releases from ToT whenever ChromeDriver devs and testers decide to. See https://bugs.chromium.org/p/chromedriver/issues/detail?id=2080 . So merging r540890 forward would have no impact on what we are shipping.

Comment 37: Understood.  Just to check, is r540890 included in ChromeDriver 2.36 (which I see is now available at https://sites.google.com/a/chromium.org/chromedriver/)?  If not, do we need a new release, or is that CL not critical for OOPIF support?  Thanks!

It should be in there since I started asking Artur to do the release after my comment #26. Artur would have used ToT Chromium to do the build. Unfortunately ChromeDriver notes ( https://chromedriver.storage.googleapis.com/2.36/notes.txt ) do no provide a revision number.

...

I just checked, and I'm no longer sure.

the command
$ google3/third_party/browser_automation/chromedriver/linux/v2_36/chromedriver --version

prints

"ChromeDriver 2.36.540471 (9c759b81a907e70363c6312294d30b6ccccc2752)"

I think the last part of the ChromeDriver version "540471" refers to a revision number (and the hash refers to the git hash from that revision), but it doesn't make sense that r540471 would be what Artur released since that is from March 2nd. I also remember John telling me that the ChromeDriver version's revision number is misleading for some reason.

John is back in office tomorrow, so let's just wait for his help unless Artur knows what build Artur released.

We don't add Chromedriver revision number in notes file.
I followed the standard procedure which we always do for Chromedriver release. The global presubmit I run when John was here. After that I just updated the Chromedriver release notes at http://chromedriver.chromium.org/.

I spoke with Artur and it sounds like the build was from before March 5th because it was a build that John gave him, bot a build that he made himself. So r540890  is not in the build.

I have no idea whether or not we need r540890. Maybe Dmitry can tell us.

r540890 is essential to work with v66 and onwards. I was under impression that Artur built a new version around the time #c27 was posted.

I could do a new global presubmit for the latest chromedriver binaries but I prefer to do that tomorrow when John is here.

Artur, we are not talking about global presubmit.

Dmitry, I was also under that impression, but Artur told me that he used a build that John gave him instead of building one himself. Sounds like we need to do a new release. John will be back tomorrow and he can guide it through.

I can build chromedriver from the repo if that is what you want to do. 

Here is the version we have on https://chromedriver.storage.googleapis.com/index.html?path=2.36/

ChromeDriver 2.36.540471 (9c759b81a907e70363c6312294d30b6ccccc2752)


Annd just FYI:
on Goggle3 we were running chromedriver 2.35 with and without changes mentioned in https://bugs.chromium.org/p/chromium/issues/detail?id=746266#c19 ==>   (changes https://chromium-review.googlesource.com/c/chromium/src/+/934145 )

Thanks for looking into this in more detail.  I think it will mainly matter when the beta channel moves to Chrome 66 later this week (likely Thursday or Friday), at which point ChromeDriver users may start running with it.  (IIUC, ChromeDriver doesn't support Dev channel.)

That means it should be fine to wait for John to help with the release.  Once it's verified, we should make sure that it's available in Google3 as well.  That will hopefully clear the path for turning off the enterprise policy workaround that disables Site Isolation when running with --enable-automation, which is the goal.

Thanks all!

John is doing the release right away but waiting for  issue 821862  so that he can make sure Android is working.