Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(navigation): do not count random failures as navigation cancel #1055

Merged
merged 1 commit into from
Feb 19, 2020
Merged

fix(navigation): do not count random failures as navigation cancel #1055

merged 1 commit into from
Feb 19, 2020

Conversation

dgozman
Copy link
Contributor

@dgozman dgozman commented Feb 18, 2020

In WebKit, we sometimes get the following:

  • page starts a navigation in some frame;
  • we issue page.goto();
  • new navigation commits;
  • old page is still alive and cancels the frame navigation;
  • while waiting for 'Page.frameNavigated' on the frontend from the new target, we receive 'Browser.provisionalLoadFailed' for the frame navigation from the old target;
  • this counts as cancel for the new navigation, although it was clearly a cancellation of some other navigation.

Wasn't able to write a reliable test. Any ideas are welcome! Fixes #1044.

@yury-s
Copy link
Member

yury-s commented Feb 18, 2020

As for testing this, what if we have two consecutive .goto and check that first is cancelled by the second while the second succeedes? The first request could be deferred on the server for better reliability

Copy link
Contributor

@JoelEinbinder JoelEinbinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good. Idk about the test.

@dgozman
Copy link
Contributor Author

dgozman commented Feb 18, 2020

As for testing this, what if we have two consecutive .goto and check that first is cancelled by the second while the second succeedes? The first request could be deferred on the server for better reliability

We have 'should fail when canceled by another navigation' which does exactly that. Unfortunately, the issue is slightly different here: first navigation is canceled and that mistakenly "cancels" the second one.

@dgozman dgozman merged commit 9f1edad into microsoft:master Feb 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REGRESSION]: Webkit multiple page navigations
3 participants