(this is on android, btw)

Thanks!  wjmaclean@ or kenrb@, can you help triage this?

> can no longer interact with the input fields

sorry, that's terrible wording. I mean clicking on the input fields no longer brings up the soft keyboard.

+ fsamuel@

Could this be related to the new hit-testing and the fact that frame sync isn't available yet on Android? (cf.  Issue 809122 )

+riajiang@

the input fields are still highlighted, and I still see the cursor move between input fields when I click on them. So I don't think it's hit testing.

I suspect it's the connection to android's software keyboard code. Haven't looked too closely yet though

> Could this be related to the new hit-testing

I'm assuming that you are referring to Viz-assisted hit-testing? That's still behind a flag

Maybe related to  Issue 817392 ?

The problem is the new RenderWidgetHostViewAndroid never got the ImeAdapterAndroid, so it's early returning here: https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_view_android.cc?rcl=21715f31b594a60d4dccc97866039332cc12ef2e&l=486

Which means RenderWidgetHostConnector probably didn't notice the newly recreated RWHVA. Although.. RenderWidgetHostConnector was only designed to work with main widgets/views, not subframes, yet input connection works before restore in the iframe. So umm... not sure exactly what's supposed to happen there.

Afaict, ImeAdapterAndroid always talks to the main RWHVA, so using RenderWidgetHostConnector is ok there.

But apparently, restoring the subframe caused WebContentsImpl::NotifyMainFrameSwappedFromRenderManager, which then caused RenderWidgetHostConnector to swap to the new RVH that doesn't have a RWHVA. NotifyMainFrameSwappedFromRenderManager happening for a subframe load seems pretty surprising to me, but I'm only beginning to learn about things in this area.

Removing this "very ugly hack" fixes this:
https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_host_manager.cc?rcl=fdbb173a345930127fa1944748daacd34bdd063f&l=631

Comments 9-11: Nice.  That's this code that Nasko added for the extension ProcessManager:

    if (navigation_rfh == render_frame_host_.get()) {
      EnsureRenderFrameHostVisibilityConsistent();
      EnsureRenderFrameHostPageFocusConsistent();
      // TODO(nasko): This is a very ugly hack. The Chrome extensions process
      // manager still uses NotificationService and expects to see a
      // RenderViewHost changed notification after WebContents and
      // RenderFrameHostManager are completely initialized. This should be
      // removed once the process manager moves away from NotificationService.
      // See https://crbug.com/462682.
      delegate_->NotifyMainFrameSwappedFromRenderManager(
          nullptr, render_frame_host_->render_view_host());
    }

Fixing issue 462682 may indeed help, though I haven't looked to see why that extra notification is causing problems in the Android case and whether that itself is a problem as well.

> Fixing issue 462682 may indeed help, though I haven't looked to see why that extra notification is causing problems in the Android case and whether that itself is a problem as well.

Been trying to understand that as well, which lead me down the path of learning about RenderFrameHostProxy again.

So iiuc, there are many RenderViewHost/RenderWidgetHost for the same FrameTreeNode, potentially one for each SiteInstance in the tree? Only the one for the SiteInstance that actually contains that frame is the "real" one, and has a RenderWidgetHostView created for it. The other ones are only created for RFHProxy's, and aren't fully initialized (at least they don't have RWHVs)?

The problem with that extra swap call is because it's swapping to a RenderViewHost that's created for a RFHProxy, so there is no RWHV. So then in this android code:
https://cs.chromium.org/chromium/src/content/browser/android/render_widget_host_connector.cc?rcl=9b0bf72c7e1391f124af4e7a34a810673d5fa0ec&l=62

It can't find a RWHVA from |new_host|, hence the broken connection causing this issue.

So anything that uses RenderWidgetHostConnector will be broken. Eg it crashes with a null pointer if I try to select text.

Confirmed this is still not working in Android canary 71.0.3564.0.  Probably a blocker for issue 841572 if we decide to reload crashed frames.  I've noticed sites often reload crashed ads subframes on their own (e.g., nytimes.com frequently does this), and this bug might raises issue in that case as well.

Looking over previous comments, one thing I don't understand is for the hack from #12, it seems that we are calling NotifyMainFrameSwappedFromRenderManager() for both main frames and subframes.  For subframes, that seems wrong -- it'll dispatch RenderViewHostChanged() for WebContentsObservers and provide the subframe's old and new RVH (which will both be inactive), probably messing up state since I think RenderViewHostChanged() is supposed to be for visible/main frame RVH changes only.  So maybe we should not call NotifyMainFrameSwappedFromRenderManager for subframes?

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

commit f818f2eff08cb7edd72413a7c5b908297f27c402
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Thu Oct 04 18:33:40 2018

Don't call NotifyMainFrameSwappedFromRenderManager for subframes.

NotifyMainFrameSwappedFromRenderManager was added in
https://codereview.chromium.org/950223006, but given its usage (and
its name), it should be only called for main frames.  Calling it on
subframes can lead to incorrectly dispatching RenderViewHostChanged
for a change to a subframe's RVH, which may be a swapped-out RVH for
an OOPIF.  RenderViewHostChanged should only be be dispatched for a
main frame's visible RenderViewHost.

Bug:  817606 
Change-Id: Ia47e6a8fe188866f9f26a205c42ad27ff9ff2b23
Reviewed-on: https://chromium-review.googlesource.com/c/1260489
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596770}
[modify] https://crrev.com/f818f2eff08cb7edd72413a7c5b908297f27c402/content/browser/frame_host/render_frame_host_manager.cc

Verified that r596770 fixed this on Android canary 71.0.3571.0.