Thanks!  Same for http://wg21.link/concepts.  Looks like PlzNavigate is failing to follow the redirects from the https://wg21.link/ redirect service.  (It starts loading, then stops kind of like a 204.)

clamy@, can you investigate?

I can't repro on 61.0.3163.91 on Linux. I get the final pdf for both links just fine. I will try on Mac and Linux when I have access to a machine (tonight).

Comment 2: I don't have M61 stable on Linux to test on, but it repros for me on 63.0.3218.0 (Dev) and on a trunk build from last week with --enable-browser-side-navigation.  Does it repro on trunk for you?

I looked into this and it looks like the navigation is being cancelled after the redirect. It isn't clear to me yet what is causing this. I also did a bisect of the behavior and it came back to https://chromium.googlesource.com/chromium/src/+log/31a45636e84fdf6cf56cac35ece3604baddf9174..bd3351963fb4da3e31478d841b91a117583f9786. Looking through the range, the likely commit is ea2f911 "PlzNavigate: make NavigationResourceHandler a LayeredResourceHandler".

Nasko and I just found that this bug affects *all* redirects to PDFs.  This is fairly serious, and could affect many more cases.  Re-titling the bug as a result (was: wg21.link/standard doesn't load with PlzNavigate).  Redirects to HTML documents still work.

Nasko confirmed it's from r468692, and he's looking closer to understand why.

A quick update on how far I've investigated. I tried to find why things fail and what is the difference between loading a PDF in the main frame directly, which works, and redirect to PDF in the main frame, which fails. Both codepaths seem identical until we reach the renderer process and call into FrameLoader::Load. In the redirect case FrameLoader doesn't seem to actually perform the load, where in the direct navigation case it does. My hunch is that some parameters aren't the same (possibly the mime type or similar), so FrameLoader doesn't know to kick off the pluging loading.

Since it is end of day in TOK, I'm going to leave this alone for now. Anyone else feel free to pick up from here and update the bug with any findings. We need to understand what the problem is quickly, so we can make a determination whether this should be a blocker for PlzNavigate in M61 or not.

@nasko: regarding whether this should be a blocker or not, I'm curious why you say we have to understand what the problem is to determine that? e.g. isn't it enough to know how common/severe this issue is to determine this?

@nasko: so I tried reproducing again on M61 and sometimes it works, sometimes not. For example clicking on the link seems to always work, but not navigation from omnibox (though sometimes it does). I will investigate some more but this really points to a race condition.

So update on where I am at: the issue is most likely coming from a bad interaction between the ResourceHandlers on the IO thread that is coming from wait time on the UI thread when the response was received.

For the record, the ResourceHandlers in the IO thread are arranged in the following manner:
MimeSniffingResourceHandler -> NavigationResourceHandler -> InterceptingResourceHandler -> StreamResourceHandler

When everything goes well, for example in direct navigations to pdf, the following happens:
- MimeSniffingResourceHandler::OnResponseStarted detects that the navigation should be intercepted by a plugin and instruct the InterceptingResourceHandler to switch the leaf ResourceHandler and provides a payload for the old handler (StreamResourceHandler).
- MimeSniffingResourceHandler::OnResponseStarted passes the response to downstream handlers, including NavigationResourceHandler. NavigationResourceHandler takes hold of the controller and asks the UI thread to check if the navigation should proceed.
- NavigationRequest asks the NavigationHandle to perform the OnResponseStarted checks.
- Once they have completed, NavigationRequest asks NavigationResourceHandler to proceed with the navigation and sends the navigation for commit to the renderer.
- NavigationResourceHandler passes the response to downstream handlers, including the InterceptingResourceHandler. The InterceptingResourceHandler gives the response to the StreamHandler, followed by the payload and a fake completion message.
- when the renderer accesses the stream, it can read the payload and start the plugin.

When things go wrong the following happens:
- MimeSniffingResourceHandler::OnResponseStarted detects that the navigation should be intercepted by a plugin and instruct the InterceptingResourceHandler to switch the leaf ResourceHandler and provides a payload for the old handler (StreamResourceHandler).
- MimeSniffingResourceHandler::OnResponseStarted passes the response to downstream handlers, including NavigationResourceHandler. NavigationResourceHandler takes hold of the controller and asks the UI thread to check if the navigation should proceed.
- NavigationRequest asks the NavigationHandle to perform the OnResponseStarted checks.
- MimeSniffingResourceHandler::OnResponseCompleted is called, which is passed to downstream handlers, including NavigationResourceHandler.
- NavigationResourceHandler thinks this means the request was aborted before it started. It detaches from the NaviagtionURLloaderImplCore and passes the completion message to downstream handlers.
- InterceptingResourceHandler::OnResponseCompleted is called. It passes the completion status to the StreamResourceHandler but doesn't write the payload into it. So the stream is closed without having the payload.
- Once the NavigationThrottle checks on the UI thread have completed, NavigationRequest asks NavigationResourceHandler to proceed with the navigation and sends the navigation for commit to the renderer.
- NavigationResourceHandler detached from the NavigationURLLoaderCore so it can no longer listen to notifications from the UI thread.
- On the other hand, the NavigationRequest is already gone, so it is not cancelled by the call to NotifyRequestFailed (https://cs.chromium.org/chromium/src/content/browser/loader/navigation_resource_handler.cc?q=navigationresourcehandler&dr=CSs&l=172) that comes from the request being completed.
- when the renderer finally accesses the stream, the stream is empty and doesn't contain the payload which allows it to start the navigation.

So to sum it up, the issue is we get OnResponseCompleted while NavigationResourceHandler is waiting on the UI thread to perform the OnResponseStarted checks. This is quite unexpected, because the NavigationResourceHandler is holding the ResourceController at that time - but maybe the rest of the IO stack doesn't care about this for the request completion?. I'm still investigating why we get the OnResponseCompleted when we have a redirect and not on direct navigations (maybe because we have more throttle checks on the UI thread?).

So, I finally got to the root cause of the issue, which is due to a problem in the MIME handler stream code badly handling the deletion of the speculative RFH. This also explains why I couldn't reproduce the issue when navigating from a link (and why it only happens on redirects).

Ok so the following happens:
1) at the beginning of the navigation, we create a speculative RFH for http://wg21.link/concepts.
2) the navigation gets redirect to http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4674.pdf.
3) MimeSniffingResourceHandler::OnResponseStarted detects that the navigation should be intercepted by a plugin and instruct the InterceptingResourceHandler to switch the leaf ResourceHandler and provides a payload for the old handler (StreamResourceHandler).
4) We create a new StreamResourceHandler. Its Stream is managed by a StreamContainer and we have a MimeStreamManager::EmbedderObserver monitor the WebContents.
5) MimeSniffingResourceHandler::OnResponseStarted passes the response to downstream handlers, including NavigationResourceHandler. NavigationResourceHandler takes hold of the controller and asks the UI thread to check if the navigation should proceed.
6) Navigation request first checks which RenderFrameHost should commit the navigation. Since there was a cross-site redirect and this is browser-initiated, the speculative RFH is no longer valid. So we create a new speculative RFH and destroy the one we created at the beginning of the navigation. NavigationRequest also asks the NavigationHandle to execute the OnResponseStarted checks.
7) The deletion of the first speculative RFH is observed by the MimeHandlerStreamManager::EmbedderObserver and this is where things go wrong. The MimeHandlerStreamManager::EmbedderObserver was very tied to a specific RFH, so it had to be ported to deal with FTN id. What they chose was to consider that they were tracking any RFH in the FTN (see https://cs.chromium.org/chromium/src/extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.cc?l=278). In case of deletion, this means that any deletion of a RFH inside the FTN will lead to an abort of the stream (https://cs.chromium.org/chromium/src/extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.cc?l=197).
8) The StreamContainer is deleted, which has the side effect of posting a task to the IO thread of closing the stream we created for the pdf (not the normal navigation stream).
9) NavigationHandle finishes it checks and determine we can proceed. We send the navigation to the second speculative RFH to commit. We also post a task to the IO thread to proceed with the navigation and delete the NavigationRequest.
10) On the IO thread, the Stream for the pdf closes - as asked by the task posted from the StreamContainer, which has the effect on calling CancelOutOfBound in its StreamResourceHandler (which is the second StreamResourceHandler and is not yet in the chain of ResourceHandler btw).
11) This will make the URL request fail with an net::ERR_ABORTED status.
12) Because of this, we start calling OnResponseCompleted on the various ResourceHandlers in particular on InterceptingResourceHandler::OnResponseCompleted. This passes the completion status to the initial StreamResourceHandler but doesn't write the payload into it. So the stream is closed without having the payload.
13) When the renderer finally accesses the stream, the stream is empty and closed with a status of ERR_ABORTED. The provisional load fails, but since the err is ABORTED, we don't show an error page and just stop loading (which deletes the NavigationHandle).

Depending on the timing, we might have other variations of the bug. For example, if a NavigationThrottle defers the navigation, we might have NavigationResourceHandler::OnResponseComplete called before the NavigationRequest has been deleted. In that case, we'll call OnRequestFailed with an err of net::ERR_ABORTED and kill the NavigationRequest along with the NavigationHandle.

Since the call to cancel the URLRequest from OutOfBoundCancel involves several post tasks on the IO thread, it's also possible that NavigationResourceHandler::ProceedWithNavigation happens before the calls to OnResponseComplete. In that case, NavigationResourceHandler passes the response to downstream handlers, including the InterceptingResourceHandler. The InterceptingResourceHandler gives the response to the StreamHandler, followed by the payload and a fake succesfull completion message. When the renderer accesses the stream, it can read the payload and start the plugin and it commits the navigation. However, the second stream the plugin was supposed to read from has been closed, so the renderer can't actually get the pdf data. So we end up with a pdf viewer without pdf (ie dark grey background).

So basically the fix would be not delete the stream in the MimeHandlerStreamManager every time a RFH is destroyed in the FTN. We could restrict it to the current RFH/all RFHs being destroyed for example.

Updated the title to reflect the findings.

And I have a fix at https://chromium-review.googlesource.com/c/chromium/src/+/675403.

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

commit 8d8fe833676a4b7779317146415a9f11877336f2
Author: clamy <clamy@chromium.org>
Date: Thu Sep 21 16:24:10 2017

PlzNavigate: fix issue with cross-site redirects to pdf

This CL prevents the MimeHandlerStreamManager from deleting its stream
if a speculative RenderFrameHost is deleted in the FrameTreeNode it is
tracking. This can happen when a navigation encounters a cross-site
redirect to a pdf document, and it had the side effect of cancelling the
URLRequest.

BUG= 765780 

Change-Id: Ia372bc55fa08d63bf4bb309490b0bcd8c64f7f5a
Reviewed-on: https://chromium-review.googlesource.com/675403
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503463}
[modify] https://crrev.com/8d8fe833676a4b7779317146415a9f11877336f2/chrome/browser/chrome_navigation_browsertest.cc
[modify] https://crrev.com/8d8fe833676a4b7779317146415a9f11877336f2/extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.cc

TPM: this is a small fix so requesting merge to M62.

Others: per Camille's findings, since this is only for browser-initiated cross-origin redirects, it seems like an edge case and we can live with it broken on M61 for a few weeks. Does anyone disagree?

This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

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

Thanks for the fix!  I agree that this is a bit more of a corner case due to the need for a cross-process navigation.  This will affect bookmarks, but it does seem less concerning than affecting all redirects to PDFs.  If there aren't a lot of other cases affected by this, we can probably wait for M62.

Some of our customers stopped some mailing campaigns due to this bug cause these contains links to PDFs with an opening count redirect. Is there any chance that this get fixed before M62 cause for them is quite blocker !

@davidef1986@gmail.com This won't be fixed in m61. Can they add workarounds in the meantime, i.e. cross origin redirect first to an html page on the origin that serves the pdf, and then that does a location.replace to the pdf?

Approving merge to M62. Branch:3202

I can help with the merge (since clamy@ is OOO).

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

commit 3e1dde048b5bf50bdbc12143e4ec3f2924c1dafb
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Sep 22 20:54:36 2017

PlzNavigate: fix issue with cross-site redirects to pdf

This CL prevents the MimeHandlerStreamManager from deleting its stream
if a speculative RenderFrameHost is deleted in the FrameTreeNode it is
tracking. This can happen when a navigation encounters a cross-site
redirect to a pdf document, and it had the side effect of cancelling the
URLRequest.

BUG= 765780 
TBR=clamy@chromium.org

(cherry picked from commit 8d8fe833676a4b7779317146415a9f11877336f2)

Change-Id: Ia372bc55fa08d63bf4bb309490b0bcd8c64f7f5a
Reviewed-on: https://chromium-review.googlesource.com/675403
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Camille Lamy <clamy@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#503463}
Reviewed-on: https://chromium-review.googlesource.com/679209
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#407}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/3e1dde048b5bf50bdbc12143e4ec3f2924c1dafb/chrome/browser/chrome_navigation_browsertest.cc
[modify] https://crrev.com/3e1dde048b5bf50bdbc12143e4ec3f2924c1dafb/extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.cc

 Issue 770711  has been merged into this issue.

jam:  bug 770711  is another user report about this issue affecting M61.

 Issue 770587  has been merged into this issue.

 Issue 771720  has been merged into this issue.

Issue 774739 has been merged into this issue.

Issue 775382 has been merged into this issue.

I guess we're not going to bother fixing M61, and just wait for M62 to roll out?

@mmenke: correct (see comments 15, 17, 19).

 Issue 780363  has been merged into this issue.