fix: promise resolved to early when browser initiated in-page navigation

[TomaszMa]

Description of Change

After this fix #36129 promise returned by loadURL function can be resolved before page finish loading. While page is loading it can trigger browser intendent in page navigation (e.g.: window.history.replaceState ). This PR fixes this problem.

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• tests are changed or added
• relevant documentation, tutorials, templates and examples are changed or added
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Fix problem with promise resolved to early when browser intendent in-page navigation

[TomaszMa]

@nornagon can you review this fix/change ? Thanks.

[zcbenz]

Can you fix the lint warnings?

/home/builduser/project/src/electron/lib/browser/api/web-contents.ts
  450:9   error  Identifier 'browser_intendent_in_page_navigation' is not in camel case  camelcase
  465:9   error  Identifier 'browser_intendent_in_page_navigation' is not in camel case  camelcase
  481:12  error  Identifier 'browser_intendent_in_page_navigation' is not in camel case  camelcase

/home/builduser/project/src/electron/spec/api-web-contents-spec.ts
  380:51  error  Missing space before function parentheses      space-before-function-paren
  381:1   error  Expected indentation of 8 spaces but found 10  indent

The code looks good to me.

[TomaszMa]

Can you fix the lint warnings?

/home/builduser/project/src/electron/lib/browser/api/web-contents.ts
  450:9   error  Identifier 'browser_intendent_in_page_navigation' is not in camel case  camelcase
  465:9   error  Identifier 'browser_intendent_in_page_navigation' is not in camel case  camelcase
  481:12  error  Identifier 'browser_intendent_in_page_navigation' is not in camel case  camelcase

/home/builduser/project/src/electron/spec/api-web-contents-spec.ts
  380:51  error  Missing space before function parentheses      space-before-function-paren
  381:1   error  Expected indentation of 8 spaces but found 10  indent

The code looks good to me.

Warnings fixed.

[codebytere]

@TomaszMa can you clarify what is meant by "browser intendent navigation"? I'm not sure I understand what that term refers to and haven't seen it before.

[TomaszMa]

@TomaszMa can you clarify what is meant by "browser intendent navigation"? I'm not sure I understand what that term refers to and haven't seen it before.

browser intendent navigation - navigation triggered by browser, e.g.: loading resources like images . Navigation indirectly caused by user. Some of the browser intendent navigation can trigger 'did-navigate-in-page' event, e.g.: window.history.replaceState . google.com page calls window.history.replaceState which triggers 'did-navigate-in-page'.

user intendent navigation - navigation triggered by user, e.g.: BrowserWindow.loadURL, BrowserWindow.loadFile, or user clicking on hyper link. Every navigation directly caused by user.

Problem is that when we call BrowserWindow.loadFile (user intendent navigation) we get Promise which is resolved before page will load because there can be browser intendent in page navigation. This PR fixes this problem.

[codebytere]

@TomaszMa ok thanks. I was asking mostly because "intendent" isn't really a term that's used. We should something that more clearly conveys what's going on - browserInitiated and userInitiated are clearer. Let's instead say browserInitiatedInPageNavigation and finishListenerWhenUserIntendentNavigation.

[TomaszMa]

@codebytere changed :)

[codebytere]

thanks :D

[release-clerk]

Release Notes Persisted

Fix problem with promise resolved to early when browser intendent in-page navigation

[welcome]

Congrats on merging your first pull request! 🎉🎉🎉

[trop]

I have automatically backported this PR to "24-x-y", please check out #39432

[trop]

I have automatically backported this PR to "26-x-y", please check out #39433

[trop]

I have automatically backported this PR to "25-x-y", please check out #39434

[codebytere]

I think we should revert this - it's causing consistent failures on Linux cc @zcbenz

Example: https://app.circleci.com/pipelines/github/electron/electron/73291/workflows/c87fabd1-c799-4c0b-afb3-9c4830707e4f/jobs/1582308/tests

[TomaszMa]

I think we should revert this - it's causing consistent failures on Linux cc @zcbenz

Example: https://app.circleci.com/pipelines/github/electron/electron/73291/workflows/c87fabd1-c799-4c0b-afb3-9c4830707e4f/jobs/1582308/tests

Test during integration passed, so or added test on linux is unstable, or something else break the test.
@codebytere I can look on this.