Fixed PageStateTest.testScrollPositionRestoring flake.

ios/web/shell/test/page_state_egtest.mm

Index: ios/web/shell/test/page_state_egtest.mm
diff --git a/ios/web/shell/test/page_state_egtest.mm b/ios/web/shell/test/page_state_egtest.mm
index ab9a244f1f4bb00b8a0e0236bbf2bd8d9909064d..37aef66e888639359eec186df5be334d31d09163 100644
--- a/ios/web/shell/test/page_state_egtest.mm
+++ b/ios/web/shell/test/page_state_egtest.mm
@@ -53,6 +53,17 @@ void WaitForOffset(CGFloat y_offset) {
   GREYAssert([condition waitWithTimeout:10], error_text);
 }
 
+// Loads the long page at |url|, scrolls to the top, and waits for the offset to
+// be {0, 0} before returning.
+void LoadLongPageAndScrollToTop(const GURL& url) {
+  // Load the page and swipe down.
+  [ShellEarlGrey loadURL:url];
+  [[EarlGrey selectElementWithMatcher:web::WebViewScrollView()]
+      performAction:grey_swipeFastInDirection(kGREYDirectionDown)];

[baxley, 2017/04/06 13:02:39]

Can you use grey_scrollToContentEdge instead? grey_swipeFastInDirection doesn't
guarantee to reach the content edge, which has bitten us before. Unfortunately
that method is much slower If it's a long distance you need to scroll up. You
can leave the method call you have and then do a second call to
grey_scrollToContentEdge.

[Eugene But (OOO till 7-30), 2017/04/06 15:28:46]

Why do we need to scroll the page to the top? If the page is loaded with
non-zero offset it sounds like a product bug, not a feature.

[kkhorimoto, 2017/04/06 23:48:10]

I've updated to use scrollToContentEdge.

I agree that it's a bug if the page is scrolled down upon loading.  I tried to
investigate, but I haven't been able to reproduce, even when running the test
1000 times.  I've created a bug (crbug.com/709278) to capture this; can you
populate it with any details you have?  You mentioned that there were other
reports of this issue.  This test is about page scroll state restoration, so
it's a different issue than a non CGPointZero contentOffset on load.

I should also note that these tests have historically been flakier because
there's no API to tell when the page has finished rendering; adding steps to
ensure the expected state before testing the actual restoration functionality
seems like a good option here.

[Eugene But (OOO till 7-30), 2017/04/07 00:45:49]

Does this test actually fail on Flaky bots? Should we update
|waitForPageToFinishLoading| to assert that page is not scrolled after the load?
Also we can add a temporary DCHECK to webView:didFinishNavigation: to assert
that page is not scrolled.

[kkhorimoto, 2017/04/12 05:03:28]

I've investigated, and it turns out that that's not always the case; the content
offset is updated by webkit after the didFinishNavigation: call :(

+  // Waits for the {0, 0} offset.
+  WaitForOffset(0.0);

[baxley, 2017/04/06 13:02:38]

This may not be necessary if we do the call to grey_scrollToContentEdge. But I
won't object to having this second check if this helps make sure it passes, or
at least fail here so we know what the problem is.

[kkhorimoto, 2017/04/06 23:48:10]

I left it in since at best it will help reduce flake, and at worst it'll be a
no-op.

+}
+
 }  // namespace
 
 using web::test::HttpServer;
@@ -65,13 +76,11 @@ @implementation PageStateTestCase
 
 // Tests that page scroll position of a page is restored upon returning to the
 // page via the back/forward buttons.
-// TODO(crbug.com/702410): This test flakily fails when it scrolls too far down
-// the page.
-- (void)FLAKY_testScrollPositionRestoring {
+- (void)testScrollPositionRestoring {
   web::test::SetUpFileBasedHttpServer();
 
   // Load first URL which is a long page.

[baxley, 2017/04/06 13:02:39]

Do you want to delete this comment? I think the new method call makes it clear.

[kkhorimoto, 2017/04/06 23:48:10]

Done.

-  [ShellEarlGrey loadURL:HttpServer::MakeUrl(kLongPage1)];
+  LoadLongPageAndScrollToTop(HttpServer::MakeUrl(kLongPage1));
 
   // Scroll the first page and verify the offset.
   [[EarlGrey selectElementWithMatcher:web::WebViewScrollView()]
@@ -80,7 +89,7 @@ - (void)FLAKY_testScrollPositionRestoring {
       assertWithMatcher:grey_scrollViewContentOffset(CGPointMake(0, kOffset1))];
 
   // Load second URL, which is also a long page.

[baxley, 2017/04/06 13:02:39]

Delete comment?

[kkhorimoto, 2017/04/06 23:48:10]

Done.

-  [ShellEarlGrey loadURL:HttpServer::MakeUrl(kLongPage2)];
+  LoadLongPageAndScrollToTop(HttpServer::MakeUrl(kLongPage2));
 
   // Scroll the second page and verify the offset.
   [[EarlGrey selectElementWithMatcher:web::WebViewScrollView()]