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

Issue 702410 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

PageStateTest.testScrollPositionRestoring is flaky

Project Member Reported by baxley@chromium.org, Mar 16 2017

Issue description

This test is failing about 5% of the time on the bots.

Here is the most recent failure:
https://build.chromium.org/p/chromium.fyi/builders/EarlGreyiOS/builds/25345

You can look at the screenshot, where it looks to be scrolling too far.

Error log:
Exception Details: Error Trace: [
  {
    "Description" : "Assertion with matcher [M] failed: UI element [E] failed to match due to the mismatch [S].",
    "Description Glossary" :     {
      "M" : "(kindOfClass('UIScrollView') && contentOffset({0, 40}))",
      "E" : "<WKScrollView:0x7fc2a883a600; AX=N; AX.frame={{0, 64}, {768, 960}}; AX.activationPoint={384, 544}; AX.traits='UIAccessibilityTraitNone'; AX.focused='N'; frame={{0, 0}, {768, 960}}; opaque; alpha=1>",
      "S" : "contentOffset({0, 40})"
    },
    "Domain" : "com.google.earlgrey.ElementInteractionErrorDomain",
    "Code" : "3",
    "File Name" : "GREYAssertions.m",
    "Function Name" : "+[GREYAssertions grey_createAssertionWithMatcher:]_block_invoke",
    "Line" : "74",
    "TestCase Class" : "PageStateTestCase",
    "TestCase Method" : "testScrollPositionRestoring"
 

Comment 1 by baxley@chromium.org, Mar 16 2017

Below are some debugging notes, not sure if it's useful or not...

Here's a hack I used to run it 1000 times locally, which typically would result in a failure. With this code, I'd just run "testABunch". It often failed around the 250th run.
+- (void)testABunch {
+  for (int i = 0; i < 1000; i++) {
+    NSLog(@"test run..");
+    [self testScrollPositionRestoring];
+    [self tearDown];
+    [self setUp];
+  }
+}


I also experimented with doing a down swipe to make sure it's at the top of the page, and I never saw a failure with that logic. However, I didn't run it enough to be certain. I added the following code after each loadURL:
+  [[EarlGrey selectElementWithMatcher:web::WebViewScrollView()]
+   performAction:grey_swipeFastInDirection(kGREYDirectionDown)];


Finally, I experimented with checking if the scroll position was 0,0 after page load, and it was not always the case. Perhaps it's a synchronization issue (not waiting for page to be rendered and scroll adjusted)?
+  NSError* error = nil;
+  [[EarlGrey selectElementWithMatcher:web::WebViewScrollView()]
+   assertWithMatcher:grey_scrollViewContentOffset(CGPointMake(0, 0))
+   error:&error];
+  if (error) {
+    NSLog(@"NOT (0,0)!!!");
+  }
Components: Mobile>WebView>Glue
Labels: -Pri-2 Pri-1
We know that users complain about scroll position, so this test could give us some clue to address the problem. Specifically it's weird that just loaded page appears to be scrolled.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 17 2017

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

commit f99ae39b7445952db64971ae798c8dc83b5776f1
Author: baxley <baxley@chromium.org>
Date: Fri Mar 17 00:53:31 2017

Mark testScrollPositionRestoring as FLAKY.

This test appears to scroll to far on some runs.

BUG= 702410 

Review-Url: https://codereview.chromium.org/2755973004
Cr-Commit-Position: refs/heads/master@{#457636}

[modify] https://crrev.com/f99ae39b7445952db64971ae798c8dc83b5776f1/ios/web/shell/test/page_state_egtest.mm

Any update here?
Status: Started (was: Assigned)

Comment 7 by cma...@chromium.org, Apr 10 2017

ping!
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 19 2017

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

commit a6618dc84a89e04cad9c0badf3097b8af91c7795
Author: kkhorimoto <kkhorimoto@chromium.org>
Date: Wed Apr 19 23:55:11 2017

Fixed PageStateTest.testScrollPositionRestoring flake.

This CL adds a step where we scoll to the top whenever a page is loaded,
ensuring that the scroll actions end at the expected offset.

BUG= 702410 

Review-Url: https://codereview.chromium.org/2805573002
Cr-Commit-Position: refs/heads/master@{#465815}

[modify] https://crrev.com/a6618dc84a89e04cad9c0badf3097b8af91c7795/ios/web/shell/test/page_state_egtest.mm

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

Comment 10 by sheriffbot@chromium.org, Apr 19 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

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

Comment 11 by sheriffbot@chromium.org, Apr 24 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 12 by sheriffbot@chromium.org, Apr 27 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 13 by bugdroid1@chromium.org, May 8 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9e512ff4dd1839c9d66904d052d5f030e12e953f

commit 9e512ff4dd1839c9d66904d052d5f030e12e953f
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Mon May 08 22:06:55 2017

Fixed PageStateTest.testScrollPositionRestoring flake.

This CL adds a step where we scoll to the top whenever a page is loaded,
ensuring that the scroll actions end at the expected offset.

BUG= 702410 

Review-Url: https://codereview.chromium.org/2805573002
Cr-Commit-Position: refs/heads/master@{#465815}
(cherry picked from commit a6618dc84a89e04cad9c0badf3097b8af91c7795)

Review-Url: https://codereview.chromium.org/2868983004 .
Cr-Commit-Position: refs/branch-heads/3071@{#471}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/9e512ff4dd1839c9d66904d052d5f030e12e953f/ios/web/shell/test/page_state_egtest.mm

Sign in to add a comment