From f3d391e3f23ec486f43ba8421f31ca37852fa196 Mon Sep 17 00:00:00 2001
From: Kevin Sawicki <kevinsawicki@gmail.com>
Date: Wed, 30 Nov 2016 16:07:02 -0800
Subject: [PATCH 1/3] Don't clear until render view is deleted for process id

---
 lib/browser/objects-registry.js | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/lib/browser/objects-registry.js b/lib/browser/objects-registry.js
index aeaf5351dc..048fe856a2 100644
--- a/lib/browser/objects-registry.js
+++ b/lib/browser/objects-registry.js
@@ -19,17 +19,14 @@ class ObjectsRegistry {
   // registered then the already assigned ID would be returned.
   add (webContents, obj) {
     // Get or assign an ID to the object.
-    let id = this.saveToStorage(obj)
+    const id = this.saveToStorage(obj)
 
     // Add object to the set of referenced objects.
-    let webContentsId = webContents.getId()
+    const webContentsId = webContents.getId()
     let owner = this.owners[webContentsId]
     if (!owner) {
       owner = this.owners[webContentsId] = new Set()
-      // Clear the storage when webContents is reloaded/navigated.
-      webContents.once('render-view-deleted', () => {
-        this.clear(webContentsId)
-      })
+      this.registerDeleteListener(webContents, webContentsId)
     }
     if (!owner.has(id)) {
       owner.add(id)
@@ -39,6 +36,18 @@ class ObjectsRegistry {
     return id
   }
 
+  // Clear the storage when webContents is reloaded/navigated.
+  registerDeleteListener (webContents, webContentsId) {
+    const processId = webContents.getProcessId()
+    const listener = (event, deletedProcessId) => {
+      if (deletedProcessId === processId) {
+        webContents.removeListener('render-view-deleted', listener)
+        this.clear(webContentsId)
+      }
+    }
+    webContents.on('render-view-deleted', listener)
+  }
+
   // Get an object according to its ID.
   get (id) {
     const pointer = this.storage[id]
@@ -90,7 +99,7 @@ class ObjectsRegistry {
     pointer.count -= 1
     if (pointer.count === 0) {
       v8Util.deleteHiddenValue(pointer.object, 'atomId')
-      return delete this.storage[id]
+      delete this.storage[id]
     }
   }
 }

From ec43dd067c6c67d7f2090b176a09e4caebc92f47 Mon Sep 17 00:00:00 2001
From: Kevin Sawicki <kevinsawicki@gmail.com>
Date: Wed, 30 Nov 2016 16:59:59 -0800
Subject: [PATCH 2/3] Co-locate with other private methods

---
 lib/browser/objects-registry.js | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/browser/objects-registry.js b/lib/browser/objects-registry.js
index 048fe856a2..bfb9091e65 100644
--- a/lib/browser/objects-registry.js
+++ b/lib/browser/objects-registry.js
@@ -36,18 +36,6 @@ class ObjectsRegistry {
     return id
   }
 
-  // Clear the storage when webContents is reloaded/navigated.
-  registerDeleteListener (webContents, webContentsId) {
-    const processId = webContents.getProcessId()
-    const listener = (event, deletedProcessId) => {
-      if (deletedProcessId === processId) {
-        webContents.removeListener('render-view-deleted', listener)
-        this.clear(webContentsId)
-      }
-    }
-    webContents.on('render-view-deleted', listener)
-  }
-
   // Get an object according to its ID.
   get (id) {
     const pointer = this.storage[id]
@@ -102,6 +90,18 @@ class ObjectsRegistry {
       delete this.storage[id]
     }
   }
+
+  // Private: Clear the storage when webContents is reloaded/navigated.
+  registerDeleteListener (webContents, webContentsId) {
+    const processId = webContents.getProcessId()
+    const listener = (event, deletedProcessId) => {
+      if (deletedProcessId === processId) {
+        webContents.removeListener('render-view-deleted', listener)
+        this.clear(webContentsId)
+      }
+    }
+    webContents.on('render-view-deleted', listener)
+  }
 }
 
 module.exports = new ObjectsRegistry()

From c1a02d65565772cc17d868506c9da6bdf7600874 Mon Sep 17 00:00:00 2001
From: Kevin Sawicki <kevinsawicki@gmail.com>
Date: Wed, 30 Nov 2016 17:29:50 -0800
Subject: [PATCH 3/3] Add spec for render-view-deleted issue

---
 spec/api-ipc-spec.js                       | 15 ++++++++++
 spec/fixtures/api/render-view-deleted.html | 32 ++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100644 spec/fixtures/api/render-view-deleted.html

diff --git a/spec/api-ipc-spec.js b/spec/api-ipc-spec.js
index 4d36813446..9cb3a4c506 100644
--- a/spec/api-ipc-spec.js
+++ b/spec/api-ipc-spec.js
@@ -517,4 +517,19 @@ describe('ipc module', function () {
     ipcRenderer.removeAllListeners('test-event')
     assert.equal(ipcRenderer.listenerCount('test-event'), 0)
   })
+
+  describe('remote objects registry', function () {
+    it('does not dereference until the render view is deleted (regression)', function (done) {
+      w = new BrowserWindow({
+        show: true
+      })
+
+      ipcMain.once('error-message', (event, message) => {
+        assert(message.startsWith('Cannot call function \'getURL\' on missing remote object'), message)
+        done()
+      })
+
+      w.loadURL('file://' + path.join(fixtures, 'api', 'render-view-deleted.html'))
+    })
+  })
 })
diff --git a/spec/fixtures/api/render-view-deleted.html b/spec/fixtures/api/render-view-deleted.html
new file mode 100644
index 0000000000..bfc281eb42
--- /dev/null
+++ b/spec/fixtures/api/render-view-deleted.html
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title></title>
+    <script>
+      const {ipcRenderer, remote} = require('electron')
+
+      const contents = remote.getCurrentWebContents()
+
+      // This should not trigger a dereference and a remote getURL call should not fail
+      contents.emit('render-view-deleted', {}, 'not-a-process-id')
+      try {
+        contents.getURL()
+      } catch (error) {
+        ipcRenderer.send('error-message', 'Unexpected error on getURL call')
+      }
+
+      // This should trigger a dereference and a remote getURL call should fail
+      contents.emit('render-view-deleted', {}, contents.getProcessId())
+      try {
+        contents.getURL()
+        ipcRenderer.send('error-message', 'No error thrown')
+      } catch (error) {
+        ipcRenderer.send('error-message', error.message)
+      }
+    </script>
+  </head>
+  <body>
+
+  </body>
+</html>