A logical culprit would be https://chromium-review.googlesource.com/c/chromium/src/+/1071713, as likely it's the TabLoader that's derailing or missing a notification at some point.

I'm having a bit of trouble building the version above, in the meantime I've verified that the TabLoader thinks it's done. It's deleting itself, leaving the remaining tabs in limbo. If I go to chrome://discards and unload/load them, they load just fine. Investigating moar tomorrow - hopefully it won't be melting hot in the office...

The TabLoader is getting destroyed at this call stack, BTW: 

 # Call Site
00 chrome!TabLoader::~TabLoader
01 chrome!TabLoader::`vector deleting destructor'
02 chrome!TabLoader::OnLoadingStateChange
03 chrome!resource_coordinator::TabLoadTracker::TransitionState
04 chrome!resource_coordinator::TabLoadTracker::MaybeTransitionToLoaded
05 chrome!resource_coordinator::PageSignalReceiver::NotifyObserversIfKnownCu<void (resource_coordinator::PageSignalObserver::*)(content::WebContents *, const resource_coordinator::PageNavigationIdentity &)>
06 chrome!resource_coordinator::mojom::PageSignalReceiverStubDispatch::Accept
07 chrome!mojo::internal::MultiplexRouter::ProcessIncomingMessage
08 chrome!mojo::internal::MultiplexRouter::Accept
09 chrome!mojo::Connector::ReadSingleMessage

Looks like https://chromium-review.googlesource.com/c/chromium/src/+/1071713 introduced this - I repro 100% with that CL, and 0% with the immediately prior version.

This landed initially in 69.0.3451.0, see https://storage.googleapis.com/chromium-find-releases-static/6b0.html#6b0f670152a7ab4440512d2370b9c40c8d66091a.

Maybe there's a second issue - I can repro about 5% with the previous change, still 100% with it in.

Mkay, somehow the afflicted WebContentsImpl instances are ending up with needs_reload_ == false, where they clearly should have needs_reload_ == true, as they've never been navigated.

Huh, looks like there has to be a pre-existing race that's aggravated by this change. I see all the tabs navigated by the TabLoader, and I've followed through to content::NavigatorImpl::Navigate so far. Something's blocking or stalling the navigation from that point.

It's not navigation throttles, I'm getting hits on NavigationRequest::OnRequestStarted - looks like there's some form of throttling or something is erring out below the loader.

I don't know what's up here, but chrome://net-internals will have one of these for each hung tab: 

139: URL_REQUEST
chrome://version/
Start Time: 2018-07-04 14:44:19.537

t=0 [st=0] +REQUEST_ALIVE  [dt=?]
            --> has_upload = false
            --> is_pending = true
            --> load_flags = 2052 (MAIN_FRAME_DEPRECATED | SKIP_CACHE_VALIDATION)
            --> load_state = 0 (IDLE)
            --> method = "GET"
            --> net_error = -1 (ERR_IO_PENDING)
            --> status = "IO_PENDING"
            --> url = "chrome://version/"

If I set up a session with 4 or more tabs navigated to chrome://settings/help I will end up with a mishmash of fully, partially and unloaded tabs. There will be chrome:// requests that seem to be hanging at various stages, here's a sample:

122: URL_REQUEST
chrome://settings/help
Start Time: 2018-07-04 15:01:13.448

t=424 [st= 0] +REQUEST_ALIVE  [dt=?]
               --> priority = "HIGHEST"
               --> url = "chrome://settings/help"
t=424 [st= 0]    NETWORK_DELEGATE_BEFORE_URL_REQUEST  [dt=0]
t=424 [st= 0]    URL_REQUEST_START_JOB  [dt=0]
                 --> load_flags = 2052 (MAIN_FRAME_DEPRECATED | SKIP_CACHE_VALIDATION)
                 --> method = "GET"
                 --> url = "chrome://settings/help"
t=424 [st= 0]   +URL_REQUEST_DELEGATE_RESPONSE_STARTED  [dt=21]
t=424 [st= 0]      DELEGATE_INFO  [dt=21]
                   --> delegate_blocked_by = "MojoAsyncResourceHandler"
t=445 [st=21]   -URL_REQUEST_DELEGATE_RESPONSE_STARTED

I can't repro this on non-chrome: URLs, so I think it has to be a WebUI-only thing. Looks like somehow the URLRequestChromeJobs are getting wedged when multiple tabs are loading them at the same time.
There's an implicit threading model there that I can't infer - likely aggressive session restore is provoking a latent race.

Demetrios, can you please triage and redirect?

> Demetrios, can you please triage and redirect?

Reading through this discussion, this part of the code is totally outside my area of expertise (too low level), so I have no insight on what is happening, nor do I know who to redirect to. Assigning back to crisha per comment #4.

Over to clamy@ who works on navigation and loading these days. Any ideas what could be up here?

-clamy@, as I don't think she needs to chime in on this.

Okay... more spelunking here.

It turns our that there's a race here. Every time a request is made to a chrome:// URL that uses the new WebUIDataSource, a instance of the data source is created. For example, in chrome://discards, this happens here:

https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/discards/discards_ui.cc?type=cs&g=0&l=217

The call to content::WebUIDataSource::Add bottoms out in URLDataManagerBackend::AddDataSource, which can be found here:

https://cs.chromium.org/chromium/src/content/browser/webui/url_data_manager_backend.cc?type=cs&q=URLDataManagerBackend&g=0&l=430

If a WebUIDataSource already exists for the given URL scheme ("discards", in this case), it first nulls out the |backend_| value of the corresponding URLDataSourceImpl.

Meanwhile, the request for the chrome://discards page is being asynchronously served over on the UX thread. When this completes the chain is as follows:

- URLDataSourceImpl::SendResponse gets called on the UI thread.
- URLDataSourceImpl::SendResponseOnIOThread gets called on the IO thread.

If the existing request didn't finished before the new one was posted the URLDataSourceImpl has had its |backend_| value nulled out in the meantime. Which means that the notification of data being ready to read simply gets dropped, by this code:

https://cs.chromium.org/chromium/src/content/browser/webui/url_data_source_impl.cc?type=cs&q=URLDataSourceImpl::SendResponseOnIOThread&g=0&l=57

So the requests sits in limbo forever and will never be served.

If I null out the "backend_ = nullptr" code then this issue goes away. However, I don't know enough about the URLDataManagerBackend to know if this is appropriate or not.

It looks like the ChromeURLDataManagerBackend code was originally added by sky@, so redirecting over there to see if he has any ideas.

(In the meantime, I'll keep poking around to see if I can divine the intent of that code, or whether or not a simple fix is safe.)

Regarding the fact that a WebUIDataSource is replaced when multiple WebUI pages on the same URL are opened, it is a known issue. AFAIK this has not been causing problems so far. In a few cases an extra step is taken to ensure this does not cause issues, see [1] for usages of WebUIDataSource::Update and specifically the comment at [2] (No volatile data should be placed on the WebUIDataSource).

Perhaps this is being violated here?


[1] https://cs.chromium.org/search/?q=WebUIDataSource::Update+-file:third_party+-file:infra+-file:out/Debug+-file:out/chromium-android/Debug+-file:src/v8+-file:src/native_client-sdk&type=cs
[2] https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/extensions/extensions_ui.cc?l=359-360

Near as I can tell it's being violated for every single chrome:// URL that uses WebUIDataSource. It's not necessarily the fact that they use volatile data, it's simply that every request causes a new data source to be created, effectively abandoning previous requests. You can create the same behavior by opening up multiple instances of chrome://settings, or a long list of others.

Looking at calls to WebUIDataSource::Create, basically any web UI that initializes this in the constructor (instead of only initializing it once upon first use for an entire browser session) can suffer from this particular behavior.

From code search it looks like there are at least many dozens of this occurring, but each case would have to be combed through one by one.

https://cs.chromium.org/search/?q=%22WebUIDataSource::Create(%22+file:_ui.cc&sq=package:chromium&type=cs

Even better would be to only create the new WebUI if needed. Unfortunately, the creation happens on the UX thread and the addition on the IO thread, so there's no way (without a thread hop) to know if one already exists, without some type of atomic counter that can be read across threads. Another fix for this is for all WebUIDataSources to return "false" for ShouldReplaceExistingSource. In which case the newly created source will be dropped and self-destroy if another one already exists. This causes churn in creating/destroying the sources, but keeps continuity of a single source.

Another fix is not nulling out the previous source when adding a new one. The sources themselves are refcounted and kept alive by outstanding queries to them, so I don't see the immediate harm. That being said, there are likely subtleties here that I'm not aware of.

Assigning back to me as sky@ doesn't have the right context. Conversation moved to email for now, will loop back once resolved.

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

commit bed9bc8ab47755b9be2032dc398af2060bc94ede
Author: Chris Hamilton <chrisha@chromium.org>
Date: Wed Jul 11 23:13:35 2018

Make URLDataSource backend reference a WeakPtr.

This allows URLDataSources that have been detached from a backend to
still refer to the backend if it still exists, allowing any pending
queries that were issued to that backend and routed to the data source
to resolve back to the backend when they are finished.

BUG= 859193 

Change-Id: I55eda45be1f24716ad31c7d98ee748a737baca51
Reviewed-on: https://chromium-review.googlesource.com/1131537
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574398}
[modify] https://crrev.com/bed9bc8ab47755b9be2032dc398af2060bc94ede/content/browser/webui/url_data_manager_backend.cc
[modify] https://crrev.com/bed9bc8ab47755b9be2032dc398af2060bc94ede/content/browser/webui/url_data_manager_backend.h
[modify] https://crrev.com/bed9bc8ab47755b9be2032dc398af2060bc94ede/content/browser/webui/url_data_source_impl.h