Suspected CL: https://chromium.googlesource.com/chromium/src/+/603474151d2ea47cbf53a54b87bf13786aa90cef

Review-Url: https://codereview.chromium.org/2472863002

Please find the log & video @ http://go/chrome-androidlogs1/6/667734

Bisect script pointed to https://chromium.googlesource.com/chromium/src/+/603474151d2ea47cbf53a54b87bf13786aa90cef

ryansturm@, Can you please take a look, Thanks

This CL definitely changed this behavior. Is there a strong reason to think this behavior is worse? The chrome model of reloading is that it tries to re-navigate the tab to the same URL, and the offline model is that when a navigation happens, it checks if the content should be an offline page.

This behavior was changed for offline previews for the reason that the user can refresh the page through the previews info bar using a simple reload mechanism and the offline_page_info is cleared during a refresh.

There is probably a way to work around this by passing information with the navigation handle (or another workaround) and some of OfflinePageTabHelper::DidStartNavigation can be removed.

The reason the UI flickers is because DidStartNavigation happens before DidFinishNavigation, and in between the two the page is no longer considered an offline page. If the offline_page_info was changed only during DidFinishNavigation, this shouldn't happen, but offline previews wouldn't work.

+dimich, bengr

The flickering is bad from the UX perspective and this affects M56, so it should be fixed.

Dmitry is out next week, but I'll gladly help you with coming up with an approach that satisfies all constraints.

I'm not sure what the best workaround for this fix is. At the time of a new navigation, whether a reload or other navigation, it's not possible to know if the page that is being navigated to is going to be offline. Because of that, an option might be to leave the offline in the omnibox until DidFinishNavigation occurs. The other option is to try to guess during DidStartNavigation if we think the next navigation will be to an offline page (this is effectively what was happening before by checking if it was a reload/fragment navigation).

The first option is achievable with the design I wrote a while ago that got voted down in design review: https://docs.google.com/document/d/1H5qlZTjjkStJYbHyFtU0MtuIqg66H0IAz7EZcErv9o4/edit#heading=h.iy1hgvat7vly

This design is way to big to merge back, and it does leave the offline badge lingering for a little bit on offline to non-offline navigations.

I can't think of a design for option 2. I suspect there is a workaround that would just add more state to the tab helper, but I haven't thought of a workaround that meets all the behavior conditions between showing badge, previews infobar, and the offline snackbar at the right times.

A new navigation (like a reload or address bar, not an anchor fragment navigation) is treated as fetching the page again by chrome, which means re-determining offline eligibility, etc. Consider navigating from offline page 1 and then to offline page 2 (which are on different domains/pages): before (and after) my change, this would cause the offline badge to flicker. Is reload the only case that matters from the flickering perspective?

Looking at possible fixes:
There is a workaround that addresses the reload flicker, but is a somewhat ugly code change.

The workaround will re-introduce one behavior that the CL fixed:
Offline to non-offline reload will show the offline badge until DidFinishNavigation occurs (when the main page headers come back, this will before the new page renders). This does not seem like a large issue since the user will get the correct information eventually, but testing on GIN-2g-poor makes it clear that the offline badge is shown for ~2 seconds while the network is being used; typically, the URL being shown during that time is the URL that the user is navigating to, so they might think it is confusing for the offline badge to all of a sudden disappear when they think they are going to an offline page.

Additionally, the CL will not address the following issue:
User navigates from Offline page A to Offline Page B. During the time period between DidStartNavigation and DidFinishNavigation, the offline badge will not be present. This will create a flicker that is exactly the same as the existing reload flicker.

I'm not so sure which trade-offs should be made, but during the time period from DidStartNavigation until DidFinishNavigation, it is not possible to determine if the new navigation will be offline or not, so we should determine a clear policy around whether the badge should be shown or not in those instances.

Does not look like a blocker. It'd be good to understand a better UI transition here. Probably should be coordinated with other navigations as well.

Dropping the milestone label

I just verified that the problem still happens no 61.0.3163.81 (M61 stable)

I don't think anything else than implementing a proper animation of offline chip with verbose status helps here.

Looping in Ted and making the bug available.

[omnibox triage] Has slipped multiple milestones -> clearly not P2.

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

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

Assigning arbitrarily to an offline pages owner to get this out of the to-be-triaged queue.  Reassign as appropriate.

Disclaimer: I did not check to see if it currently reproduces.