+ Charlie , who is familiar with content's navigation system.

Could you offer any insight into how content handles redirects?  NavigationEntry has a page transition type, which may include one of the REDIRECT types, which indicates that a redirect would be expressed via multiple NavigationEntries with REDIRECT transition types.  However, it also looks like NavigationEntry has the [Get/Set]RedirectChain() API, which indicates that a redirect chain is reflected within a single NavigationEntry.  Could you explain how those two systems interact with each other?  The web layer's handling of redirects uses a confusing mixture of the two strategies, and we'd like to mimic content's behavior if possible.

Sure.  NavigationEntries represent stops in the back/forward session history, so we don't have separate NavigationEntries for server redirects (which you can't go back to).  Client redirects involve a commit of one page followed by a commit of another, so I think they tend to create a new NavigationEntry that replaces the first NavigationEntry.

The redirect chain is supposed to include all the URLs we redirected through on the way.  I thought it would include both server and client redirects, but estark@ was mentioning today that it might not include client redirects.  This might be fixable if necessary.

(In general, I've found PageTransition isn't very reliable for making policy decisions about navigations, so I tend not to rely on it when possible.)

I was merely guessing that the redirect chain wouldn't include client-side redirects; I don't have any real knowledge on the topic. :) But now that I'm poking at the code, it does seem that the redirect chain is populated only via network requests redirecting, for example NavigationResourceThrottle::WillRedirectRequest.

I'm not even really sure what the definition of a "client redirect" is. Would we want to consider it a client redirect if a page does `location.replace` at any point? What about assigning to location.href?

There's some magic, I think.  location.href assignments before some point in time (onload?) get treated as client redirects, and later ones don't.  japhet@ might remember.

Yep, client redirect never get history entries before onload *completes*. So:

window.onload = function() {
  location = 'foo.html';
}

...will never create a history entry. But the following generally will:

window.onload = function() {
  setTimeout(function() {
    location = 'foo.html';
  }, 0);
}

Since content uses a redirect chain to keep all the redirects, do you have an example when the chain is used? or may be useful?

Looks like ios/web does not crate extra navigation items for server side redirects and the problem is only limited to client-side redirects

 Issue 661670  has been merged into this issue.

It turns out that we still create extra navigation items for server side redirects, but in rare cases:

One example of re-producing is with AMP (accelerated mobile page), and steps are as following:

(1) Navigate to an AMP (Accelerate Mobile Page) -- I do this from news.google.fr
(2) Shutdown Chrome on iOS (i.e. switch to another app, then kill the app)
(3) Relaunch Chrome on iOS

Somehow, when relaunches Chrome, a server side redirect is fired, and due to crbug.com/676129, a new item with server side redirect transition type is added.

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/5bc2b7751a2861cb1cf631b5f214720d8ac31b2a

commit 5bc2b7751a2861cb1cf631b5f214720d8ac31b2a
Author: mrefaat <mrefaat@google.com>
Date: Wed May 31 19:14:30 2017

We will get consistent behavior as mobile Safari after switching to WKBasedNavigationManager (crbug.com/734150), which is no extra NavigationItem entry for server-side redirects, and same for client-side redirects that happen before page onload (as described in comment #6).

The new navigation manager is targeted for M64. Eugene -- is it worth fixing this on the old navigation stack before then?

I don't think we should fix old navigation stack before getting any results from WKBasedNavigationManager experiment.