Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2805573002:
Fixed PageStateTest.testScrollPositionRestoring flake. (Closed)
|
Created:
3 years, 8 months ago by kkhorimoto Modified:
3 years, 8 months ago CC:
chromium-reviews, ios-reviews+web_chromium.org, ios-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionFixed 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}
Committed: https://chromium.googlesource.com/chromium/src/+/a6618dc84a89e04cad9c0badf3097b8af91c7795
Patch Set 1 #
Total comments: 11
Patch Set 2 : baxley's comments #
Total comments: 2
Patch Set 3 : rename function #Messages
Total messages: 22 (6 generated)
kkhorimoto@chromium.org changed reviewers: + baxley@chromium.org
https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... File ios/web/shell/test/page_state_egtest.mm (right): https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:62: performAction:grey_swipeFastInDirection(kGREYDirectionDown)]; 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. https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:64: WaitForOffset(0.0); 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. https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:82: // Load first URL which is a long page. Do you want to delete this comment? I think the new method call makes it clear. https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:91: // Load second URL, which is also a long page. Delete comment?
eugenebut@chromium.org changed reviewers: + eugenebut@chromium.org
https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... File ios/web/shell/test/page_state_egtest.mm (right): https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:62: performAction:grey_swipeFastInDirection(kGREYDirectionDown)]; On 2017/04/06 13:02:39, baxley wrote: > 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. 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.
https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... File ios/web/shell/test/page_state_egtest.mm (right): https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:62: performAction:grey_swipeFastInDirection(kGREYDirectionDown)]; On 2017/04/06 15:28:46, Eugene But wrote: > On 2017/04/06 13:02:39, baxley wrote: > > 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. > 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. 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. https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:64: WaitForOffset(0.0); On 2017/04/06 13:02:38, baxley wrote: > 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. I left it in since at best it will help reduce flake, and at worst it'll be a no-op. https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:82: // Load first URL which is a long page. On 2017/04/06 13:02:39, baxley wrote: > Do you want to delete this comment? I think the new method call makes it clear. Done. https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:91: // Load second URL, which is also a long page. On 2017/04/06 13:02:39, baxley wrote: > Delete comment? Done.
https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... File ios/web/shell/test/page_state_egtest.mm (right): https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:62: performAction:grey_swipeFastInDirection(kGREYDirectionDown)]; On 2017/04/06 23:48:10, kkhorimoto_ wrote: > On 2017/04/06 15:28:46, Eugene But wrote: > > On 2017/04/06 13:02:39, baxley wrote: > > > 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. > > 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. > > 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. 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.
On 2017/04/07 00:45:49, Eugene But wrote: > https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... > File ios/web/shell/test/page_state_egtest.mm (right): > > https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... > ios/web/shell/test/page_state_egtest.mm:62: > performAction:grey_swipeFastInDirection(kGREYDirectionDown)]; > On 2017/04/06 23:48:10, kkhorimoto_ wrote: > > On 2017/04/06 15:28:46, Eugene But wrote: > > > On 2017/04/06 13:02:39, baxley wrote: > > > > 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. > > > 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. +1 > > > > 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. > 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. I was able to reproduce it reasonably well (I have info in the bug). I'm not sure if anything has changed since I last looked. I can try again tomorrow to see if I have better luck. Ping me if you want me to look.
On 2017/04/07 00:48:46, baxley wrote: > On 2017/04/07 00:45:49, Eugene But wrote: > > > https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... > > File ios/web/shell/test/page_state_egtest.mm (right): > > > > > https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... > > ios/web/shell/test/page_state_egtest.mm:62: > > performAction:grey_swipeFastInDirection(kGREYDirectionDown)]; > > On 2017/04/06 23:48:10, kkhorimoto_ wrote: > > > On 2017/04/06 15:28:46, Eugene But wrote: > > > > On 2017/04/06 13:02:39, baxley wrote: > > > > > 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. > > > > 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. > +1 > > > > > > > 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. > > 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. > > I was able to reproduce it reasonably well (I have info in the bug). I'm not > sure if anything has changed since I last looked. I can try again tomorrow to > see if I have better luck. Ping me if you want me to look. When I first created the fix for this flake, I was able to reproduce by doing baxley's |-testABunch| strategy that he details in the bug. However, when I revisited yesterday, I did multiple 1000x runs, and wasn't able to reproduce the load-at-offset bug. I'm going to go ahead and add the |-webView:didFinishNavigation:| DCHECK, so at least we can tell if it's a WKWebView bug or resulting from fullscreen/pull-to-action code.
On 2017/04/07 17:57:53, kkhorimoto_ wrote: > On 2017/04/07 00:48:46, baxley wrote: > > On 2017/04/07 00:45:49, Eugene But wrote: > > > > > > https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... > > > File ios/web/shell/test/page_state_egtest.mm (right): > > > > > > > > > https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... > > > ios/web/shell/test/page_state_egtest.mm:62: > > > performAction:grey_swipeFastInDirection(kGREYDirectionDown)]; > > > On 2017/04/06 23:48:10, kkhorimoto_ wrote: > > > > On 2017/04/06 15:28:46, Eugene But wrote: > > > > > On 2017/04/06 13:02:39, baxley wrote: > > > > > > 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. > > > > > 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. > > +1 > > > > > > > > > > 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. > > > 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. > > > > I was able to reproduce it reasonably well (I have info in the bug). I'm not > > sure if anything has changed since I last looked. I can try again tomorrow to > > see if I have better luck. Ping me if you want me to look. > > When I first created the fix for this flake, I was able to reproduce by doing > baxley's |-testABunch| strategy that he details in the bug. However, when I > revisited yesterday, I did multiple 1000x runs, and wasn't able to reproduce the > load-at-offset bug. I'm going to go ahead and add the > |-webView:didFinishNavigation:| DCHECK, so at least we can tell if it's a > WKWebView bug or resulting from fullscreen/pull-to-action code. Can we disable fullscreen and pull to action features and try reproducing (assuming that we still can repro this)?
On 2017/04/07 18:04:33, Eugene But wrote: > On 2017/04/07 17:57:53, kkhorimoto_ wrote: > > On 2017/04/07 00:48:46, baxley wrote: > > > On 2017/04/07 00:45:49, Eugene But wrote: > > > > > > > > > > https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... > > > > File ios/web/shell/test/page_state_egtest.mm (right): > > > > > > > > > > > > > > https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... > > > > ios/web/shell/test/page_state_egtest.mm:62: > > > > performAction:grey_swipeFastInDirection(kGREYDirectionDown)]; > > > > On 2017/04/06 23:48:10, kkhorimoto_ wrote: > > > > > On 2017/04/06 15:28:46, Eugene But wrote: > > > > > > On 2017/04/06 13:02:39, baxley wrote: > > > > > > > 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. > > > > > > 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. > > > +1 > > > > > > > > > > > > > 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. > > > > 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. > > > > > > I was able to reproduce it reasonably well (I have info in the bug). I'm not > > > sure if anything has changed since I last looked. I can try again tomorrow > to > > > see if I have better luck. Ping me if you want me to look. > > > > When I first created the fix for this flake, I was able to reproduce by doing > > baxley's |-testABunch| strategy that he details in the bug. However, when I > > revisited yesterday, I did multiple 1000x runs, and wasn't able to reproduce > the > > load-at-offset bug. I'm going to go ahead and add the > > |-webView:didFinishNavigation:| DCHECK, so at least we can tell if it's a > > WKWebView bug or resulting from fullscreen/pull-to-action code. > Can we disable fullscreen and pull to action features and try reproducing > (assuming that we still can repro this)? This test is on the web shell, which doesn't have either of these features. Also, as I noted in my reply to your comment, the content offset is updated after the didFinishNaivgation call, so we can't use that DCHECK.
https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... File ios/web/shell/test/page_state_egtest.mm (right): https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... ios/web/shell/test/page_state_egtest.mm:62: performAction:grey_swipeFastInDirection(kGREYDirectionDown)]; On 2017/04/07 00:45:49, Eugene But wrote: > On 2017/04/06 23:48:10, kkhorimoto_ wrote: > > On 2017/04/06 15:28:46, Eugene But wrote: > > > On 2017/04/06 13:02:39, baxley wrote: > > > > 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. > > > 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. > > > > 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. > 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. 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 :(
On 2017/04/12 05:03:29, kkhorimoto_ wrote: > https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... > File ios/web/shell/test/page_state_egtest.mm (right): > > https://codereview.chromium.org/2805573002/diff/1/ios/web/shell/test/page_sta... > ios/web/shell/test/page_state_egtest.mm:62: > performAction:grey_swipeFastInDirection(kGREYDirectionDown)]; > On 2017/04/07 00:45:49, Eugene But wrote: > > On 2017/04/06 23:48:10, kkhorimoto_ wrote: > > > On 2017/04/06 15:28:46, Eugene But wrote: > > > > On 2017/04/06 13:02:39, baxley wrote: > > > > > 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. > > > > 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. > > > > > > 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. > > 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. > > 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 :( Is this reproducible with stock WKWebView or only with ios_web_shell, which has bunch of chrome specific scripts injected?
Per offline conversation, a separate test that verifies correct scroll position after the load will be added. lgtm https://codereview.chromium.org/2805573002/diff/20001/ios/web/shell/test/page... File ios/web/shell/test/page_state_egtest.mm (right): https://codereview.chromium.org/2805573002/diff/20001/ios/web/shell/test/page... ios/web/shell/test/page_state_egtest.mm:58: void LoadLongPageAndScrollToTop(const GURL& url) { "And" word in a method name is kind of a red flag. Could you please change it to ScrollToTop.
https://codereview.chromium.org/2805573002/diff/20001/ios/web/shell/test/page... File ios/web/shell/test/page_state_egtest.mm (right): https://codereview.chromium.org/2805573002/diff/20001/ios/web/shell/test/page... ios/web/shell/test/page_state_egtest.mm:58: void LoadLongPageAndScrollToTop(const GURL& url) { On 2017/04/19 22:48:27, Eugene But wrote: > "And" word in a method name is kind of a red flag. Could you please change it to > ScrollToTop. Done.
rename function
The CQ bit was checked by kkhorimoto@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from eugenebut@chromium.org Link to the patchset: https://codereview.chromium.org/2805573002/#ps40001 (title: "rename function")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492645403723010,
"parent_rev": "3278d07f94c85839bac14d4f9495056c82977b56", "commit_rev":
"a6618dc84a89e04cad9c0badf3097b8af91c7795"}
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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} Committed: https://chromium.googlesource.com/chromium/src/+/a6618dc84a89e04cad9c0badf309... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/a6618dc84a89e04cad9c0badf309... |
