Is this an iOS-only problem? (I can not develop for this specific platform but if it's a cross-platform issue, I can probably tackle it)

Sorry for the spam, I just saw the remark. I will investigate it.

Thanks for all the provided details, esp. the videos! This can definitely be improved but I am not sure about the amount of work required.

FTR, the current state is justified by some UX decisions: 

1. Using a real icons is an exception from the iron "no updates while visible" rule. (No change in position of functionality) 

2. If we have the possibility of getting a real icon, we try that and do not show the baked-in icon first. 
--> In airplane mode, this must fail. We try anyway. This is the time frame when we see the gray icon. Obviously, it might make sense to check for any data connection before even trying to fetch an icon. 


It's interesting, that iOS takes so long to notice the missing connection. Maybe there are still retries there.

@Gauthier: has the iOS icon display behavior ever changed?
In issue 742411, it was claimed that this used to work in M59.

We're going to launch baked-in popular sites on iOS in M61. Can we check if it's broken there as well?

M61 builds still show the same (okay for Android, esp with CrHome). Over to Gauthier for iOS.

The baked-in popular sites will be launched on M61, so I cannot reproduce on M60.
I have trouble reproducing on M61 and M62: I only have the grey icons. I never see the baked-in icons, even on opening a new NTP or doing a cold start with the app.
Does iOS have the baked-in icons?

Yep, iOS M61 should have baked-in icons. Issue 725961 is our launch bug for that.

@fhorschig: Can you pls quickly sync with Gautier and figure out why it is not working?

After some investigations, this is caused by a starvation of the thread used to decode WebP on iOS.
On iOS, WebP images are not decoded by the system and we have our own decoder.

Here is the execution flow:
- IconCacherImpl::StartFetchPopularSites asks the IOSImageDecoder to decode the backed in WebP favicon.
  - The IOSImageDecoder asynchronously starts the decoding on a separate tasks runner when/if this decode returns, the backed in favicon will be displayed.
- IconCacherImpl::StartFetchPopularSites starts a Request on the image_fetcher to fetch the favicon on the remote website.
  - As there is no network, the image_fetcher returns with empty result.
  - The callback is called with an empty image, triggering (I am not sure to understand how) IconCacherImpl::StartFetchPopularSites again.

The calls from the image_fetcher are not synchronous, but are too fast to let the callback from the WebP decoding to be scheduled on the thread, leading to a starvation for this callback.

As this is caused by the WebP decoding specific to iOS, this is not reproducible on Android.

I was actually wrong. There is no starvation, the callback is cancelling all other fetch of favicon. So the non-WebP callback returns before the WebP one, cancelling it.
I am concern that the reverse of this bug is happening: if the WebP (backed-in) returns first, is it cancelling the non-WebP fetch? Are we fetching the favicon?

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

commit 247d8bf7a33ef9c33a4ee25089f6adc16ed84272
Author: gambard <gambard@chromium.org>
Date: Wed Jul 26 16:51:38 2017

Use task runner for decoding image

When multiple images are decoded at the same time, the WebP images are
send to another thread and the non-WebP images are not. If the object
using the decoder expect a sequential decoding of the image, it will
fail.
With this CL, the decoder uses the task runner for both WebP and
non-WebP images, allowing a better scheduling.

Bug:  737084 
Change-Id: I16d09c5cc03ca1f046e68a5c6a370d8c2cc45481
Reviewed-on: https://chromium-review.googlesource.com/584842
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489671}
[modify] https://crrev.com/247d8bf7a33ef9c33a4ee25089f6adc16ed84272/components/image_fetcher/ios/ios_image_decoder_impl.mm

Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

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

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

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

commit b457c4cd0cc7d1fc8dc3ac6f1ddcf2ef09475da6
Author: gambard <gambard@chromium.org>
Date: Mon Jul 31 15:16:54 2017

Use task runner for decoding image

When multiple images are decoded at the same time, the WebP images are
send to another thread and the non-WebP images are not. If the object
using the decoder expect a sequential decoding of the image, it will
fail.
With this CL, the decoder uses the task runner for both WebP and
non-WebP images, allowing a better scheduling.

TBR=gambard@chromium.org

(cherry picked from commit 247d8bf7a33ef9c33a4ee25089f6adc16ed84272)

Bug:  737084 
Change-Id: I16d09c5cc03ca1f046e68a5c6a370d8c2cc45481
Reviewed-on: https://chromium-review.googlesource.com/584842
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#489671}
Reviewed-on: https://chromium-review.googlesource.com/594147
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#153}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/b457c4cd0cc7d1fc8dc3ac6f1ddcf2ef09475da6/components/image_fetcher/ios/ios_image_decoder_impl.mm

This issue is verified in 62.0.3170.0 canary
Device: iPhone6, 6s, iPad Air2, iPad Pro
iOS: 10.3.3, 10.2.1

Popular sites with icons are displayed correctly without any delay while in Airplane mode.

This issue is verified in 61.0.3163.52 Beta
Device: iPad Mini, iPhone 6plus
iOS: 10.3.3, 11.0 Beta 6

Popular sites with icons are displayed correctly without any delay while in Airplane mode.