Thanks for finding this Chris. We definitely need to cherrypick it to M69

What behavior is controlled by this flag?  What specifically do we lose if we don't cherry-pick it?

We're late enough that the risk of cherry-picking this is probably pretty high, since I'm assuming this code has never been run by users?

Mainly the favicon styling logic for UIRefresh.

I'm guessing either most users just didn't know the styling had to be a certain way (that Bookmarks fallback favicon style should look just like Recent Tabs and History).

Here's Chris's CL is just favicon styling so its pretty safe:
https://chromium-review.googlesource.com/c/chromium/src/+/1178932

Pls apply appropriate OS labels.

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

commit 5cfaf0a0e58b99506f313f25068e91661cabd2e2
Author: Chris Lu <thegreenfrog@chromium.org>
Date: Fri Aug 17 15:11:26 2018

[ios] Move all logic under Collections flag to under UIRefresh flag

Remove all references to CollectionsUIReboot flag.

Bug:  875063 

Change-Id: I5222da0ba5ec50da282a434f8e075758feebb59a
Reviewed-on: https://chromium-review.googlesource.com/1178932
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584071}
[modify] https://crrev.com/5cfaf0a0e58b99506f313f25068e91661cabd2e2/ios/chrome/browser/about_flags.mm
[modify] https://crrev.com/5cfaf0a0e58b99506f313f25068e91661cabd2e2/ios/chrome/browser/experimental_flags.h
[modify] https://crrev.com/5cfaf0a0e58b99506f313f25068e91661cabd2e2/ios/chrome/browser/experimental_flags.mm
[modify] https://crrev.com/5cfaf0a0e58b99506f313f25068e91661cabd2e2/ios/chrome/browser/favicon/favicon_loader.mm
[modify] https://crrev.com/5cfaf0a0e58b99506f313f25068e91661cabd2e2/ios/chrome/browser/ios_chrome_flag_descriptions.cc
[modify] https://crrev.com/5cfaf0a0e58b99506f313f25068e91661cabd2e2/ios/chrome/browser/ios_chrome_flag_descriptions.h
[modify] https://crrev.com/5cfaf0a0e58b99506f313f25068e91661cabd2e2/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm
[modify] https://crrev.com/5cfaf0a0e58b99506f313f25068e91661cabd2e2/ios/chrome/browser/ui/tab_grid/tab_grid_mediator.mm
[modify] https://crrev.com/5cfaf0a0e58b99506f313f25068e91661cabd2e2/ios/chrome/browser/ui/ui_feature_flags.cc
[modify] https://crrev.com/5cfaf0a0e58b99506f313f25068e91661cabd2e2/ios/chrome/browser/ui/ui_feature_flags.h

verified on canary on 8/20

This bug requires manual review: Less than 11 days to go before AppStore submit on M69
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

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

How confident are you in this fix? What's the worst that could happen if favicon logic remains under UICollections flag for M69? Why does this need to make M69?

I'm pretty confident, since this is just switching checking one flag for another. The worst case is that the favicons in history and recent tabs look ugly in M-69. Nothing breaks.

https://drive.google.com/file/d/1PE6t_UCwNdWWpWdh8I8C6rJmX3FOd3H6/view?usp=sharing
https://drive.google.com/file/d/1fD5qTBp_zVqCkbR__fp0YV3VmR_5Hg4L/view?usp=sharing
https://drive.google.com/file/d/1w33scAoLfVmc0yCpeT9xJ46IayDMF-qC/view?usp=sharing
Verified on iPhone X iOS 12 beta #9, iPhone 6+ iOS 11.4 on 69.0.3497.50 Beta

Chris, per c#14, the history screenshot still shows the darker gray favicons. Is this as intended?

Also, since Martijn had some concerns about the lighter gray favicons, did the lighter favicons get approval from accessibility?

That's exactly what this CL is trying to fix, so it makes sense c#14 https://drive.google.com/file/d/1w33scAoLfVmc0yCpeT9xJ46IayDMF-qC/view?usp=sharing has the dark gray background on M69. We should try verifying on the latest canary.

I think Martijn had some concern about the outermost gray background on TabGrid -> Recent Tabs (Where the background is black and not white). That's a different bug that I believe Chris is working on as well.

I think it might be hard to have a different favicon background just for tabgrid. And I actually assumed that we were going to just change it for all. Martijn is that correct?

Here's how they look: 

Deleted: Simulator Screen Shot - iPhone X - 2018-08-20 at 16.29.48.png 292 KB

	Simulator Screen Shot - iPhone X - 2018-08-20 at 16.29.48.png 292 KB View Download

Deleted: Simulator Screen Shot - iPhone X - 2018-08-20 at 16.29.50.png 318 KB

	Simulator Screen Shot - iPhone X - 2018-08-20 at 16.29.50.png 318 KB View Download

The new dark assets I provided were only meant for the dark theme (recents in tab grid). I think it's important to have but let me know the additional work it creates. 

Ok per our short chat offline, we're going to keep the favicon fallback background color the same as it is.

Per comment 13 and that we are less than a week from stable cut, I'm rejecting this merge.