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