https://logs.chromium.org/v/?s=chrome%2Fbb%2Finternal.bling.main%2Fiphone10-simulator%2F12631%2F%2B%2Frecipes%2Fsteps%2Fios_chrome_ui_egtests__iPhone_5_iOS_10.3__on_Mac%2F0%2Flogs%2FTabSwitcherTransitionTestCase__x2f_testLeaveSwitcherByOpeningNewNormalTab%2F0

This bug is marked as not time critical but is also blocking M69 beta. Please adjust priority or remove RBB. 

Downgrading to RBS.  I don't think this needs to block beta, but I'll investigate more before removing from 69.

Over to gambard for a bit to see what he makes of it.

If you run testLeaveSwitcherByOpeningNewIncognitoTab followed by testLeaveSwitcherByOpeningNewNormalTab, on iOS 10.3 only, we seem to get in a state where the secondary toolbar is gone (has height 0).

This happens during setUp of the second test, at the point where we open a new tab.  The tab animates in with no secondary toolbar, and this causes the test to fail when trying to open the tools menu.

The previous test closes all tabs programmatically in tearDown, which might be an important part of the repro steps.  I have not been able to make the secondary toolbar disappear in the real app.

@gambard if you have a few minutes, would you mind looking at this to see if anything jumps out at you?  I'll take it back tomorrow morning if you haven't found anything interesting.  Thanks!

I can reproduce:
- Have 0 open tabs
- Open a new incognito tab
- LongPress on the TabGrid button > Close This Tab
- Open a NewTab

It seems that the BVC is having a SizeClass undefined at this point. I don't really know why. The -traitCollectionDidChange: isn't called even if the trait collection is changed. This is really weird. Is this a new failure? Or is it a new test?

Last time I got something similar was because an object was overriding -traitCollectionDidChange: and wasn't calling super so the BVC wasn't notified (but IIRC is was also failing for iOS 11).

The difference is clear between iOS 10 and 11: the bottom toolbar is updated in iOS 11 during the call to -viewSafeAreaInsetsDidChange.

Concerning the bug itself, the difference comes from the way the tab is closed. If the tab is closed using the TabGrid, then everything is fine. In that case, when a new tab is opened, the height of the bottom toolbar is updated in -viewDidAppear.

If the tab is closed using the overscroll action or the long press on the TabGrid button, then the normal BVC is presented at some point, even if there is no tab and it is not visible to the user (this is why you end up in the "normal" tab switcher and not the incognito one). I am not sure to understand why. Presenting the BVC is calling -viewDidAppear, which activates the BVC. Once activated it is observing the fullscreen model and the toolbar height get reset when the new web state is opened (at which point the BVC isn't on screen, so its traitCollection is empty).

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

commit 835c5e1e84342eaf2bfff2bb420baaebed165dd2
Author: Gauthier Ambard <gambard@chromium.org>
Date: Fri Jul 27 13:37:28 2018

Don't always change to mainBVC when closing tab

When a tab is closed using a gesture (i.e. overscroll action or long
press on the TabGrid button), the non-incognito BVC was selected even
if it has no Tab.

This leads to an issue as the BVC is very briefly presented during this
time, before being dismissed. But the -viewDidAppear: call is executed,
which leads to a situation where the BVC is marked as "active" even if
it is not presented on screen. The -viewWillDisappear: call is never
made.

This CL fixes it by setting the main BVC as active only if it contains
tabs.

Bug:  849937 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I88f9a36247a7b5fc9ea34b686cc17850aca1615d
Reviewed-on: https://chromium-review.googlesource.com/1150533
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578620}
[modify] https://crrev.com/835c5e1e84342eaf2bfff2bb420baaebed165dd2/ios/chrome/app/main_controller.mm
[modify] https://crrev.com/835c5e1e84342eaf2bfff2bb420baaebed165dd2/ios/chrome/browser/metrics/BUILD.gn
[modify] https://crrev.com/835c5e1e84342eaf2bfff2bb420baaebed165dd2/ios/chrome/browser/metrics/ukm_egtest.mm
[modify] https://crrev.com/835c5e1e84342eaf2bfff2bb420baaebed165dd2/ios/chrome/browser/ui/toolbar/adaptive/BUILD.gn
[modify] https://crrev.com/835c5e1e84342eaf2bfff2bb420baaebed165dd2/ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_egtest.mm
[modify] https://crrev.com/835c5e1e84342eaf2bfff2bb420baaebed165dd2/ios/chrome/test/app/tab_test_util.mm

+kariahda@ for merge request

Approved!

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

commit 13794791f441ce48b671bface1f2d98a2ca1a550
Author: Gauthier Ambard <gambard@chromium.org>
Date: Tue Jul 31 07:57:03 2018

Don't always change to mainBVC when closing tab

When a tab is closed using a gesture (i.e. overscroll action or long
press on the TabGrid button), the non-incognito BVC was selected even
if it has no Tab.

This leads to an issue as the BVC is very briefly presented during this
time, before being dismissed. But the -viewDidAppear: call is executed,
which leads to a situation where the BVC is marked as "active" even if
it is not presented on screen. The -viewWillDisappear: call is never
made.

This CL fixes it by setting the main BVC as active only if it contains
tabs.

Bug:  849937 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I88f9a36247a7b5fc9ea34b686cc17850aca1615d
Reviewed-on: https://chromium-review.googlesource.com/1150533
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#578620}(cherry picked from commit 835c5e1e84342eaf2bfff2bb420baaebed165dd2)
Reviewed-on: https://chromium-review.googlesource.com/1156264
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#262}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/13794791f441ce48b671bface1f2d98a2ca1a550/ios/chrome/app/main_controller.mm
[modify] https://crrev.com/13794791f441ce48b671bface1f2d98a2ca1a550/ios/chrome/browser/metrics/BUILD.gn
[modify] https://crrev.com/13794791f441ce48b671bface1f2d98a2ca1a550/ios/chrome/browser/metrics/ukm_egtest.mm
[modify] https://crrev.com/13794791f441ce48b671bface1f2d98a2ca1a550/ios/chrome/browser/ui/toolbar/adaptive/BUILD.gn
[modify] https://crrev.com/13794791f441ce48b671bface1f2d98a2ca1a550/ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_egtest.mm
[modify] https://crrev.com/13794791f441ce48b671bface1f2d98a2ca1a550/ios/chrome/test/app/tab_test_util.mm