Debugging site like becu.org is hard, since it requires having credentials that we might not have. Is it possible to capture and share chrome://tracing trace with "navigation" category enabled? It will be useful to have a working and broken case trace, which will allow us to compare the two and see if there is a difference we can spot.

rpop@ - Could you please provide trace from chrome://tracing as per comment #1.

Thanks...!!

I tried. I cleared my cookies to get a clean repro w/ security question, and now it won't let me load the login page - --site-per-process or not. I think canary is in a weird state because stable works. I'll try again on another machine tomorrow.

Thank you for providing more feedback. Adding requester "krajshree@chromium.org" to the cc list and removing "Needs-Feedback" label.

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

I've tried this out, since I also have a becu account.  I couldn't reproduce the main problem of an unfocusable text field on the security question page, because becu was logging me in without a security question altogether (I do have them set up), even in a clean profile or incognito.  I might try this from another network later, but otherwise I'm able to login successfully.

I did, however, consistently reproduce the issue with the page never finishing loading.  The spinner just spins indefinitely on the login page, and the status bar is stuck at "Waiting for ad.doubleclick.net...".  This happens even before trying to log in, so anyone can repro this.  Without --site-per-process, the page finishes loading properly, so we should fix this.

I see that this page has two OOPIFs from https://doubleclick.net/ and https://krxd.net/, which is probably related.

Attaching trace for the infinite spinner issue.

Deleted: trace_becu.json.gz 5.4 MB

trace_becu.json.gz 5.4 MB Download

Nasko mentioned he might be able to poke at the trace to see if it's related to a known PlzNavigate issue when a subframe is removed while loading, so assigning to him for now.

Here's a successful trace without --site-per-process for comparison.

Deleted: trace_becu_success.json.gz 2.5 MB

trace_becu_success.json.gz 2.5 MB Download

Assigning to clamy@ to help investigating as per offline discussion, since there is a known race condition with DidStopLoading and PlzNavigate. I could not find an evidence of how it happens in this specific set of traces, but just going to the login page itself does leave the tab spinner spinning indefinitely, so it should be reliable repro for the time being.

AFAICS this is not the issue of the iframe being deleted.
Instead we have an OOPIF loading  https://cdn.krxd.net/partnerjs/xdi/proxy.3d2100fd7107262ecb55ce6847f01fa5.html#!kxcid=r7b1vj9ny&kxt=https%3A%2F%2Fonlinebanking.becu.org&kxcl=cdn&kxp= which succeeds.
Then it tries to commit a navigation to the same URL again, except this navigation never commits, and we don't get a DidStopLoading IPC either -> note that we do get the IPC if we actually close the browser, so I suspect something weird is happening in the renderer process.

There is also a difference between site-per-process and non site-per-process around this URL:
In non site-per process mode, we get:
1) RFH asks the renderer process to commit the cdn URL
2) RFH gets the DidCommitProvisionalLoad for the cdn URL
3) RFH gets a second DidCommitProvisionalLoad for the same cdn URL (but without the request to commit so it must be a same-document navigation?)

With site-per-process we get:
1) RFH asks the renderer process to commit the cdn URL
2) RFH gets the DidCommitProvisionalLoad for the cdn URL
3) RFH asks the renderer process to commit the cdn URL again and we never get the second commit.

This is a bit weird, will investigate some more tomorrow.

So finally got to the root cause of the issue: we drop a same-document navigation in a subframe.

In normal mode, what happens is the following:
1) A frame wants the subframe to navigate to the same URL it's already on. This goes into blink's FrameLoader which classifies it as a same-document navigation.
2) We commit the same document navigation.

In site-per-process mode, the following happens:
1) A frame wants the cross-process subframe to navigate to the same URL it's already on.
2) This goes through RFProxyHost::OpenURL, and the browser-initiated navigation path.
3) We detect that it is a same-document navigation, and ask the renderer process to commit it as a same-document navigation (normal load).
4) In FrameLoader, we recompute the load type and assign it a reload type (https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/loader/FrameLoader.cpp?gsn=ShouldPerformFragmentNavigation&l=696), because the URL is the same (and we don't have an origin document since the initiator is cross-process).
5) Since the load type is now reload, we cannot treat it as a same-document navigation. As the browser mandated that it is treated as a same-document navigation, but we can't do it, we silently drop the navigation: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/loader/FrameLoader.cpp?gsn=ShouldPerformFragmentNavigation&l=958.

So I'm thinking we shouldn't recompute the load type for navigations the browser asked us to commit, and the renderer process should inform the browser process it dropped the same document navigation the browser process wanted the renderer process to commit.

Note that this will be improved with better interfaces around navigation and loading state.

I have a test for the issue at https://chromium-review.googlesource.com/c/chromium/src/+/814534. Note that for the issue to reproduce, the URL we navigate to needs to have a fragment. On the browser-side we classify a navigation to the same URL as different document if the URL has no fragment, but same-document if it has. I'm not sure this makes sense, though blink behaves similarly, so I will leave it as is.

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

commit af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394
Author: clamy <clamy@chromium.org>
Date: Tue Feb 06 10:54:36 2018

Introduce a mojom::Frame::CommitSameDocumentNavigation

This CL introduces a CommitSameDocumentNavigation method in mojom::Frame, along
with a specific path to handle it in FrameLoader. Same document navigations are
handled very differently from different document navigations in the renderer,
and this simplifies the code path for their handling when the same-document
navigation is browser-initiated. This is a preparatory work for introducing
per-navigation Mojo interfaces. These interfaces will only be used for
different document navigations. Same-document navigations will still be handled
by the Frame directly, through the IPC this CL introduces.

This CL also fixes an issue with same-document navigations in out-of-process
iframes. Browser-initiated same-document navigations would see their
FrameLoadType recomputed at commit type, which sometimes caused them to be
wrongly classified as non same-document and dropped. By taking a same-document
path from the start, we ensure that FrameLoader doesn't recompute the
FrameLoadType for same-document navigations.

Bug:  788901 ,784904
Change-Id: Ie2fbef9afd9c99d22b7bb6d33170eaa5e32de622
Reviewed-on: https://chromium-review.googlesource.com/817296
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534675}
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/DEPS
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigation_request.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigator.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigator_impl.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/common/frame.mojom
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/renderer/render_frame_impl.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/test/test_render_frame_host.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.cpp
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/Source/core/loader/FrameLoader.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/public/web/WebLocalFrame.h
[add] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/public/web/commit_result.mojom