extensionview cannot do cross extension navigation with OOPIF based guests |
||||
Issue descriptionChrome Version: 65.0.3316.0 As seen in the currently disabled test ExtensionViewTest.LoadAPICall, the attempt to navigate an extensionview from one extension to another does not work with OOPIF based guests. Note that the test passes for non-OOPIF based guests. Note that extensionview is an internal API and the only thing that appears to use it at the moment is the WebUI for chrome://cast which does not navigate between extensions. So this does not appear to be an issue at the moment.
,
Jan 9 2018
,
Jan 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e813105cc50038870c18ccb7e6525a6106e1928b commit e813105cc50038870c18ccb7e6525a6106e1928b Author: Kevin McNee <mcnee@chromium.org> Date: Wed Jan 10 16:00:16 2018 Rewrite extension_view/load_api tests to not have large multi-step tests These tests have been disabled since 2015. There were only two tests, each with multiple steps. So due to a failure of one part of each test, we lost all coverage. We refactor these tests so that they can be run individually. The parts of the tests that were failing before are disabled individually, namely the tests for navigating to a new extension. Bug: 800407 , 545656 , 512092 , 538114 Change-Id: I8b30fa861bd70174fc78117b3affebaf12c623e1 Reviewed-on: https://chromium-review.googlesource.com/857594 Reviewed-by: James MacLean <wjmaclean@chromium.org> Commit-Queue: Kevin McNee <mcnee@chromium.org> Cr-Commit-Position: refs/heads/master@{#528327} [modify] https://crrev.com/e813105cc50038870c18ccb7e6525a6106e1928b/chrome/browser/apps/guest_view/extension_view/extension_view_browsertest.cc [modify] https://crrev.com/e813105cc50038870c18ccb7e6525a6106e1928b/chrome/test/data/extensions/platform_apps/extension_view/load_api/main.js
,
Jan 10 2018
I suspect that this is not extensionview specific. I modified AppViewTest.TestAppViewMultipleConnects so that <appview>.connect calls wait until the previous connect completes and the same issue occurred where the render frame was null for the second attach. Like the extensionview cross extension navigation case, <appview>.connect destroys and recreates the guest. I also notice that when we access the iframe's contentWindow for the first attach, the |content_frame_| is local, whereas on the second attach, the |content_frame_| is remote. Perhaps there's some cleanup during detachment that's not happening in the OOPIF based guest case.
,
Jan 10 2018
Interesting findings. The RenderFrame we are trying to get is the one used for guest view, right? So if the frame is remote for the second "guest" navigation, it is quite natural to get nullptr for RenderFrame given that none exists in the same process (embedder process). Perhaps we need to detach and reattach the <iframe> before navigating to the new guest?
,
Jan 10 2018
Right, in the guest_view_iframe.js implementation of |attachImpl$|, the contentWindow of the internal iframe element is used to identify the RenderFrame of the iframe element, which determines the |embedder_local_frame_routing_id| sent in |GuestViewHostMsg_AttachToEmbedderFrame|. So it certainly looks like the renderer only knows how to replace a local frame. So when the guest is detached, we should presumably have local frame swapped back for the iframe. I'm not entirely sure how detachment works in OOPIF based guests. In |destroyImpl|, we call |DetachGuest| but that appears to only actually do something in the BrowserPlugin case. Another observation: In the callback in |attachImpl$|, we access the contentWindow again now that it's attached and it's still local. That doesn't seem right. I played around with an OOPIF and tried getting a reference to a local frame's contentWindow, navigating cross site, and then using the reference to access the document and it was correctly denied. So I guess contentWindow references can go from local to remote, and I assume that's what's happening here. It still seems a bit strange that we acknowledge the attachment before the remote frame is actually in place.
,
Jan 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec31db29d7a9a2df8e93a1b115326c74b4333f19 commit ec31db29d7a9a2df8e93a1b115326c74b4333f19 Author: Kevin McNee <mcnee@chromium.org> Date: Mon Jan 29 18:17:35 2018 OOPIF guest view: Allow a GuestViewContainer to be reattached. In the OOPIF-based guest implementation of attachment, the contentWindow of the internal iframe element is used to identify the local frame to swap for a remote frame. When the guest is destroyed, we currently don't "reset" the internal iframe element so that it can be used for another attachment. When we try to attach, the frame is still remote, which causes GuestViewInternalCustomBindings::AttachIframeGuest to be unable to find the frame to swap. We now replace our internal iframe element upon guest destruction, so that the frame is local when we reattach. Bug: 800407 Change-Id: I4424793c7c0230d62a424765bc3c288080b2e0b9 Reviewed-on: https://chromium-review.googlesource.com/871292 Reviewed-by: Paul Meyer <paulmeyer@chromium.org> Commit-Queue: Kevin McNee <mcnee@chromium.org> Cr-Commit-Position: refs/heads/master@{#532495} [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/chrome/browser/apps/guest_view/app_view_browsertest.cc [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/chrome/browser/apps/guest_view/extension_view/extension_view_browsertest.cc [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/chrome/test/data/extensions/platform_apps/app_view/shim/main.js [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/chrome/test/data/extensions/platform_apps/extension_view/load_api/main.js [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/extensions/renderer/resources/guest_view/app_view/app_view.js [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/extensions/renderer/resources/guest_view/extension_options/extension_options.js [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/extensions/renderer/resources/guest_view/extension_view/extension_view.js [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/extensions/renderer/resources/guest_view/guest_view_container.js [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/extensions/renderer/resources/guest_view/guest_view_iframe.js [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/extensions/renderer/resources/guest_view/guest_view_iframe_container.js [modify] https://crrev.com/ec31db29d7a9a2df8e93a1b115326c74b4333f19/extensions/renderer/resources/guest_view/web_view/web_view.js
,
Jan 29 2018
Also note that the attachment ack order issue I mentioned in c#6 is fixed in https://chromium-review.googlesource.com/c/chromium/src/+/868277 |
||||
►
Sign in to add a comment |
||||
Comment 1 by mcnee@chromium.org
, Jan 9 2018