Do you suggest a bug in components/favicon? If not, please reassign to sammc@. In doubt, I can investigate.

From what I can see, DidUpdateWebManifestURL() is never called for "https://app.ft.com/index_page/home". Note the https scheme. Reassigning since it seems an issue with WebContents.

ContentFaviconDriver::DidUpdateWebManifestURL() is called for me for https://app.ft.com/index_page/home at r473488. It isn't called again for same-page navigation, but it isn't expected to since the manifest URL doesn't change.

I do think we need to change WebContents to report the manifest's URL in these cases, consistently with how favicon URLs are reported.

I don't see a good way that the client code (FaviconDriver hierarchy) can make these pages work otherwise, since the lack of notification is ambiguous and can be either:
a) The loaded site doesn't contain a manifest.
b) The loaded site has the same manifest URL as the previous one.

sammc@: if you agree on the above, but nobody in your team has the cycles to change this behavior, please reassign to me and we'll take care, thanks.

Friendly ping, thanks.

I think content::NavigationHandle::IsSameDocument() should allow you to detect (b).

Do you suggest we use IsSameDocument() to assume the manifest URL didn't change? Looks like a sane heuristic but it's not guaranteed, is it?

Generally, it looks like consistency between DidUpdateFaviconURL() and DidUpdateWebManifestURL() would be desirable. Achieving that in upper layers seems a workaround to me.

For the archive, here are the steps I use to repro this issue:
1. Use M60+ of clank (preferably from head).
2. Clear browsing history.
3. Use chrome://flags to enable enable-favicons-from-web-manifest.
4. Type "https://app.ft.com/index_page/home" in the omnibox.
5. Open chrome://ntp-tiles-internals, where the app.ft.com should show up.

Associated to app.ft.com, there should be a row starting with WEB_MANIFEST_ICON that points to the icon listed in the Web Manifest.

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

commit 406899d78bedbdb4d09f65f65991126e61a8e758
Author: mastiz <mastiz@chromium.org>
Date: Wed Jun 28 18:29:06 2017

Fix in-document navigations breaking icons from Web Manifests

Content notifies with DidUpdateWebManifestURL() in page loads *unless*
its a navigation within the same document. This requires handling those
cases differently and hence not clearing the manifest URL, because
otherwise the page is believed to contain no manifest.

BUG= 724832 

Review-Url: https://codereview.chromium.org/2950563002
Cr-Commit-Position: refs/heads/master@{#483072}

[modify] https://crrev.com/406899d78bedbdb4d09f65f65991126e61a8e758/chrome/browser/favicon/content_favicon_driver_browsertest.cc
[add] https://crrev.com/406899d78bedbdb4d09f65f65991126e61a8e758/chrome/test/data/favicon/pushstate_with_manifest.html
[modify] https://crrev.com/406899d78bedbdb4d09f65f65991126e61a8e758/components/favicon/content/content_favicon_driver.cc