From 2579071b989e75f65f622a05f1619d3f2173d1a9 Mon Sep 17 00:00:00 2001 From: Nitish Sakhawalkar Date: Mon, 23 Apr 2018 12:46:12 -0700 Subject: [PATCH] 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 | 4 +-- docs/api/web-contents.md | 7 ++-- docs/api/webview-tag.md | 7 ++-- lib/browser/api/web-contents.js | 3 ++ lib/browser/guest-view-manager.js | 4 +-- lib/common/api/deprecate.js | 38 +++++++++++--------- lib/renderer/web-view/guest-view-internal.js | 4 +-- spec/api-browser-window-spec.js | 9 ++--- spec/api-web-contents-spec.js | 4 +-- spec/webview-spec.js | 4 +-- 10 files changed, 47 insertions(+), 37 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 2eaba18abb8d..f89cc86fdd6c 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/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()); diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index 5623a723fbd4..b49d274b23b8 100644 --- a/docs/api/web-contents.md +++ b/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: diff --git a/docs/api/webview-tag.md b/docs/api/webview-tag.md index 381205068dd5..b623db083ee3 100644 --- a/docs/api/webview-tag.md +++ b/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. diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index e4d96fafff7c..96e0e16442c1 100644 --- a/lib/browser/api/web-contents.js +++ b/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() diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js index dd8e0ff1a923..2a8e6c4e4e93 100644 --- a/lib/browser/guest-view-manager.js +++ b/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-redirect-request', + '-did-get-response-details', + '-did-get-redirect-request', 'dom-ready', 'console-message', 'context-menu', diff --git a/lib/common/api/deprecate.js b/lib/common/api/deprecate.js index fb156c65c1b0..36f18a90741a 100644 --- a/lib/common/api/deprecate.js +++ b/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 diff --git a/lib/renderer/web-view/guest-view-internal.js b/lib/renderer/web-view/guest-view-internal.js index 110b0e453afd..043f1bf913c0 100644 --- a/lib/renderer/web-view/guest-view-internal.js +++ b/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-redirect-request': ['oldURL', 'newURL', 'isMainFrame'], + '-did-get-response-details': ['status', 'newURL', 'originalURL', 'httpResponseCode', 'requestMethod', 'referrer', 'headers', 'resourceType'], + '-did-get-redirect-request': ['oldURL', 'newURL', 'isMainFrame'], 'dom-ready': [], 'console-message': ['level', 'message', 'line', 'sourceId'], 'context-menu': ['params'], diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 4a3853fc0bb7..dd1ee21eeb3e 100644 --- a/spec/api-browser-window-spec.js +++ b/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-response-details', url: `${server.url}/200` }, + { name: '-did-get-redirect-request', url: `${server.url}/301` }, + { 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] diff --git a/spec/api-web-contents-spec.js b/spec/api-web-contents-spec.js index dd31343edbe7..847462e2f48e 100644 --- a/spec/api-web-contents-spec.js +++ b/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-response-details', url: `${server.url}/200` }, + { name: '-did-get-redirect-request', url: `${server.url}/301` }, + { 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` }, diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 8511e7bb2200..3567ef56478a 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -1088,7 +1088,7 @@ describe(' 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(' 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]