New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 812664 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-03-16
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

FullscreenTestCase/testShowHeaderWhenChildTabCloses is flaky on devices

Project Member Reported by liaoyuke@chromium.org, Feb 15 2018

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Feb 15 2018

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

commit 312590315b04a872da752b1fda872d722590f584
Author: Yuke Liao <liaoyuke@chromium.org>
Date: Thu Feb 15 19:38:02 2018

[iOS] Disable a fullscreen egtest on devices.

FullscreenTestCase/testShowHeaderWhenChildTabCloses is flaky on
devices.

Bug:  812664 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I7e632efa8823e253264c0d20012ac80c628562e7
Reviewed-on: https://chromium-review.googlesource.com/922261
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537102}
[modify] https://crrev.com/312590315b04a872da752b1fda872d722590f584/ios/chrome/browser/ui/fullscreen/fullscreen_egtest.mm

Cc: linds...@chromium.org
Labels: ReleaseBlock-Beta Hotlist-DisableReview
Kurt have you already reviewed this disabled test? 
Labels: M-66
NextAction: 2018-03-16
Sorry, have not looked into this.  Will take a look at this tomorrow.
The NextAction date has arrived: 2018-03-16

Comment 6 by cmasso@google.com, Mar 19 2018

Any update here Kurt?
Status: Started (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 25 2018

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

commit b4383a791f180a25c3ef5952233e4f13bd2e33af
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Sun Mar 25 19:37:22 2018

[iOS] Only verify error code when link tapping fails.

The behavior of TapWebViewElementWithId() for links that close a
WebState is flaky on both simulator and device, regardless of whether
the Phase 1 UI Reboot flag is enabled.  This CL updates
FullscreenTestCase.testShowHeaderWhenChildTabCloses to check for the
error code only if the link tap is unsuccessful.

Note that the closure of the Tab and the visibility of the toolbar is
still verified after this occurs, so the test will still catch
fullscreen regressions after this change.

Bug:  812664 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ia56293763e185eeb615a5f734aa9a3a473f0b424
Reviewed-on: https://chromium-review.googlesource.com/974791
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545713}
[modify] https://crrev.com/b4383a791f180a25c3ef5952233e4f13bd2e33af/ios/chrome/browser/ui/fullscreen/fullscreen_egtest.mm

Labels: Merge-Request-66
Status: Fixed (was: Started)
Project Member

Comment 11 by sheriffbot@chromium.org, Mar 25 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: Less than 19 days to go before AppStore submit on M66
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

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

Comment 12 by cmasso@google.com, Mar 26 2018

Please verify in canary
@cmasso This verification request is directed to... the test owner kkhorimoto?
Cc: cma...@chromium.org

Comment 15 by cmasso@google.com, Mar 27 2018

Labels: -Hotlist-Merge-Review -Merge-Review-66 Merge-Approved-66
Since this does not require manual verification, then I Kurt should verify.

Kurt, I am approving the merge upon verification of the fix in canary
It looks like there have not been any failures of this test since being reenabled on device, so I will cherry-pick now.

https://uberchromegw.corp.google.com/i/internal.bling.main/waterfall?builder=iphone10-device-x64
https://uberchromegw.corp.google.com/i/internal.bling.main/waterfall?builder=ipad10-device-x64
Project Member

Comment 17 by bugdroid1@chromium.org, Mar 27 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d18463ba74c2268929b3a61307cf8204baeef8d

commit 2d18463ba74c2268929b3a61307cf8204baeef8d
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Tue Mar 27 01:36:06 2018

[iOS] Only verify error code when link tapping fails.

The behavior of TapWebViewElementWithId() for links that close a
WebState is flaky on both simulator and device, regardless of whether
the Phase 1 UI Reboot flag is enabled.  This CL updates
FullscreenTestCase.testShowHeaderWhenChildTabCloses to check for the
error code only if the link tap is unsuccessful.

Note that the closure of the Tab and the visibility of the toolbar is
still verified after this occurs, so the test will still catch
fullscreen regressions after this change.

Bug:  812664 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ia56293763e185eeb615a5f734aa9a3a473f0b424
Reviewed-on: https://chromium-review.googlesource.com/974791
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#545713}(cherry picked from commit b4383a791f180a25c3ef5952233e4f13bd2e33af)
Reviewed-on: https://chromium-review.googlesource.com/981613
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#460}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/2d18463ba74c2268929b3a61307cf8204baeef8d/ios/chrome/browser/ui/fullscreen/fullscreen_egtest.mm

Sign in to add a comment