Browse Source

Merge pull request #8340 from electron/options-cycle

Handle cycles when merging browser window options
Kevin Sawicki 8 years ago
parent
commit
52390120ae

+ 16 - 10
lib/browser/guest-window-manager.js

@@ -7,19 +7,25 @@ const hasProp = {}.hasOwnProperty
 const frameToGuest = {}
 
 // Copy attribute of |parent| to |child| if it is not defined in |child|.
-const mergeOptions = function (child, parent) {
-  let key, value
-  for (key in parent) {
+const mergeOptions = function (child, parent, visited) {
+  // Check for circular reference.
+  if (visited == null) visited = new Set()
+  if (visited.has(parent)) return
+
+  visited.add(parent)
+  for (const key in parent) {
     if (!hasProp.call(parent, key)) continue
-    value = parent[key]
-    if (!(key in child)) {
-      if (typeof value === 'object') {
-        child[key] = mergeOptions({}, value)
-      } else {
-        child[key] = value
-      }
+    if (key in child) continue
+
+    const value = parent[key]
+    if (typeof value === 'object') {
+      child[key] = mergeOptions({}, value, visited)
+    } else {
+      child[key] = value
     }
   }
+  visited.delete(parent)
+
   return child
 }
 

+ 32 - 4
spec/chromium-spec.js

@@ -3,7 +3,7 @@ const http = require('http')
 const path = require('path')
 const ws = require('ws')
 const url = require('url')
-const remote = require('electron').remote
+const {ipcRenderer, remote} = require('electron')
 const {closeWindow} = require('./window-helpers')
 
 const {BrowserWindow, ipcMain, protocol, session, webContents} = remote
@@ -46,7 +46,7 @@ describe('chromium feature', function () {
     var w = null
 
     afterEach(function () {
-      w != null ? w.destroy() : void 0
+      return closeWindow(w).then(function () { w = null })
     })
 
     it('is set correctly when window is not shown', function (done) {
@@ -157,7 +157,7 @@ describe('chromium feature', function () {
     var w = null
 
     afterEach(function () {
-      w != null ? w.destroy() : void 0
+      return closeWindow(w).then(function () { w = null })
     })
 
     it('should register for file scheme', function (done) {
@@ -187,6 +187,12 @@ describe('chromium feature', function () {
       return
     }
 
+    let w = null
+
+    afterEach(() => {
+      return closeWindow(w).then(function () { w = null })
+    })
+
     it('returns a BrowserWindowProxy object', function () {
       var b = window.open('about:blank', '', 'show=no')
       assert.equal(b.closed, false)
@@ -260,6 +266,28 @@ describe('chromium feature', function () {
       b = window.open('file://' + fixtures + '/pages/window-open-size.html', '', 'show=no,width=' + size.width + ',height=' + size.height)
     })
 
+    it('handles cycles when merging the parent options into the child options', (done) => {
+      w = BrowserWindow.fromId(ipcRenderer.sendSync('create-window-with-options-cycle'))
+      w.loadURL('file://' + fixtures + '/pages/window-open.html')
+      w.webContents.once('new-window', (event, url, frameName, disposition, options) => {
+        assert.equal(options.show, false)
+        assert.deepEqual(options.foo, {
+          bar: null,
+          baz: {
+            hello: {
+              world: true
+            }
+          },
+          baz2: {
+            hello: {
+              world: true
+            }
+          }
+        })
+        done()
+      })
+    })
+
     it('defines a window.location getter', function (done) {
       var b, targetURL
       if (process.platform === 'win32') {
@@ -312,7 +340,7 @@ describe('chromium feature', function () {
     let w = null
 
     afterEach(function () {
-      if (w) w.destroy()
+      return closeWindow(w).then(function () { w = null })
     })
 
     it('is null for main window', function (done) {

+ 1 - 1
spec/fixtures/pages/window-open.html

@@ -1,7 +1,7 @@
 <html>
 <body>
 <script type="text/javascript" charset="utf-8">
-  window.open('http://host', 'host' , 'this-is-not-a-standard-feature');
+  window.open('http://host', 'host', 'this-is-not-a-standard-feature');
 </script>
 </body>
 </html>

+ 15 - 0
spec/static/main.js

@@ -230,3 +230,18 @@ ipcMain.on('close-on-will-navigate', (event, id) => {
     contents.send('closed-on-will-navigate')
   })
 })
+
+ipcMain.on('create-window-with-options-cycle', (event) => {
+  // This can't be done over remote since cycles are already
+  // nulled out at the IPC layer
+  const foo = {}
+  foo.bar = foo
+  foo.baz = {
+    hello: {
+      world: true
+    }
+  }
+  foo.baz2 = foo.baz
+  const window = new BrowserWindow({show: false, foo: foo})
+  event.returnValue = window.id
+})