kkhorimoto@ PTAL when you have time. cc'ing gambard@ for toolbar changes.

We'd like to fix this for M-65 but if there's no time M-66 should do.

This issue is also reproducible on latest chrome beta version 65.0.3325.39.

Video:
https://drive.google.com/file/d/1v5m72wqyi58e_gV37pntMJ0ZQ_7hVTuM/view?usp=sharing

Kurt, will be nice to have this in M65 as well.

Issue is also reproducible on iPads

Steps to reproduce:
  1.  Launch chrome beta
  2.  Open any URL
  3.  Scroll the page down by swiping two times 

Observed results:
iPad - Notice that the page goes full screen and does not come out of full screen
iPhones - Omnibox gets faded or pull to refresh actions are displayed

The issue is easily reproducible and visible in current build 65.0.3325.83, compared to the builds on which it is reported (66.0.3328.0, 65.0.3325.39).  Reproducibility rate 5/5 times.

Videos:
iPad - https://drive.google.com/file/d/12ca577wCMALkhzowXgxAVOSRe7DR-3_L/view?usp=sharing

iPhone - https://drive.google.com/file/d/1s3iu4XRfI1IsGc68y0FNKjYYlOaImUUr/view?usp=sharing

Kurt, are you looking into this issue? M65 stable release is next week.

Still investigating a fix here.  It looks like when this reproduces, the overscroll actions view is removed right when the radial "ink" animation finishes.  It's possible that this is somehow cancelling the rest of the animations as well.

Actually the timing was coincidental.  This is occurring because the SnapshotTabHelper is attempting to snapshot the page due to the reload event, and the overscroll animations are cancelled when snapshotting occurs so it's not captured in the image.  I created crrev.com/c/933586 to skip snapshotting for reloads caused by overscroll.

I am not comfortable merging the proposed fix to M65, because we don't fully understand why this broke in the first place.

It sounds like this is only an issue if you trigger the reload action and then immediately pull-to-action again.  If that's true, I think this is uncommon enough that it's not worthy of a last-minute fix?

(The fix doesn't look too bad, but it's touching snapshot code in a way that could break snapshots entirely.  I don't think the bug is bad enough to be worth that risk.)

 Thanks Rohit! Let's leave the fix bake in M66.

I've added crrev.com/c/933586 to the CQ, so it should be landed soon.  I agree that it should be left for M66 given how late it is landing.  I don't know if I agree with Rohit about the severity of the risk here though.  This function is only ever called for reloads triggered from overscroll actions, so snapshots will still be generated for any other type of navigation and when entering the tab switcher.

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

commit 902a6261bd189cdfb33a71d35c5127fd59f0d7de
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Tue Feb 27 00:11:49 2018

[iOS] Don't snapshot page loads occuring because of overscroll actions.

Snapshotting cancels in-progress overscroll animations, so page loads
that occur before the overscroll "bounce back" animation finishes will
cut that animation short.

Bug:  805165 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ifb105b79bbb700bca752000ebfb15f4123e90f38
Reviewed-on: https://chromium-review.googlesource.com/933586
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539320}
[modify] https://crrev.com/902a6261bd189cdfb33a71d35c5127fd59f0d7de/ios/chrome/browser/snapshots/snapshot_tab_helper.h
[modify] https://crrev.com/902a6261bd189cdfb33a71d35c5127fd59f0d7de/ios/chrome/browser/snapshots/snapshot_tab_helper.mm
[modify] https://crrev.com/902a6261bd189cdfb33a71d35c5127fd59f0d7de/ios/chrome/browser/ui/browser_view_controller.mm

Forgot to mark this as closed due to prior discussion about whether this should be merged into M65.  The fix had landed before the M66 branch point, though, so no merge is required.

[Auto-generated comment by a script] We noticed that this issue is targeted for M-66; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-66 label, otherwise remove Merge-TBD label. Thanks.

Is this fix in M66?

Please request merge approval

This landed before M66 branch point, so no merge necessary!

Verified on 66.0.3359.30 Beta on iPhone7+ iOS 10.3.3, iPhone 6S+ iOS 11.3 beta#4