chore: add deprecation warning on setting app.allowRendererProcessReuse to false (#22337)

* chore: add deprecation warning on setting app.allowRendererProcessReuse to false

* fix: no deprecation warnings in process reuse spec

* chore: add test for new deprecate removeProperty behavior

lib/browser/api/app.ts

@@ -117,6 +117,10 @@ deprecate.fnToProperty(App.prototype, 'accessibilitySupportEnabled', '_isAccessi
 deprecate.fnToProperty(App.prototype, 'badgeCount', '_getBadgeCount', '_setBadgeCount')
 deprecate.fnToProperty(App.prototype, 'name', '_getName', '_setName')
 
+// Deprecate allowRendererProcessReuse but only if they set it to false, no need to log if
+// they are setting it to true
+deprecate.removeProperty(app, 'allowRendererProcessReuse', [false])
+
 // Wrappers for native classes.
 const { DownloadItem } = process.electronBinding('download_item')
 Object.setPrototypeOf(DownloadItem.prototype, EventEmitter.prototype)

lib/common/api/deprecate.ts

@@ -94,24 +94,31 @@ const deprecate: ElectronInternal.DeprecationUtil = {
   },
 
   // remove a property with no replacement
-  removeProperty: (o, removedName) => {
+  removeProperty: (o, removedName, onlyForValues) => {
     // if the property's already been removed, warn about it
-    if (!(removedName in o)) {
+    const info = Object.getOwnPropertyDescriptor((o as any).__proto__, removedName) // eslint-disable-line
+    if (!info) {
       deprecate.log(`Unable to remove property '${removedName}' from an object that lacks it.`)
+      return o
+    }
+    if (!info.get || !info.set) {
+      deprecate.log(`Unable to remove property '${removedName}' from an object does not have a getter / setter`)
+      return o
     }
 
     // wrap the deprecated property in an accessor to warn
     const warn = warnOnce(removedName)
-    let val = o[removedName]
     return Object.defineProperty(o, removedName, {
       configurable: true,
       get: () => {
         warn()
-        return val
+        return info.get!.call(o)
       },
       set: newVal => {
-        warn()
-        val = newVal
+        if (!onlyForValues || onlyForValues.includes(newVal)) {
+          warn()
+        }
+        return info.set!.call(o, newVal)
       }
     })
   },

spec-main/api-deprecate-spec.ts

@@ -83,6 +83,30 @@ describe('deprecate', () => {
     expect(msg).to.include(prop)
   })
 
+  it('deprecates a property of an but retains the existing accessors and setters', () => {
+    let msg
+    deprecate.setHandler(m => { msg = m })
+
+    const prop = 'itMustGo'
+    let i = 1
+    const o = {
+      get itMustGo () {
+        return i
+      },
+      set itMustGo (thing) {
+        i = thing + 1
+      }
+    }
+
+    deprecate.removeProperty(o, prop)
+
+    expect(o[prop]).to.equal(1)
+    expect(msg).to.be.a('string')
+    expect(msg).to.include(prop)
+    o[prop] = 2
+    expect(o[prop]).to.equal(3)
+  })
+
   it('warns exactly once when a function is deprecated with no replacement', () => {
     let msg
     deprecate.setHandler(m => { msg = m })

spec/fixtures/api/site-instance-overrides/main.js

@@ -1,6 +1,8 @@
 const { app, BrowserWindow, ipcMain } = require('electron')
 const path = require('path')
 
+process.noDeprecation = true
+
 process.on('uncaughtException', (e) => {
   console.error(e)
   process.exit(1)

typings/internal-electron.d.ts

@@ -87,7 +87,7 @@ declare namespace ElectronInternal {
     renameFunction(fn: Function, newName: string | Function): Function;
     event(emitter: NodeJS.EventEmitter, oldName: string, newName: string): void;
     fnToProperty(module: any, prop: string, getter: string, setter?: string): void;
-    removeProperty<T, K extends (keyof T & string)>(object: T, propertyName: K): T;
+    removeProperty<T, K extends (keyof T & string)>(object: T, propertyName: K, onlyForValues?: any[]): T;
     renameProperty<T, K extends (keyof T & string)>(object: T, oldName: string, newName: K): T;
     moveAPI(fn: Function, oldUsage: string, newUsage: string): Function;
   }