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 bdd4e43..1002aa8 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 03709dc..334e8f5 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 3da9910..b95c2e7 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 6b56bfc..479a85c 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 16e0891..7a2353e 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 c7d1169..03103f1 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 5d06f98..20b8ddc 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 7eaafc2..07b0600 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) -- 2.7.4