From 623e0f3ae4112e40b309a1e8c5f886a0b05e99f1 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 6 Dec 2013 14:44:25 +0800 Subject: [PATCH] Release render view's remote objects when it's deleted. Privously we release them when the window is unloaded, which is not correct since a render view can have multiple windows (or js contexts) and when the unload event is emitted the render view could already have gone. This PR does the cleaning work purely in browser, so here is no need to worry about renderer's life time. --- browser/api/atom_api_window.cc | 4 ++++ browser/api/atom_api_window.h | 1 + browser/api/lib/browser-window.coffee | 5 +++++ browser/atom/rpc-server.coffee | 9 +++++---- browser/native_window.cc | 9 +++++++++ browser/native_window.h | 1 + browser/native_window_observer.h | 3 +++ renderer/api/lib/remote.coffee | 7 ------- 8 files changed, 28 insertions(+), 11 deletions(-) diff --git a/browser/api/atom_api_window.cc b/browser/api/atom_api_window.cc index bdd4e43edf24..1002aa81bee1 100644 --- a/browser/api/atom_api_window.cc +++ b/browser/api/atom_api_window.cc @@ -76,6 +76,10 @@ void Window::OnRendererResponsive() { Emit("responsive"); } +void Window::OnRenderViewDeleted() { + Emit("render-view-deleted"); +} + void Window::OnRendererCrashed() { Emit("crashed"); } diff --git a/browser/api/atom_api_window.h b/browser/api/atom_api_window.h index 03709dc22a10..334e8f554260 100644 --- a/browser/api/atom_api_window.h +++ b/browser/api/atom_api_window.h @@ -43,6 +43,7 @@ class Window : public EventEmitter, virtual void OnWindowBlur() OVERRIDE; virtual void OnRendererUnresponsive() OVERRIDE; virtual void OnRendererResponsive() OVERRIDE; + virtual void OnRenderViewDeleted() OVERRIDE; virtual void OnRendererCrashed() OVERRIDE; private: diff --git a/browser/api/lib/browser-window.coffee b/browser/api/lib/browser-window.coffee index 3da99104b91b..b95c2e7b73e8 100644 --- a/browser/api/lib/browser-window.coffee +++ b/browser/api/lib/browser-window.coffee @@ -11,6 +11,11 @@ BrowserWindow::_init = -> menu = app.getApplicationMenu() @setMenu menu if menu? + # Tell the rpc server that a render view has been deleted and we need to + # release all objects owned by it. + @on 'render-view-deleted', -> + process.emit 'ATOM_BROWSER_RELEASE_RENDER_VIEW', @getProcessId(), @getRoutingId() + BrowserWindow::toggleDevTools = -> if @isDevToolsOpened() then @closeDevTools() else @openDevTools() diff --git a/browser/atom/rpc-server.coffee b/browser/atom/rpc-server.coffee index 6b56bfca2063..479a85c3fca0 100644 --- a/browser/atom/rpc-server.coffee +++ b/browser/atom/rpc-server.coffee @@ -78,6 +78,11 @@ callFunction = (event, processId, routingId, func, caller, args) -> ret = func.apply caller, args event.returnValue = valueToMeta processId, routingId, ret +# Send by BrowserWindow when its render view is deleted. +process.on 'ATOM_BROWSER_RELEASE_RENDER_VIEW', (processId, routingId) -> + console.log 'ATOM_BROWSER_RELEASE_RENDER_VIEW', processId, routingId + objectsRegistry.clear processId, routingId + ipc.on 'ATOM_BROWSER_REQUIRE', (event, processId, routingId, module) -> try event.returnValue = valueToMeta processId, routingId, require(module) @@ -90,10 +95,6 @@ ipc.on 'ATOM_BROWSER_GLOBAL', (event, processId, routingId, name) -> catch e event.returnValue = errorToMeta e -ipc.on 'ATOM_BROWSER_RELEASE_RENDER_VIEW', (event, processId, routingId) -> - objectsRegistry.clear processId, routingId - event.returnValue = null - ipc.on 'ATOM_BROWSER_CURRENT_WINDOW', (event, processId, routingId) -> try BrowserWindow = require 'browser-window' diff --git a/browser/native_window.cc b/browser/native_window.cc index 16e0891bc8d3..7a2353e574de 100644 --- a/browser/native_window.cc +++ b/browser/native_window.cc @@ -268,6 +268,11 @@ void NativeWindow::NotifyWindowClosed() { if (is_closed_) return; + // The OnRenderViewDeleted is not called when the WebContents is destroyed + // directly (e.g. when closing the window), so we make sure it's always + // emitted to users by sending it before window is closed.. + FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRenderViewDeleted()); + is_closed_ = true; FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnWindowClosed()); @@ -393,6 +398,10 @@ bool NativeWindow::OnMessageReceived(const IPC::Message& message) { return handled; } +void NativeWindow::RenderViewDeleted(content::RenderViewHost*) { + FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRenderViewDeleted()); +} + void NativeWindow::RenderViewGone(base::TerminationStatus status) { FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRendererCrashed()); } diff --git a/browser/native_window.h b/browser/native_window.h index c7d1169cdc3d..03103f118259 100644 --- a/browser/native_window.h +++ b/browser/native_window.h @@ -182,6 +182,7 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate, virtual void RendererResponsive(content::WebContents* source) OVERRIDE; // Implementations of content::WebContentsObserver. + virtual void RenderViewDeleted(content::RenderViewHost*) OVERRIDE; virtual void RenderViewGone(base::TerminationStatus status) OVERRIDE; virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; diff --git a/browser/native_window_observer.h b/browser/native_window_observer.h index 5d06f983bd55..20b8ddcc17f0 100644 --- a/browser/native_window_observer.h +++ b/browser/native_window_observer.h @@ -35,6 +35,9 @@ class NativeWindowObserver { // Called when renderer recovers. virtual void OnRendererResponsive() {} + // Called when a render view has been deleted. + virtual void OnRenderViewDeleted() {} + // Called when renderer has crashed. virtual void OnRendererCrashed() {} }; diff --git a/renderer/api/lib/remote.coffee b/renderer/api/lib/remote.coffee index 7eaafc20aa52..07b06001038a 100644 --- a/renderer/api/lib/remote.coffee +++ b/renderer/api/lib/remote.coffee @@ -2,7 +2,6 @@ ipc = require 'ipc' CallbacksRegistry = require 'callbacks-registry' v8Util = process.atomBinding 'v8_util' -currentContextExist = true callbacksRegistry = new CallbacksRegistry # Convert the arguments object into an array of meta data. @@ -82,7 +81,6 @@ metaToValue = (meta) -> # Track delegate object's life time, and tell the browser to clean up # when the object is GCed. v8Util.setDestructor ret, -> - return unless currentContextExist ipc.sendChannel 'ATOM_BROWSER_DEREFERENCE', meta.storeId # Remember object's id. @@ -98,11 +96,6 @@ ipc.on 'ATOM_RENDERER_CALLBACK', (id, args) -> ipc.on 'ATOM_RENDERER_RELEASE_CALLBACK', (id) -> callbacksRegistry.remove id -# Release all resources of current render view when it's going to be unloaded. -window.addEventListener 'unload', (event) -> - currentContextExist = false - ipc.sendChannelSync 'ATOM_BROWSER_RELEASE_RENDER_VIEW' - # Get remote module. # (Just like node's require, the modules are cached permanently, note that this # is safe leak since the object is not expected to get freed in browser)