Deprecate did-get-response-details and did-get-redirect-request (#12615)

* Deprecate webContents events did-get-response-details and did-get-redirect-request.

* Update guest view files

* Update webview tag docs and update specs

* Update deprecate.event function

* Update comment

* Update more

* Update documentation for other deprecated event

atom/browser/api/atom_api_web_contents.cc

@@ -872,7 +872,7 @@ void WebContents::DidGetResourceResponseStart(
       (details.resource_type == content::RESOURCE_TYPE_MAIN_FRAME ||
        details.resource_type == content::RESOURCE_TYPE_SUB_FRAME))
     return;
-  Emit("did-get-response-details", details.socket_address.IsEmpty(),
+  Emit("-did-get-response-details", details.socket_address.IsEmpty(),
        details.url, details.original_url, details.http_response_code,
        details.method, details.referrer, details.headers.get(),
        ResourceTypeToString(details.resource_type));
@@ -880,7 +880,7 @@ void WebContents::DidGetResourceResponseStart(
 
 void WebContents::DidGetRedirectForResourceRequest(
     const content::ResourceRedirectDetails& details) {
-  Emit("did-get-redirect-request", details.url, details.new_url,
+  Emit("-did-get-redirect-request", details.url, details.new_url,
        (details.resource_type == content::RESOURCE_TYPE_MAIN_FRAME),
        details.http_response_code, details.method, details.referrer,
        details.headers.get());

docs/api/web-contents.md

@@ -89,7 +89,7 @@ Corresponds to the points in time when the spinner of the tab started spinning.
 
 Corresponds to the points in time when the spinner of the tab stopped spinning.
 
-#### Event: 'did-get-response-details'
+#### Event: 'did-get-response-details' *(Deprecated)*
 
 Returns:
 
@@ -106,7 +106,8 @@ Returns:
 Emitted when details regarding a requested resource are available.
 `status` indicates the socket connection to download the resource.
 
-#### Event: 'did-get-redirect-request'
+**Deprecated**: This event has been deprecated. Use the [`webRequest`](web-request.md) module which provides similar navigation details on a subscription basis.
+#### Event: 'did-get-redirect-request' *(Deprecated)*
 
 Returns:
 
@@ -120,7 +121,7 @@ Returns:
 * `headers` Object
 
 Emitted when a redirect is received while requesting a resource.
-
+**Deprecated**: This event has been deprecated. Use the [`webRequest`](web-request.md) module which provides similar navigation details on a subscription basis.
 #### Event: 'dom-ready'
 
 Returns:

docs/api/webview-tag.md

@@ -663,7 +663,7 @@ Corresponds to the points in time when the spinner of the tab starts spinning.
 
 Corresponds to the points in time when the spinner of the tab stops spinning.
 
-### Event: 'did-get-response-details'
+### Event: 'did-get-response-details' *(Deprecated)*
 
 Returns:
 
@@ -679,7 +679,8 @@ Returns:
 Fired when details regarding a requested resource is available.
 `status` indicates socket connection to download the resource.
 
-### Event: 'did-get-redirect-request'
+**Deprecated**: This event has been deprecated. Use the [`webRequest`](web-request.md) module which provides similar navigation details on a subscription basis.
+### Event: 'did-get-redirect-request' *(Deprecated)*
 
 Returns:
 
@@ -688,7 +689,7 @@ Returns:
 * `isMainFrame` Boolean
 
 Fired when a redirect was received while requesting a resource.
-
+**Deprecated**: This event has been deprecated. Use the [`webRequest`](web-request.md) module which provides similar navigation details on a subscription basis.
 ### Event: 'dom-ready'
 
 Fired when document in the given frame is loaded.

lib/browser/api/web-contents.js

@@ -307,6 +307,9 @@ WebContents.prototype._init = function () {
     })
   })
 
+  deprecate.event(this, 'did-get-response-details', '-did-get-response-details')
+  deprecate.event(this, 'did-get-redirect-request', '-did-get-redirect-request')
+
   // The devtools requests the webContents to reload.
   this.on('devtools-reload-page', function () {
     this.reload()

lib/browser/guest-view-manager.js

@@ -14,8 +14,8 @@ const supportedWebViewEvents = [
   'did-frame-finish-load',
   'did-start-loading',
   'did-stop-loading',
-  'did-get-response-details',
+  '-did-get-response-details',
-  'did-get-redirect-request',
+  '-did-get-redirect-request',
   'dom-ready',
   'console-message',
   'context-menu',

lib/common/api/deprecate.js

@@ -46,6 +46,27 @@ deprecate.log = (message) => {
   }
 }
 
+// Deprecate an event.
+deprecate.event = (emitter, oldName, newName) => {
+  let warned = false
+  return emitter.on(newName, function (...args) {
+    // There are no listeners for this event
+    if (this.listenerCount(oldName) === 0) { return }
+    // noDeprecation set or if user has already been warned
+    if (warned || process.noDeprecation) { return }
+    warned = true
+    const isInternalEvent = newName.startsWith('-')
+    if (isInternalEvent) {
+      // The event cannot be use anymore. Log that.
+      deprecate.log(`'${oldName}' event has been deprecated.`)
+    } else {
+      // The event has a new name now. Warn with that.
+      deprecate.warn(`'${oldName}' event`, `'${newName}' event`)
+    }
+    this.emit(oldName, ...args)
+  })
+}
+
 deprecate.setHandler = (handler) => {
   deprecationHandler = handler
 }
@@ -81,22 +102,5 @@ deprecate.getHandler = () => deprecationHandler
 //   })
 // }
 //
-// // Deprecate an event.
-// deprecate.event = (emitter, oldName, newName, fn) => {
-//   let warned = false
-//   return emitter.on(newName, function (...args) {
-//     if (this.listenerCount(oldName) > 0) {
-//       if (!(warned || process.noDeprecation)) {
-//         warned = true
-//         deprecate.warn(`'${oldName}' event`, `'${newName}' event`)
-//       }
-//       if (fn != null) {
-//         fn.apply(this, arguments)
-//       } else {
-//         this.emit.apply(this, [oldName].concat(args))
-//       }
-//     }
-//   })
-// }
 
 module.exports = deprecate

lib/renderer/web-view/guest-view-internal.js

@@ -12,8 +12,8 @@ const WEB_VIEW_EVENTS = {
   'did-frame-finish-load': ['isMainFrame'],
   'did-start-loading': [],
   'did-stop-loading': [],
-  'did-get-response-details': ['status', 'newURL', 'originalURL', 'httpResponseCode', 'requestMethod', 'referrer', 'headers', 'resourceType'],
+  '-did-get-response-details': ['status', 'newURL', 'originalURL', 'httpResponseCode', 'requestMethod', 'referrer', 'headers', 'resourceType'],
-  'did-get-redirect-request': ['oldURL', 'newURL', 'isMainFrame'],
+  '-did-get-redirect-request': ['oldURL', 'newURL', 'isMainFrame'],
   'dom-ready': [],
   'console-message': ['level', 'message', 'line', 'sourceId'],
   'context-menu': ['params'],

spec/api-browser-window-spec.js

@@ -152,8 +152,8 @@ describe('BrowserWindow module', () => {
     it('should not crash when invoked synchronously inside navigation observer', (done) => {
       const events = [
         { name: 'did-start-loading', url: `${server.url}/200` },
-        { name: 'did-get-redirect-request', url: `${server.url}/301` },
+        { name: '-did-get-redirect-request', url: `${server.url}/301` },
-        { name: 'did-get-response-details', url: `${server.url}/200` },
+        { name: '-did-get-response-details', url: `${server.url}/200` },
         { name: 'dom-ready', url: `${server.url}/200` },
         { name: 'page-title-updated', url: `${server.url}/title` },
         { name: 'did-stop-loading', url: `${server.url}/200` },
@@ -218,14 +218,15 @@ describe('BrowserWindow module', () => {
       w.on('ready-to-show', () => { done() })
       w.loadURL('about:blank')
     })
-    it('should emit did-get-response-details event', (done) => {
+    // TODO(nitsakh): Deprecated
+    it('should emit did-get-response-details(deprecated) event', (done) => {
       // expected {fileName: resourceType} pairs
       const expectedResources = {
         'did-get-response-details.html': 'mainFrame',
         'logo.png': 'image'
       }
       let responses = 0
-      w.webContents.on('did-get-response-details', (event, status, newUrl, oldUrl, responseCode, method, referrer, headers, resourceType) => {
+      w.webContents.on('-did-get-response-details', (event, status, newUrl, oldUrl, responseCode, method, referrer, headers, resourceType) => {
         responses += 1
         const fileName = newUrl.slice(newUrl.lastIndexOf('/') + 1)
         const expectedType = expectedResources[fileName]

spec/api-web-contents-spec.js

@@ -650,8 +650,8 @@ describe('webContents module', () => {
     it('should not crash when invoked synchronously inside navigation observer', (done) => {
       const events = [
         { name: 'did-start-loading', url: `${server.url}/200` },
-        { name: 'did-get-redirect-request', url: `${server.url}/301` },
+        { name: '-did-get-redirect-request', url: `${server.url}/301` },
-        { name: 'did-get-response-details', url: `${server.url}/200` },
+        { name: '-did-get-response-details', url: `${server.url}/200` },
         { name: 'dom-ready', url: `${server.url}/200` },
         { name: 'did-stop-loading', url: `${server.url}/200` },
         { name: 'did-finish-load', url: `${server.url}/200` },

spec/webview-spec.js

@@ -1088,7 +1088,7 @@ describe('<webview> tag', function () {
     })
   })
 
-  describe('did-get-response-details event', () => {
+  describe('did-get-response-details event (deprecated)', () => {
     it('emits for the page and its resources', (done) => {
       // expected {fileName: resourceType} pairs
       const expectedResources = {
@@ -1096,7 +1096,7 @@ describe('<webview> tag', function () {
         'logo.png': 'image'
       }
       let responses = 0
-      webview.addEventListener('did-get-response-details', (event) => {
+      webview.addEventListener('-did-get-response-details', (event) => {
         responses += 1
         const fileName = event.newURL.slice(event.newURL.lastIndexOf('/') + 1)
         const expectedType = expectedResources[fileName]