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.

diff --git a/browser/api/atom_api_window.cc b/browser/api/atom_api_window.cc
index bdd4e43edf24c3562cc7d0b4e0c638d66ba81c54..1002aa81bee1e5be6b632a8db6b09fee23170cca 100644 (file)
--- 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 03709dc22a10d1cb59b6df34158d1961fecb0739..334e8f5542606a96073b39ef858413591b6dbbbd 100644 (file)
--- 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 3da99104b91bc7acacdecf0573d6448a6bec1ca2..b95c2e7b73e81617f8ce5b7e8e46b0b1bd198843 100644 (file)
--- 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 6b56bfca2063ceb73910f81e6e945eab75025b97..479a85c3fca0cd3b8a94532981101490719958de 100644 (file)
--- 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 16e0891bc8d34986a36a2f566807cbca7b41b3a3..7a2353e574de7fded00c2cc310137cfd3ae7d119 100644 (file)
--- 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 c7d1169cdc3d973fc3dfd8eba3525d1ff68c579e..03103f11825975e9277c9bd8b38f9537ce4b6764 100644 (file)
--- 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 5d06f983bd55948961e363d9eba7f4661918bbf8..20b8ddcc17f0b5a2da5364057131c67014a78a13 100644 (file)
--- 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 7eaafc20aa52e2dcc62a5da939fb1261c130282a..07b06001038a5b97b16246fa052574188e6d699a 100644 (file)
--- 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)