feat: webContents.loadURL returns a promise (#15855)

docs/api/browser-window.md

@@ -1241,7 +1241,11 @@ Captures a snapshot of the page within `rect`. Omitting `rect` will capture the
   * `postData` ([UploadRawData[]](structures/upload-raw-data.md) | [UploadFile[]](structures/upload-file.md) | [UploadBlob[]](structures/upload-blob.md)) (optional)
   * `baseURLForDataURL` String (optional) - Base url (with trailing path separator) for files to be loaded by the data url. This is needed only if the specified `url` is a data url and needs to load other files.
 
-Same as `webContents.loadURL(url[, options])`.
+Returns `Promise<void>` - the promise will resolve when the page has finished loading
+(see [`did-finish-load`](web-contents.md#event-did-finish-load)), and rejects
+if the page fails to load (see [`did-fail-load`](web-contents.md#event-did-fail-load)).
+
+Same as [`webContents.loadURL(url[, options])`](web-contents.md#contentsloadurlurl-options).
 
 The `url` can be a remote address (e.g. `http://`) or a path to a local
 HTML file using the `file://` protocol.
@@ -1281,6 +1285,10 @@ win.loadURL('http://localhost:8000/post', {
   * `search` String (optional) - Passed to `url.format()`.
   * `hash` String (optional) - Passed to `url.format()`.
 
+Returns `Promise<void>` - the promise will resolve when the page has finished loading
+(see [`did-finish-load`](web-contents.md#event-did-finish-load)), and rejects
+if the page fails to load (see [`did-fail-load`](web-contents.md#event-did-fail-load)).
+
 Same as `webContents.loadFile`, `filePath` should be a path to an HTML
 file relative to the root of your application.  See the `webContents` docs
 for more information.

docs/api/web-contents.md

@@ -697,6 +697,11 @@ Custom value can be returned by setting `event.returnValue`.
   * `postData` ([UploadRawData[]](structures/upload-raw-data.md) | [UploadFile[]](structures/upload-file.md) | [UploadBlob[]](structures/upload-blob.md)) (optional)
   * `baseURLForDataURL` String (optional) - Base url (with trailing path separator) for files to be loaded by the data url. This is needed only if the specified `url` is a data url and needs to load other files.
 
+Returns `Promise<void>` - the promise will resolve when the page has finished loading
+(see [`did-finish-load`](web-contents.md#event-did-finish-load)), and rejects
+if the page fails to load (see
+[`did-fail-load`](web-contents.md#event-did-fail-load)).
+
 Loads the `url` in the window. The `url` must contain the protocol prefix,
 e.g. the `http://` or `file://`. If the load should bypass http cache then
 use the `pragma` header to achieve it.
@@ -715,6 +720,10 @@ webContents.loadURL('https://github.com', options)
   * `search` String (optional) - Passed to `url.format()`.
   * `hash` String (optional) - Passed to `url.format()`.
 
+Returns `Promise<void>` - the promise will resolve when the page has finished loading
+(see [`did-finish-load`](web-contents.md#event-did-finish-load)), and rejects
+if the page fails to load (see [`did-fail-load`](web-contents.md#event-did-fail-load)).
+
 Loads the given file in the window, `filePath` should be a path to
 an HTML file relative to the root of your application.  For instance
 an app structure like this:

lib/browser/navigation-controller.js

@@ -63,9 +63,66 @@ const NavigationController = (function () {
     if (options == null) {
       options = {}
     }
+    const p = new Promise((resolve, reject) => {
+      const resolveAndCleanup = () => {
+        removeListeners()
+        resolve()
+      }
+      const rejectAndCleanup = (errorCode, errorDescription, url) => {
+        const err = new Error(`${errorDescription} (${errorCode}) loading '${url}'`)
+        Object.assign(err, { errno: errorCode, code: errorDescription, url })
+        removeListeners()
+        reject(err)
+      }
+      const finishListener = () => {
+        resolveAndCleanup()
+      }
+      const failListener = (event, errorCode, errorDescription, validatedURL, isMainFrame, frameProcessId, frameRoutingId) => {
+        if (isMainFrame) {
+          rejectAndCleanup(errorCode, errorDescription, validatedURL)
+        }
+      }
+
+      let navigationStarted = false
+      const navigationListener = (event, url, isSameDocument, isMainFrame, frameProcessId, frameRoutingId, navigationId) => {
+        if (isMainFrame) {
+          if (navigationStarted) {
+            // the webcontents has started another unrelated navigation in the
+            // main frame (probably from the app calling `loadURL` again); reject
+            // the promise
+            return rejectAndCleanup(-3, 'ERR_ABORTED', url)
+          }
+          navigationStarted = true
+        }
+      }
+      const stopLoadingListener = () => {
+        // By the time we get here, either 'finish' or 'fail' should have fired
+        // if the navigation occurred. However, in some situations (e.g. when
+        // attempting to load a page with a bad scheme), loading will stop
+        // without emitting finish or fail. In this case, we reject the promise
+        // with a generic failure.
+        // TODO(jeremy): enumerate all the cases in which this can happen. If
+        // the only one is with a bad scheme, perhaps ERR_INVALID_ARGUMENT
+        // would be more appropriate.
+        rejectAndCleanup(-2, 'ERR_FAILED', url)
+      }
+      const removeListeners = () => {
+        this.webContents.removeListener('did-finish-load', finishListener)
+        this.webContents.removeListener('did-fail-load', failListener)
+        this.webContents.removeListener('did-start-navigation', navigationListener)
+        this.webContents.removeListener('did-stop-loading', stopLoadingListener)
+      }
+      this.webContents.on('did-finish-load', finishListener)
+      this.webContents.on('did-fail-load', failListener)
+      this.webContents.on('did-start-navigation', navigationListener)
+      this.webContents.on('did-stop-loading', stopLoadingListener)
+    })
+    // Add a no-op rejection handler to silence the unhandled rejection error.
+    p.catch(() => {})
     this.pendingIndex = -1
     this.webContents._loadURL(url, options)
-    return this.webContents.emit('load-url', url, options)
+    this.webContents.emit('load-url', url, options)
+    return p
   }
 
   NavigationController.prototype.getURL = function () {

spec/api-ipc-renderer-spec.js

@@ -5,7 +5,6 @@ const dirtyChai = require('dirty-chai')
 const http = require('http')
 const path = require('path')
 const { closeWindow } = require('./window-helpers')
-const { emittedOnce } = require('./events-helpers')
 
 const { expect } = chai
 chai.use(dirtyChai)
@@ -229,9 +228,7 @@ describe('ipc renderer module', () => {
   describe('ipcRenderer.on', () => {
     it('is not used for internals', async () => {
       w = new BrowserWindow({ show: false })
-      w.loadURL('about:blank')
-
-      await emittedOnce(w.webContents, 'did-finish-load')
+      await w.loadURL('about:blank')
 
       const script = `require('electron').ipcRenderer.eventNames()`
       const result = await w.webContents.executeJavaScript(script)

spec/api-remote-spec.js

@@ -429,9 +429,10 @@ describe('remote module', () => {
     })
 
     it('emits unhandled rejection events in the renderer process', (done) => {
-      window.addEventListener('unhandledrejection', function (event) {
+      window.addEventListener('unhandledrejection', function handler (event) {
         event.preventDefault()
         assert.strictEqual(event.reason.message, 'rejected')
+        window.removeEventListener('unhandledrejection', handler)
         done()
       })
 

spec/api-web-contents-spec.js

@@ -39,6 +39,105 @@ describe('webContents module', () => {
 
   afterEach(() => closeWindow(w).then(() => { w = null }))
 
+  describe('loadURL() promise API', () => {
+    it('resolves when done loading', async () => {
+      await expect(w.loadURL('about:blank')).to.eventually.be.fulfilled
+    })
+
+    it('resolves when done loading a file URL', async () => {
+      await expect(w.loadFile(path.join(fixtures, 'pages', 'base-page.html'))).to.eventually.be.fulfilled
+    })
+
+    it('rejects when failing to load a file URL', async () => {
+      await expect(w.loadURL('file:non-existent')).to.eventually.be.rejected
+        .and.have.property('code', 'ERR_FILE_NOT_FOUND')
+    })
+
+    it('rejects when loading fails due to DNS not resolved', async () => {
+      await expect(w.loadURL('https://err.name.not.resolved')).to.eventually.be.rejected
+        .and.have.property('code', 'ERR_NAME_NOT_RESOLVED')
+    })
+
+    it('rejects when navigation is cancelled due to a bad scheme', async () => {
+      await expect(w.loadURL('bad-scheme://foo')).to.eventually.be.rejected
+        .and.have.property('code', 'ERR_FAILED')
+    })
+
+    it('sets appropriate error information on rejection', async () => {
+      let err
+      try {
+        await w.loadURL('file:non-existent')
+      } catch (e) {
+        err = e
+      }
+      expect(err).not.to.be.null()
+      expect(err.code).to.eql('ERR_FILE_NOT_FOUND')
+      expect(err.errno).to.eql(-6)
+      expect(err.url).to.eql(process.platform === 'win32' ? 'file://non-existent/' : 'file:///non-existent')
+    })
+
+    it('rejects if the load is aborted', async () => {
+      const s = http.createServer((req, res) => { /* never complete the request */ })
+      await new Promise(resolve => s.listen(0, '127.0.0.1', resolve))
+      const { port } = s.address()
+      const p = expect(w.loadURL(`http://127.0.0.1:${port}`)).to.eventually.be.rejectedWith(Error, /ERR_ABORTED/)
+      // load a different file before the first load completes, causing the
+      // first load to be aborted.
+      await w.loadFile(path.join(fixtures, 'pages', 'base-page.html'))
+      await p
+      s.close()
+    })
+
+    it("doesn't reject when a subframe fails to load", async () => {
+      let resp = null
+      const s = http.createServer((req, res) => {
+        res.writeHead(200, { 'Content-Type': 'text/html' })
+        res.write('<iframe src="http://err.name.not.resolved"></iframe>')
+        resp = res
+        // don't end the response yet
+      })
+      await new Promise(resolve => s.listen(0, '127.0.0.1', resolve))
+      const { port } = s.address()
+      const p = new Promise(resolve => {
+        w.webContents.on('did-fail-load', (event, errorCode, errorDescription, validatedURL, isMainFrame, frameProcessId, frameRoutingId) => {
+          if (!isMainFrame) {
+            resolve()
+          }
+        })
+      })
+      const main = w.loadURL(`http://127.0.0.1:${port}`)
+      await p
+      resp.end()
+      await main
+      s.close()
+    })
+
+    it("doesn't resolve when a subframe loads", async () => {
+      let resp = null
+      const s = http.createServer((req, res) => {
+        res.writeHead(200, { 'Content-Type': 'text/html' })
+        res.write('<iframe src="data:text/html,hi"></iframe>')
+        resp = res
+        // don't end the response yet
+      })
+      await new Promise(resolve => s.listen(0, '127.0.0.1', resolve))
+      const { port } = s.address()
+      const p = new Promise(resolve => {
+        w.webContents.on('did-frame-finish-load', (event, errorCode, errorDescription, validatedURL, isMainFrame, frameProcessId, frameRoutingId) => {
+          if (!isMainFrame) {
+            resolve()
+          }
+        })
+      })
+      const main = w.loadURL(`http://127.0.0.1:${port}`)
+      await p
+      resp.destroy() // cause the main request to fail
+      await expect(main).to.eventually.be.rejected
+        .and.have.property('errno', -355) // ERR_INCOMPLETE_CHUNKED_ENCODING
+      s.close()
+    })
+  })
+
   describe('getAllWebContents() API', () => {
     it('returns an array of web contents', (done) => {
       w.webContents.on('devtools-opened', () => {
@@ -978,9 +1077,7 @@ describe('webContents module', () => {
         }
       })
 
-      const p = emittedOnce(w.webContents, 'did-finish-load')
-      w.loadURL('about:blank')
-      await p
+      await w.loadURL('about:blank')
 
       const filePath = path.join(remote.app.getPath('temp'), 'test.heapsnapshot')
 
@@ -1010,9 +1107,7 @@ describe('webContents module', () => {
         }
       })
 
-      const p = emittedOnce(w.webContents, 'did-finish-load')
-      w.loadURL('about:blank')
-      await p
+      await w.loadURL('about:blank')
 
       const promise = w.webContents.takeHeapSnapshot('')
       return expect(promise).to.be.eventually.rejectedWith(Error, 'takeHeapSnapshot failed')