feat: webContents.loadURL returns a promise#15855
Conversation
775952e to
cfdca61
Compare
| navigationStarted = true | ||
| } | ||
| } | ||
| const stopLoadingListener = () => { |
There was a problem hiding this comment.
We could instead use content::ChildProcessSecurityPolicy::CanRequestURL to determine if a navigation will happen and abort with did-fail-load on our end for these types of cases.
There was a problem hiding this comment.
I figured this would be a reasonable "catch-all", since I think did-stop-loading will be emitted regardless of what happens?
There was a problem hiding this comment.
but CanRequestURL might be a good extra check to give a more informative error
There was a problem hiding this comment.
since I think did-stop-loading will be emitted regardless of what happens?
yup that is correct, its emitted whenever a document stops its resource loading be it failure or success. Having this check can simplify existing url validity check https://github.com/electron/electron/blob/master/atom/browser/api/atom_api_web_contents.cc#L1157
cfdca61 to
886aefc
Compare
|
Release Notes Persisted
|
Description of Change
Many tests use the following pattern to load a page in a WebContents and wait for the load to finish:
This is an aesthetic improvement over the callback method of
webContents.on('did-finish-load', ...), but is unfortunately flaky: it is sometimes possible that the'did-finish-load'event is emitted before the listener is registered (see #15853 for example, which fixes the flaky test described in #15095). This is to do with the fact that the tests use theBrowserWindowmodule via theremoteAPI, so there is a short window of time after loading the URL but before the event listener is registered. If the event is fired during that window, it will be lost, and the test times out.It's possible to eliminate the additional
emittedOncehelper call, clean up the code, and remove the possibility of flakiness by havingloadURLreturn a promise. The above could would be written as follows with this patch:Checklist
npm testpassesRelease Notes
Notes: webContents.loadURL and loadFile now return a promise