From 424477fd566e8706cbd413b1e22f07547b3a6620 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Mon, 30 Jul 2018 14:07:17 +1000 Subject: [PATCH 1/8] feat: add will-redirect to allow people to prevent 30X redirects --- atom/browser/api/atom_api_web_contents.cc | 26 ++++++++++++++++++++--- atom/browser/api/atom_api_web_contents.h | 4 ++++ docs/api/web-contents.md | 21 ++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 4fadf30545a69..e65406424955e 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -845,7 +845,8 @@ void WebContents::DidStopLoading() { Emit("did-stop-loading"); } -void WebContents::DidStartNavigation( +bool WebContents::EmitNavigationEvent( + const std::string& event, content::NavigationHandle* navigation_handle) { bool is_main_frame = navigation_handle->IsInMainFrame(); int frame_tree_node_id = navigation_handle->GetFrameTreeNodeId(); @@ -866,8 +867,27 @@ void WebContents::DidStartNavigation( } bool is_same_document = navigation_handle->IsSameDocument(); auto url = navigation_handle->GetURL(); - Emit("did-start-navigation", url, is_same_document, is_main_frame, - frame_process_id, frame_routing_id); + return Emit(event, url, is_same_document, is_main_frame, frame_process_id, + frame_routing_id); +} + +void WebContents::DidStartNavigation( + content::NavigationHandle* navigation_handle) { + EmitNavigationEvent("did-start-navigation", navigation_handle); +} + +void OnStopSoon(WebContents* web_contents) { + if (web_contents && !web_contents->IsDestroyed()) + web_contents->Stop(); +} + +void WebContents::DidRedirectNavigation( + content::NavigationHandle* navigation_handle) { + if (EmitNavigationEvent("will-redirect", navigation_handle)) { + scoped_refptr task_runner( + base::ThreadTaskRunnerHandle::Get()); + task_runner->PostTask(FROM_HERE, base::BindOnce(&OnStopSoon, this)); + } } void WebContents::DidFinishNavigation( diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index 558a1a27adf18..cf773fd6eab77 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -366,6 +366,8 @@ class WebContents : public mate::TrackableObject, void DidStopLoading() override; void DidStartNavigation( content::NavigationHandle* navigation_handle) override; + void DidRedirectNavigation( + content::NavigationHandle* navigation_handle) override; void DidFinishNavigation( content::NavigationHandle* navigation_handle) override; bool OnMessageReceived(const IPC::Message& message) override; @@ -447,6 +449,8 @@ class WebContents : public mate::TrackableObject, void InitZoomController(content::WebContents* web_contents, const mate::Dictionary& options); + bool EmitNavigationEvent(const std::string& event, + content::NavigationHandle* navigation_handle); v8::Global session_; v8::Global devtools_web_contents_; diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index f9bc611317743..67d4315d995f4 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -170,6 +170,7 @@ Calling `event.preventDefault()` will prevent the navigation. Returns: +* `event` Event * `url` String * `isInPlace` Boolean * `isMainFrame` Boolean @@ -179,6 +180,26 @@ Returns: Emitted when any frame (including main) starts navigating. `isInplace` will be `true` for in-page navigations. +#### Event: 'will-redirect' + +Returns: + +* `event` Event +* `url` String +* `isInPlace` Boolean +* `isMainFrame` Boolean +* `frameProcessId` Integer +* `frameRoutingId` Integer + +Emitted when a server side redirect occurs during navigation. For example a 302 +redirect. + +This event will be emitted after `did-start-navigation` and always before the +`did-navigate` event for the same navigation. + +Calling `event.preventDefault()` will prevent the navigation (not just the +redirect). + #### Event: 'did-navigate' Returns: From 48a16dea0ea591ade0640d668409e6a14e351bfa Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Mon, 30 Jul 2018 15:01:49 +1000 Subject: [PATCH 2/8] spec: add tests for the will-redirect event --- spec/api-browser-window-spec.js | 63 +++++++++++++++++++++++++++++++++ spec/static/main.js | 16 +++++++++ 2 files changed, 79 insertions(+) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 2d612ba48b019..2e4ebc8130f89 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -84,6 +84,12 @@ describe('BrowserWindow module', () => { } }) }) + } else if (req.url === '/302') { + res.setHeader('Location', '/200') + res.statusCode = 302 + res.end() + } else if (req.url === '/navigate-302') { + res.end(``) } else { res.end() } @@ -344,6 +350,63 @@ describe('BrowserWindow module', () => { }) }) + describe('will-redirect event', () => { + it('is emitted on redirects', (done) => { + w.webContents.on('will-redirect', (event, url) => { + done() + }) + w.loadURL(`${server.url}/302`) + }) + + it('is emitted after will-navigate on redirects', (done) => { + let navigateCalled = false + w.loadURL(`${server.url}/navigate-302`) + w.webContents.on('will-navigate', () => { + navigateCalled = true + }) + w.webContents.on('will-redirect', (event, url) => { + expect(navigateCalled).to.equal(true, 'should have called will-navigate first') + done() + }) + }) + + it('is emitted before did-stop-loading on redirects', (done) => { + let stopCalled = false + w.webContents.on('did-stop-loading', () => { + stopCalled = true + }) + w.webContents.on('will-redirect', (event, url) => { + expect(stopCalled).to.equal(false, 'should not have called did-stop-loading first') + done() + }) + w.loadURL(`${server.url}/302`) + }) + + it('allows the window to be closed from the event listener', (done) => { + ipcRenderer.send('close-on-will-redirect', w.id) + ipcRenderer.once('closed-on-will-redirect', () => { done() }) + w.loadURL(`${server.url}/302`) + }) + + it('can be prevented', (done) => { + ipcRenderer.send('prevent-will-redirect', w.id) + w.webContents.on('will-navigate', (e, url) => { + expect(url).to.equal(`${server.url}/302`) + }) + w.webContents.on('did-stop-loading', () => { + expect(w.webContents.getURL()).to.equal( + `${server.url}/navigate-302`, + 'url should not have changed after navigation event' + ) + done() + }) + w.webContents.on('will-redirect', (e, url) => { + expect(url).to.equal(`${server.url}/200`) + }) + w.loadURL(`${server.url}/navigate-302`) + }) + }) + describe('BrowserWindow.show()', () => { before(function () { if (isCI) { diff --git a/spec/static/main.js b/spec/static/main.js index 1948eb8e710ca..c1cd975cb2bf4 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -265,6 +265,22 @@ ipcMain.on('close-on-will-navigate', (event, id) => { }) }) +ipcMain.on('close-on-will-redirect', (event, id) => { + const contents = event.sender + const window = BrowserWindow.fromId(id) + window.webContents.once('will-redirect', (event, input) => { + window.close() + contents.send('closed-on-will-redirect') + }) +}) + +ipcMain.on('prevent-will-redirect', (event, id) => { + const window = BrowserWindow.fromId(id) + window.webContents.once('will-redirect', (event) => { + event.preventDefault() + }) +}) + 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 From c8eedd93585088b5f124108fe9b5fac510838856 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Wed, 1 Aug 2018 17:50:47 +1000 Subject: [PATCH 3/8] refactor: implement will-redirect using NavigationThrottle instead of PostTask This avoids a potential race condition and immediately cancels the navigation --- atom/browser/api/atom_api_web_contents.cc | 12 ++----- atom/browser/api/atom_api_web_contents.h | 5 +-- atom/browser/atom_browser_client.cc | 9 +++++ atom/browser/atom_browser_client.h | 3 ++ atom/browser/net/atom_navigation_throttle.cc | 36 ++++++++++++++++++++ atom/browser/net/atom_navigation_throttle.h | 29 ++++++++++++++++ filenames.gni | 2 ++ 7 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 atom/browser/net/atom_navigation_throttle.cc create mode 100644 atom/browser/net/atom_navigation_throttle.h diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index e65406424955e..5c323711047a0 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -19,6 +19,7 @@ #include "atom/browser/child_web_contents_tracker.h" #include "atom/browser/lib/bluetooth_chooser.h" #include "atom/browser/native_window.h" +#include "atom/browser/net/atom_navigation_throttle.h" #include "atom/browser/net/atom_network_delegate.h" #if defined(ENABLE_OSR) #include "atom/browser/osr/osr_output_device.h" @@ -876,18 +877,9 @@ void WebContents::DidStartNavigation( EmitNavigationEvent("did-start-navigation", navigation_handle); } -void OnStopSoon(WebContents* web_contents) { - if (web_contents && !web_contents->IsDestroyed()) - web_contents->Stop(); -} - void WebContents::DidRedirectNavigation( content::NavigationHandle* navigation_handle) { - if (EmitNavigationEvent("will-redirect", navigation_handle)) { - scoped_refptr task_runner( - base::ThreadTaskRunnerHandle::Get()); - task_runner->PostTask(FROM_HERE, base::BindOnce(&OnStopSoon, this)); - } + EmitNavigationEvent("did-redirect-navigation", navigation_handle); } void WebContents::DidFinishNavigation( diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index cf773fd6eab77..294a82e761934 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -266,6 +266,9 @@ class WebContents : public mate::TrackableObject, observers_.RemoveObserver(obs); } + bool EmitNavigationEvent(const std::string& event, + content::NavigationHandle* navigation_handle); + protected: WebContents(v8::Isolate* isolate, content::WebContents* web_contents, @@ -449,8 +452,6 @@ class WebContents : public mate::TrackableObject, void InitZoomController(content::WebContents* web_contents, const mate::Dictionary& options); - bool EmitNavigationEvent(const std::string& event, - content::NavigationHandle* navigation_handle); v8::Global session_; v8::Global devtools_web_contents_; diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 5a795bbd2e2f2..1635a6f0a1ccf 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -21,6 +21,7 @@ #include "atom/browser/atom_speech_recognition_manager_delegate.h" #include "atom/browser/child_web_contents_tracker.h" #include "atom/browser/native_window.h" +#include "atom/browser/net/atom_navigation_throttle.h" #include "atom/browser/session_preferences.h" #include "atom/browser/web_contents_permission_helper.h" #include "atom/browser/web_contents_preferences.h" @@ -613,4 +614,12 @@ bool AtomBrowserClient::HandleExternalProtocol( return true; } +std::vector> +AtomBrowserClient::CreateThrottlesForNavigation( + content::NavigationHandle* handle) { + std::vector> throttles; + throttles.push_back(base::WrapUnique(new AtomNavigationThrottle(handle))); + return throttles; +} + } // namespace atom diff --git a/atom/browser/atom_browser_client.h b/atom/browser/atom_browser_client.h index df955ec99c7ff..4f28bddb89fae 100644 --- a/atom/browser/atom_browser_client.h +++ b/atom/browser/atom_browser_client.h @@ -47,6 +47,9 @@ class AtomBrowserClient : public brightray::BrowserClient, static void SetCustomServiceWorkerSchemes( const std::vector& schemes); + std::vector> + CreateThrottlesForNavigation(content::NavigationHandle* handle) override; + protected: // content::ContentBrowserClient: void RenderProcessWillLaunch( diff --git a/atom/browser/net/atom_navigation_throttle.cc b/atom/browser/net/atom_navigation_throttle.cc new file mode 100644 index 0000000000000..2e93af488de82 --- /dev/null +++ b/atom/browser/net/atom_navigation_throttle.cc @@ -0,0 +1,36 @@ +// Copyright (c) 2015 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/net/atom_navigation_throttle.h" + +#include "atom/browser/api/atom_api_web_contents.h" +#include "content/public/browser/navigation_handle.h" + +namespace atom { + +AtomNavigationThrottle::AtomNavigationThrottle( + content::NavigationHandle* navigation_handle) + : content::NavigationThrottle(navigation_handle) {} + +AtomNavigationThrottle::~AtomNavigationThrottle() {} + +content::NavigationThrottle::ThrottleCheckResult +AtomNavigationThrottle::WillRedirectRequest() { + auto* handle = navigation_handle(); + auto* contents = handle->GetWebContents(); + if (!contents) + return PROCEED; + + auto api_contents = + atom::api::WebContents::CreateFrom(v8::Isolate::GetCurrent(), contents); + if (api_contents.IsEmpty()) + return PROCEED; + + if (api_contents->EmitNavigationEvent("will-redirect", handle)) { + return CANCEL; + } + return PROCEED; +} + +} // namespace atom diff --git a/atom/browser/net/atom_navigation_throttle.h b/atom/browser/net/atom_navigation_throttle.h new file mode 100644 index 0000000000000..f7636b2dd6aea --- /dev/null +++ b/atom/browser/net/atom_navigation_throttle.h @@ -0,0 +1,29 @@ +// Copyright (c) 2018 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_NET_ATOM_NAVIGATION_THROTTLE_H_ +#define ATOM_BROWSER_NET_ATOM_NAVIGATION_THROTTLE_H_ + +#include "content/public/browser/navigation_throttle.h" + +class URLPattern; + +namespace atom { + +class AtomNavigationThrottle : public content::NavigationThrottle { + public: + AtomNavigationThrottle(content::NavigationHandle* handle); + ~AtomNavigationThrottle() override; + + AtomNavigationThrottle::ThrottleCheckResult WillRedirectRequest() override; + + const char* GetNameForLogging() { return "AtomNavigationThrottle"; } + + private: + DISALLOW_COPY_AND_ASSIGN(AtomNavigationThrottle); +}; + +} // namespace atom + +#endif // ATOM_BROWSER_NET_ATOM_NAVIGATION_THROTTLE_H_ diff --git a/filenames.gni b/filenames.gni index 29fc65b928959..1ea2f9cc545bc 100644 --- a/filenames.gni +++ b/filenames.gni @@ -209,6 +209,8 @@ filenames = { "atom/browser/atom_browser_main_parts_posix.cc", "atom/browser/atom_javascript_dialog_manager.cc", "atom/browser/atom_javascript_dialog_manager.h", + "atom/browser/net/atom_navigation_throttle.h", + "atom/browser/net/atom_navigation_throttle.cc", "atom/browser/atom_permission_manager.cc", "atom/browser/atom_permission_manager.h", "atom/browser/atom_quota_permission_context.cc", From 82ea3b96a9df406d9bc0c07b9ec3e894c63b67df Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Wed, 1 Aug 2018 17:53:01 +1000 Subject: [PATCH 4/8] docs: add docs for did-redirect-navigation --- docs/api/web-contents.md | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index 67d4315d995f4..9570ac904167b 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -191,15 +191,32 @@ Returns: * `frameProcessId` Integer * `frameRoutingId` Integer -Emitted when a server side redirect occurs during navigation. For example a 302 +Emitted as a server side redirect occurs during navigation. For example a 302 redirect. This event will be emitted after `did-start-navigation` and always before the -`did-navigate` event for the same navigation. +`did-redirect-navigation` event for the same navigation. Calling `event.preventDefault()` will prevent the navigation (not just the redirect). +#### Event: 'did-redirect-navigation' + +Returns: + +* `event` Event +* `url` String +* `isInPlace` Boolean +* `isMainFrame` Boolean +* `frameProcessId` Integer +* `frameRoutingId` Integer + +Emitted after a server side redirect occurs during navigation. For example a 302 +redirect. + +This event can not be prevented, if you want to prevent redirects you should +checkout out the `will-redirect` event above. + #### Event: 'did-navigate' Returns: From f96dad4023e0cbb8f13aa5446e72899597a7e9ba Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Fri, 3 Aug 2018 11:08:43 +1000 Subject: [PATCH 5/8] refactor: move AtomNavigationThrottle out of net folder --- atom/browser/api/atom_api_web_contents.cc | 2 +- atom/browser/atom_browser_client.cc | 2 +- atom/browser/{net => }/atom_navigation_throttle.cc | 10 +++++++--- atom/browser/{net => }/atom_navigation_throttle.h | 4 +--- 4 files changed, 10 insertions(+), 8 deletions(-) rename atom/browser/{net => }/atom_navigation_throttle.cc (81%) rename atom/browser/{net => }/atom_navigation_throttle.h (88%) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 5c323711047a0..d9cdc980d6156 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -16,10 +16,10 @@ #include "atom/browser/atom_browser_context.h" #include "atom/browser/atom_browser_main_parts.h" #include "atom/browser/atom_javascript_dialog_manager.h" +#include "atom/browser/atom_navigation_throttle.h" #include "atom/browser/child_web_contents_tracker.h" #include "atom/browser/lib/bluetooth_chooser.h" #include "atom/browser/native_window.h" -#include "atom/browser/net/atom_navigation_throttle.h" #include "atom/browser/net/atom_network_delegate.h" #if defined(ENABLE_OSR) #include "atom/browser/osr/osr_output_device.h" diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 1635a6f0a1ccf..f326c8c1da32e 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -16,12 +16,12 @@ #include "atom/browser/api/atom_api_web_contents.h" #include "atom/browser/atom_browser_context.h" #include "atom/browser/atom_browser_main_parts.h" +#include "atom/browser/atom_navigation_throttle.h" #include "atom/browser/atom_quota_permission_context.h" #include "atom/browser/atom_resource_dispatcher_host_delegate.h" #include "atom/browser/atom_speech_recognition_manager_delegate.h" #include "atom/browser/child_web_contents_tracker.h" #include "atom/browser/native_window.h" -#include "atom/browser/net/atom_navigation_throttle.h" #include "atom/browser/session_preferences.h" #include "atom/browser/web_contents_permission_helper.h" #include "atom/browser/web_contents_preferences.h" diff --git a/atom/browser/net/atom_navigation_throttle.cc b/atom/browser/atom_navigation_throttle.cc similarity index 81% rename from atom/browser/net/atom_navigation_throttle.cc rename to atom/browser/atom_navigation_throttle.cc index 2e93af488de82..f88862a525ae5 100644 --- a/atom/browser/net/atom_navigation_throttle.cc +++ b/atom/browser/atom_navigation_throttle.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by the MIT license that can be // found in the LICENSE file. -#include "atom/browser/net/atom_navigation_throttle.h" +#include "atom/browser/atom_navigation_throttle.h" #include "atom/browser/api/atom_api_web_contents.h" #include "content/public/browser/navigation_handle.h" @@ -19,13 +19,17 @@ content::NavigationThrottle::ThrottleCheckResult AtomNavigationThrottle::WillRedirectRequest() { auto* handle = navigation_handle(); auto* contents = handle->GetWebContents(); - if (!contents) + if (!contents) { + DCHECK(false); // This should be unreachable return PROCEED; + } auto api_contents = atom::api::WebContents::CreateFrom(v8::Isolate::GetCurrent(), contents); - if (api_contents.IsEmpty()) + if (api_contents.IsEmpty()) { + DCHECK(false); // This should be unreachable return PROCEED; + } if (api_contents->EmitNavigationEvent("will-redirect", handle)) { return CANCEL; diff --git a/atom/browser/net/atom_navigation_throttle.h b/atom/browser/atom_navigation_throttle.h similarity index 88% rename from atom/browser/net/atom_navigation_throttle.h rename to atom/browser/atom_navigation_throttle.h index f7636b2dd6aea..c243902015e5e 100644 --- a/atom/browser/net/atom_navigation_throttle.h +++ b/atom/browser/atom_navigation_throttle.h @@ -7,8 +7,6 @@ #include "content/public/browser/navigation_throttle.h" -class URLPattern; - namespace atom { class AtomNavigationThrottle : public content::NavigationThrottle { @@ -18,7 +16,7 @@ class AtomNavigationThrottle : public content::NavigationThrottle { AtomNavigationThrottle::ThrottleCheckResult WillRedirectRequest() override; - const char* GetNameForLogging() { return "AtomNavigationThrottle"; } + const char* GetNameForLogging() override { return "AtomNavigationThrottle"; } private: DISALLOW_COPY_AND_ASSIGN(AtomNavigationThrottle); From 4b9e1e4819b87903228d0081f6f7ef0a9fb05ace Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Sat, 4 Aug 2018 07:30:12 +1000 Subject: [PATCH 6/8] refactor: update header guard for atom_navigation_throttle.h --- atom/browser/atom_navigation_throttle.h | 8 ++++---- filenames.gni | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/atom/browser/atom_navigation_throttle.h b/atom/browser/atom_navigation_throttle.h index c243902015e5e..ebb7ab6fa2a7a 100644 --- a/atom/browser/atom_navigation_throttle.h +++ b/atom/browser/atom_navigation_throttle.h @@ -2,8 +2,8 @@ // Use of this source code is governed by the MIT license that can be // found in the LICENSE file. -#ifndef ATOM_BROWSER_NET_ATOM_NAVIGATION_THROTTLE_H_ -#define ATOM_BROWSER_NET_ATOM_NAVIGATION_THROTTLE_H_ +#ifndef ATOM_BROWSER_ATOM_NAVIGATION_THROTTLE_H_ +#define ATOM_BROWSER_ATOM_NAVIGATION_THROTTLE_H_ #include "content/public/browser/navigation_throttle.h" @@ -11,7 +11,7 @@ namespace atom { class AtomNavigationThrottle : public content::NavigationThrottle { public: - AtomNavigationThrottle(content::NavigationHandle* handle); + explicit AtomNavigationThrottle(content::NavigationHandle* handle); ~AtomNavigationThrottle() override; AtomNavigationThrottle::ThrottleCheckResult WillRedirectRequest() override; @@ -24,4 +24,4 @@ class AtomNavigationThrottle : public content::NavigationThrottle { } // namespace atom -#endif // ATOM_BROWSER_NET_ATOM_NAVIGATION_THROTTLE_H_ +#endif // ATOM_BROWSER_ATOM_NAVIGATION_THROTTLE_H_ diff --git a/filenames.gni b/filenames.gni index 1ea2f9cc545bc..1b104ef6ad402 100644 --- a/filenames.gni +++ b/filenames.gni @@ -209,8 +209,8 @@ filenames = { "atom/browser/atom_browser_main_parts_posix.cc", "atom/browser/atom_javascript_dialog_manager.cc", "atom/browser/atom_javascript_dialog_manager.h", - "atom/browser/net/atom_navigation_throttle.h", - "atom/browser/net/atom_navigation_throttle.cc", + "atom/browser/atom_navigation_throttle.h", + "atom/browser/atom_navigation_throttle.cc", "atom/browser/atom_permission_manager.cc", "atom/browser/atom_permission_manager.h", "atom/browser/atom_quota_permission_context.cc", From d01e84e5f85ae8f3ddbf2f0220470309eaa94b6f Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Sun, 16 Sep 2018 00:47:16 +1000 Subject: [PATCH 7/8] refactor: fix chromium style errors in the GN build --- atom/browser/atom_navigation_throttle.cc | 4 ++++ atom/browser/atom_navigation_throttle.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/atom/browser/atom_navigation_throttle.cc b/atom/browser/atom_navigation_throttle.cc index f88862a525ae5..85e2276e4d9ad 100644 --- a/atom/browser/atom_navigation_throttle.cc +++ b/atom/browser/atom_navigation_throttle.cc @@ -15,6 +15,10 @@ AtomNavigationThrottle::AtomNavigationThrottle( AtomNavigationThrottle::~AtomNavigationThrottle() {} +const char* AtomNavigationThrottle::GetNameForLogging() { + return "AtomNavigationThrottle"; +} + content::NavigationThrottle::ThrottleCheckResult AtomNavigationThrottle::WillRedirectRequest() { auto* handle = navigation_handle(); diff --git a/atom/browser/atom_navigation_throttle.h b/atom/browser/atom_navigation_throttle.h index ebb7ab6fa2a7a..779258aa14f4c 100644 --- a/atom/browser/atom_navigation_throttle.h +++ b/atom/browser/atom_navigation_throttle.h @@ -16,7 +16,7 @@ class AtomNavigationThrottle : public content::NavigationThrottle { AtomNavigationThrottle::ThrottleCheckResult WillRedirectRequest() override; - const char* GetNameForLogging() override { return "AtomNavigationThrottle"; } + const char* GetNameForLogging() override; private: DISALLOW_COPY_AND_ASSIGN(AtomNavigationThrottle); From 4b860586d963d04e7e5b5f62794d76903f345d2a Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Sun, 16 Sep 2018 00:51:20 +1000 Subject: [PATCH 8/8] refactor: update throttle impl to NOTREACHED and std::make_unqique --- atom/browser/atom_browser_client.cc | 2 +- atom/browser/atom_navigation_throttle.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index f326c8c1da32e..576da03427eee 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -618,7 +618,7 @@ std::vector> AtomBrowserClient::CreateThrottlesForNavigation( content::NavigationHandle* handle) { std::vector> throttles; - throttles.push_back(base::WrapUnique(new AtomNavigationThrottle(handle))); + throttles.push_back(std::make_unique(handle)); return throttles; } diff --git a/atom/browser/atom_navigation_throttle.cc b/atom/browser/atom_navigation_throttle.cc index 85e2276e4d9ad..bcb0dfe56118b 100644 --- a/atom/browser/atom_navigation_throttle.cc +++ b/atom/browser/atom_navigation_throttle.cc @@ -24,14 +24,14 @@ AtomNavigationThrottle::WillRedirectRequest() { auto* handle = navigation_handle(); auto* contents = handle->GetWebContents(); if (!contents) { - DCHECK(false); // This should be unreachable + NOTREACHED(); return PROCEED; } auto api_contents = atom::api::WebContents::CreateFrom(v8::Isolate::GetCurrent(), contents); if (api_contents.IsEmpty()) { - DCHECK(false); // This should be unreachable + NOTREACHED(); return PROCEED; }