Merge pull request #590 from Octopus-ITSM/jsbindings_tooManyChannels

amaitland · amaitland · commit 9c8b984c7237 · 2014-11-12T14:37:51.000+10:00
CefRenderProcess process creates too many channels and does not close them
diff --git a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp
@@ -50,7 +50,7 @@ namespace CefSharp
         {
             auto window = context->GetGlobal();
 
-            auto jsRootWrapper = gcnew JavascriptRootObjectWrapper(wrapper->JavascriptRootObject, wrapper->CreateBrowserProxyDelegate);
+            auto jsRootWrapper = gcnew JavascriptRootObjectWrapper(wrapper->JavascriptRootObject, wrapper->BrowserProcess);
 
             jsRootWrapper->V8Value = window;
             jsRootWrapper->Bind();
diff --git a/CefSharp.BrowserSubprocess.Core/CefBrowserWrapper.h b/CefSharp.BrowserSubprocess.Core/CefBrowserWrapper.h
@@ -43,7 +43,7 @@ namespace CefSharp
         property bool IsPopup;
         property DuplexChannelFactory<IBrowserProcess^>^ ChannelFactory;
         property JavascriptRootObject^ JavascriptRootObject;
-        property Func<IBrowserProcess^>^ CreateBrowserProxyDelegate;
+        property IBrowserProcess^ BrowserProcess;
 
         JavascriptResponse^ EvaluateScriptInContext(CefRefPtr<CefV8Context> context, CefString script)
         {
diff --git a/CefSharp.BrowserSubprocess.Core/JavascriptMethodWrapper.cpp b/CefSharp.BrowserSubprocess.Core/JavascriptMethodWrapper.cpp
@@ -22,8 +22,6 @@ namespace CefSharp
 
     BrowserProcessResponse^ JavascriptMethodWrapper::Execute(array<Object^>^ parameters)
     {
-        auto browserProxy = _createBrowserProxyDelegate();
-
-        return browserProxy->CallMethod(_ownerId, _javascriptMethod->JavascriptName, parameters);
+        return _browserProcess->CallMethod(_ownerId, _javascriptMethod->JavascriptName, parameters);
     }
 }
diff --git a/CefSharp.BrowserSubprocess.Core/JavascriptMethodWrapper.h b/CefSharp.BrowserSubprocess.Core/JavascriptMethodWrapper.h
@@ -19,17 +19,17 @@ namespace CefSharp
         MCefRefPtr<JavascriptMethodHandler> _javascriptMethodHandler;
         JavascriptMethod^ _javascriptMethod;
         int64 _ownerId;
-        Func<IBrowserProcess^>^ _createBrowserProxyDelegate;
+        IBrowserProcess^ _browserProcess;
 
     internal:
         MCefRefPtr<CefV8Value> V8Value;
 
     public:
-        JavascriptMethodWrapper(JavascriptMethod^ javascriptMethod, int64 ownerId, Func<IBrowserProcess^>^ createBrowserProxyDelegate)
+        JavascriptMethodWrapper(JavascriptMethod^ javascriptMethod, int64 ownerId, IBrowserProcess^ browserProcess)
         {
             _javascriptMethod = javascriptMethod;
             _ownerId = ownerId;
-            _createBrowserProxyDelegate = createBrowserProxyDelegate;
+            _browserProcess = browserProcess;
             _javascriptMethodHandler = new JavascriptMethodHandler(gcnew Func<array<Object^>^, BrowserProcessResponse^>(this, &JavascriptMethodWrapper::Execute));
         }
 
diff --git a/CefSharp.BrowserSubprocess.Core/JavascriptObjectWrapper.cpp b/CefSharp.BrowserSubprocess.Core/JavascriptObjectWrapper.cpp
@@ -28,7 +28,7 @@ namespace CefSharp
 
 		for each (JavascriptMethod^ method in Enumerable::OfType<JavascriptMethod^>(_object->Methods))
 		{
-			auto wrappedMethod = gcnew JavascriptMethodWrapper(method, _object->Id, _createBrowserProxyDelegate);
+			auto wrappedMethod = gcnew JavascriptMethodWrapper(method, _object->Id, _browserProcess);
 			wrappedMethod->V8Value = javascriptObject;
 			wrappedMethod->Bind();
 
@@ -37,7 +37,7 @@ namespace CefSharp
 
 		for each (JavascriptProperty^ prop in Enumerable::OfType<JavascriptProperty^>(_object->Properties))
 		{
-			auto wrappedproperty = gcnew JavascriptPropertyWrapper(prop, _object->Id, _createBrowserProxyDelegate);
+			auto wrappedproperty = gcnew JavascriptPropertyWrapper(prop, _object->Id, _browserProcess);
 			wrappedproperty->V8Value = javascriptObject;
 			wrappedproperty->Bind();
 
@@ -47,15 +47,11 @@ namespace CefSharp
 
 	BrowserProcessResponse^ JavascriptObjectWrapper::GetProperty(String^ memberName)
 	{
-		auto browserProxy = _createBrowserProxyDelegate();
-
-		return browserProxy->GetProperty(_object->Id, memberName);
+		return _browserProcess->GetProperty(_object->Id, memberName);
 	};
 
 	BrowserProcessResponse^ JavascriptObjectWrapper::SetProperty(String^ memberName, Object^ value)
 	{
-		auto browserProxy = _createBrowserProxyDelegate();
-
-		return browserProxy->SetProperty(_object->Id, memberName, value);
+		return _browserProcess->SetProperty(_object->Id, memberName, value);
 	};
 }
diff --git a/CefSharp.BrowserSubprocess.Core/JavascriptObjectWrapper.h b/CefSharp.BrowserSubprocess.Core/JavascriptObjectWrapper.h
@@ -24,17 +24,17 @@ namespace CefSharp
         JavascriptObject^ _object;
         List<JavascriptMethodWrapper^>^ _wrappedMethods;
         List<JavascriptPropertyWrapper^>^ _wrappedProperties;
-        Func<IBrowserProcess^>^ _createBrowserProxyDelegate;
+        IBrowserProcess^ _browserProcess;
 
     internal:
         MCefRefPtr<CefV8Value> V8Value;
         MCefRefPtr<JavascriptPropertyHandler> JsPropertyHandler;
 
     public:
-        JavascriptObjectWrapper(JavascriptObject^ object, Func<IBrowserProcess^>^ createBrowserProxyDelegate)
+        JavascriptObjectWrapper(JavascriptObject^ object, IBrowserProcess^ browserProcess)
         {
             _object = object;
-            _createBrowserProxyDelegate = createBrowserProxyDelegate;
+            _browserProcess = browserProcess;
 
             _wrappedMethods = gcnew List<JavascriptMethodWrapper^>();
             _wrappedProperties = gcnew List<JavascriptPropertyWrapper^>();
diff --git a/CefSharp.BrowserSubprocess.Core/JavascriptPropertyWrapper.cpp b/CefSharp.BrowserSubprocess.Core/JavascriptPropertyWrapper.cpp
@@ -19,7 +19,7 @@ namespace CefSharp
 
         if (_javascriptProperty->IsComplexType)
         {
-            auto wrapperObject = gcnew JavascriptObjectWrapper(_javascriptProperty->JsObject, _createBrowserProxyDelegate);
+            auto wrapperObject = gcnew JavascriptObjectWrapper(_javascriptProperty->JsObject, _browserProcess);
             wrapperObject->V8Value = V8Value.get();
             wrapperObject->Bind();
         }
diff --git a/CefSharp.BrowserSubprocess.Core/JavascriptPropertyWrapper.h b/CefSharp.BrowserSubprocess.Core/JavascriptPropertyWrapper.h
@@ -18,17 +18,17 @@ namespace CefSharp
     private:
         JavascriptProperty^ _javascriptProperty;
         int64 _ownerId;
-        Func<IBrowserProcess^>^ _createBrowserProxyDelegate;
+        IBrowserProcess^ _browserProcess;
 
     internal:
         MCefRefPtr<CefV8Value> V8Value;
 
     public:
-        JavascriptPropertyWrapper(JavascriptProperty^ javascriptProperty, int64 ownerId, Func<IBrowserProcess^>^ createBrowserProxyDelegate)
+        JavascriptPropertyWrapper(JavascriptProperty^ javascriptProperty, int64 ownerId, IBrowserProcess^ browserProcess)
         {
             _javascriptProperty = javascriptProperty;
             _ownerId = ownerId;
-            _createBrowserProxyDelegate = createBrowserProxyDelegate;
+            _browserProcess = browserProcess;
         }
 
         ~JavascriptPropertyWrapper()
diff --git a/CefSharp.BrowserSubprocess.Core/JavascriptRootObjectWrapper.h b/CefSharp.BrowserSubprocess.Core/JavascriptRootObjectWrapper.h
@@ -24,30 +24,30 @@ namespace CefSharp
     private:
         JavascriptRootObject^ _rootObject;
         List<JavascriptObjectWrapper^>^ _wrappedObjects;
-        Func<IBrowserProcess^>^ _createBrowserProxyDelegate;
+        IBrowserProcess^ _browserProcess;
 
     internal:
         MCefRefPtr<CefV8Value> V8Value;
 
     public:
-        JavascriptRootObjectWrapper(JavascriptRootObject^ rootObject, Func<IBrowserProcess^>^ createBrowserProxyDelegate)
+        JavascriptRootObjectWrapper(JavascriptRootObject^ rootObject, IBrowserProcess^ browserProcess)
         {
             _rootObject = rootObject;
-            _createBrowserProxyDelegate = createBrowserProxyDelegate;
+            _browserProcess = browserProcess;
             _wrappedObjects = gcnew List<JavascriptObjectWrapper^>();
         }
 
         ~JavascriptRootObjectWrapper()
         {
             V8Value = nullptr;
         }
-        
+
         void Bind()
         {
             auto memberObjects = _rootObject->MemberObjects;
             for each (JavascriptObject^ obj in Enumerable::OfType<JavascriptObject^>(memberObjects))
             {
-                auto wrapperObject = gcnew JavascriptObjectWrapper(obj, _createBrowserProxyDelegate);
+                auto wrapperObject = gcnew JavascriptObjectWrapper(obj, _browserProcess);
                 wrapperObject->V8Value = V8Value.get();
                 wrapperObject->Bind();
 
diff --git a/CefSharp.BrowserSubprocess/CefRenderProcess.cs b/CefSharp.BrowserSubprocess/CefRenderProcess.cs
@@ -58,30 +58,26 @@ public override void OnBrowserCreated(CefBrowserWrapper browser)
 
             channelFactory.Open();
 
-            var proxy = channelFactory.CreateChannel();
+            var browserProcess = channelFactory.CreateChannel();
+            var clientChannel = ((IClientChannel)browserProcess);
 
-            var clientChannel = ((IClientChannel)proxy);
-            
             try
             {
                 clientChannel.Open();
+                browserProcess.Connect();
 
-                proxy.Connect();
-
-                var javascriptObject = proxy.GetRegisteredJavascriptObjects();
+                var javascriptObject = browserProcess.GetRegisteredJavascriptObjects();
 
                 if (javascriptObject.MemberObjects.Count > 0)
                 {
                     browser.JavascriptRootObject = javascriptObject;
-                    browser.CreateBrowserProxyDelegate = channelFactory.CreateChannel;
                 }
 
                 browser.ChannelFactory = channelFactory;
-
+                browser.BrowserProcess = browserProcess;
             }
             catch(Exception)
             {
-
             }
         }
 
@@ -96,8 +92,15 @@ public override void OnBrowserDestroyed(CefBrowserWrapper browser)
                 channelFactory.Close();
             }
 
+            var clientChannel = ((IClientChannel)browser.BrowserProcess);
+
+            if (clientChannel.State == CommunicationState.Opened)
+            {
+                clientChannel.Close();
+            }
+
             browser.ChannelFactory = null;
-            browser.CreateBrowserProxyDelegate = null;
+            browser.BrowserProcess = null;
             browser.JavascriptRootObject = null;
         }
 
diff --git a/CefSharp.Example/Resources/BindingTest.html b/CefSharp.Example/Resources/BindingTest.html
@@ -33,6 +33,20 @@
             </script>
         </p>
 
+        <p>
+            Stress Test
+            <br />
+            <script type="text/javascript">
+                var stressTestCallCount = 1000;
+                for (var i = 1; i <= stressTestCallCount; i++) {
+                    bound.repeat("hi ", 5);
+                }
+
+                document.write("Stress Test done with : " + stressTestCallCount + " call to bound.repeat(\"hi \", 5)");
+            </script>
+        </p>
+
+
         Methods on bound object 'bound':<br />
         <ul>
             <script type="text/javascript">

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ namespace CefSharp`
`50`	`50`	`{`
`51`	`51`	`auto window = context->GetGlobal();`
`52`	`52`
`53`		`- auto jsRootWrapper = gcnew JavascriptRootObjectWrapper(wrapper->JavascriptRootObject, wrapper->CreateBrowserProxyDelegate);`
	`53`	`+ auto jsRootWrapper = gcnew JavascriptRootObjectWrapper(wrapper->JavascriptRootObject, wrapper->BrowserProcess);`
`54`	`54`
`55`	`55`	`jsRootWrapper->V8Value = window;`
`56`	`56`	`jsRootWrapper->Bind();`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ namespace CefSharp`
`43`	`43`	`property bool IsPopup;`
`44`	`44`	`property DuplexChannelFactory<IBrowserProcess^>^ ChannelFactory;`
`45`	`45`	`property JavascriptRootObject^ JavascriptRootObject;`
`46`		`- property Func<IBrowserProcess^>^ CreateBrowserProxyDelegate;`
	`46`	`+ property IBrowserProcess^ BrowserProcess;`
`47`	`47`
`48`	`48`	`JavascriptResponse^ EvaluateScriptInContext(CefRefPtr<CefV8Context> context, CefString script)`
`49`	`49`	`{`
Original file line number	Diff line number	Diff line change
`@@ -22,8 +22,6 @@ namespace CefSharp`
`22`	`22`
`23`	`23`	`BrowserProcessResponse^ JavascriptMethodWrapper::Execute(array<Object^>^ parameters)`
`24`	`24`	`{`
`25`		`- auto browserProxy = _createBrowserProxyDelegate();`
`26`		`-`
`27`		`- return browserProxy->CallMethod(_ownerId, _javascriptMethod->JavascriptName, parameters);`
	`25`	`+ return _browserProcess->CallMethod(_ownerId, _javascriptMethod->JavascriptName, parameters);`
`28`	`26`	`}`
`29`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ namespace CefSharp`
`28`	`28`
`29`	`29`	`for each (JavascriptMethod^ method in Enumerable::OfType<JavascriptMethod^>(_object->Methods))`
`30`	`30`	`{`
`31`		`- auto wrappedMethod = gcnew JavascriptMethodWrapper(method, _object->Id, _createBrowserProxyDelegate);`
	`31`	`+ auto wrappedMethod = gcnew JavascriptMethodWrapper(method, _object->Id, _browserProcess);`
`32`	`32`	`wrappedMethod->V8Value = javascriptObject;`
`33`	`33`	`wrappedMethod->Bind();`
`34`	`34`
`@@ -37,7 +37,7 @@ namespace CefSharp`
`37`	`37`
`38`	`38`	`for each (JavascriptProperty^ prop in Enumerable::OfType<JavascriptProperty^>(_object->Properties))`
`39`	`39`	`{`
`40`		`- auto wrappedproperty = gcnew JavascriptPropertyWrapper(prop, _object->Id, _createBrowserProxyDelegate);`
	`40`	`+ auto wrappedproperty = gcnew JavascriptPropertyWrapper(prop, _object->Id, _browserProcess);`
`41`	`41`	`wrappedproperty->V8Value = javascriptObject;`
`42`	`42`	`wrappedproperty->Bind();`
`43`	`43`
`@@ -47,15 +47,11 @@ namespace CefSharp`
`47`	`47`
`48`	`48`	`BrowserProcessResponse^ JavascriptObjectWrapper::GetProperty(String^ memberName)`
`49`	`49`	`{`
`50`		`- auto browserProxy = _createBrowserProxyDelegate();`
`51`		`-`
`52`		`- return browserProxy->GetProperty(_object->Id, memberName);`
	`50`	`+ return _browserProcess->GetProperty(_object->Id, memberName);`
`53`	`51`	`};`
`54`	`52`
`55`	`53`	`BrowserProcessResponse^ JavascriptObjectWrapper::SetProperty(String^ memberName, Object^ value)`
`56`	`54`	`{`
`57`		`- auto browserProxy = _createBrowserProxyDelegate();`
`58`		`-`
`59`		`- return browserProxy->SetProperty(_object->Id, memberName, value);`
	`55`	`+ return _browserProcess->SetProperty(_object->Id, memberName, value);`
`60`	`56`	`};`
`61`	`57`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ namespace CefSharp`
`19`	`19`
`20`	`20`	`if (_javascriptProperty->IsComplexType)`
`21`	`21`	`{`
`22`		`- auto wrapperObject = gcnew JavascriptObjectWrapper(_javascriptProperty->JsObject, _createBrowserProxyDelegate);`
	`22`	`+ auto wrapperObject = gcnew JavascriptObjectWrapper(_javascriptProperty->JsObject, _browserProcess);`
`23`	`23`	`wrapperObject->V8Value = V8Value.get();`
`24`	`24`	`wrapperObject->Bind();`
`25`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,30 +24,30 @@ namespace CefSharp`
`24`	`24`	`private:`
`25`	`25`	`JavascriptRootObject^ _rootObject;`
`26`	`26`	`List<JavascriptObjectWrapper^>^ _wrappedObjects;`
`27`		`- Func<IBrowserProcess^>^ _createBrowserProxyDelegate;`
	`27`	`+ IBrowserProcess^ _browserProcess;`
`28`	`28`
`29`	`29`	`internal:`
`30`	`30`	`MCefRefPtr<CefV8Value> V8Value;`
`31`	`31`
`32`	`32`	`public:`
`33`		`- JavascriptRootObjectWrapper(JavascriptRootObject^ rootObject, Func<IBrowserProcess^>^ createBrowserProxyDelegate)`
	`33`	`+ JavascriptRootObjectWrapper(JavascriptRootObject^ rootObject, IBrowserProcess^ browserProcess)`
`34`	`34`	`{`
`35`	`35`	`_rootObject = rootObject;`
`36`		`- _createBrowserProxyDelegate = createBrowserProxyDelegate;`
	`36`	`+ _browserProcess = browserProcess;`
`37`	`37`	`_wrappedObjects = gcnew List<JavascriptObjectWrapper^>();`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`~JavascriptRootObjectWrapper()`
`41`	`41`	`{`
`42`	`42`	`V8Value = nullptr;`
`43`	`43`	`}`
`44`		`-`
	`44`	`+`
`45`	`45`	`void Bind()`
`46`	`46`	`{`
`47`	`47`	`auto memberObjects = _rootObject->MemberObjects;`
`48`	`48`	`for each (JavascriptObject^ obj in Enumerable::OfType<JavascriptObject^>(memberObjects))`
`49`	`49`	`{`
`50`		`- auto wrapperObject = gcnew JavascriptObjectWrapper(obj, _createBrowserProxyDelegate);`
	`50`	`+ auto wrapperObject = gcnew JavascriptObjectWrapper(obj, _browserProcess);`
`51`	`51`	`wrapperObject->V8Value = V8Value.get();`
`52`	`52`	`wrapperObject->Bind();`
`53`	`53`
Original file line number	Diff line number	Diff line change
`@@ -58,30 +58,26 @@ public override void OnBrowserCreated(CefBrowserWrapper browser)`
`58`	`58`
`59`	`59`	`channelFactory.Open();`
`60`	`60`
`61`		`- var proxy = channelFactory.CreateChannel();`
	`61`	`+ var browserProcess = channelFactory.CreateChannel();`
	`62`	`+ var clientChannel = ((IClientChannel)browserProcess);`
`62`	`63`
`63`		`- var clientChannel = ((IClientChannel)proxy);`
`64`		`-`
`65`	`64`	`try`
`66`	`65`	`{`
`67`	`66`	`clientChannel.Open();`
	`67`	`+ browserProcess.Connect();`
`68`	`68`
`69`		`- proxy.Connect();`
`70`		`-`
`71`		`- var javascriptObject = proxy.GetRegisteredJavascriptObjects();`
	`69`	`+ var javascriptObject = browserProcess.GetRegisteredJavascriptObjects();`
`72`	`70`
`73`	`71`	`if (javascriptObject.MemberObjects.Count > 0)`
`74`	`72`	`{`
`75`	`73`	`browser.JavascriptRootObject = javascriptObject;`
`76`		`- browser.CreateBrowserProxyDelegate = channelFactory.CreateChannel;`
`77`	`74`	`}`
`78`	`75`
`79`	`76`	`browser.ChannelFactory = channelFactory;`
`80`		`-`
	`77`	`+ browser.BrowserProcess = browserProcess;`
`81`	`78`	`}`
`82`	`79`	`catch(Exception)`
`83`	`80`	`{`
`84`		`-`
`85`	`81`	`}`
`86`	`82`	`}`
`87`	`83`
`@@ -96,8 +92,15 @@ public override void OnBrowserDestroyed(CefBrowserWrapper browser)`
`96`	`92`	`channelFactory.Close();`
`97`	`93`	`}`
`98`	`94`
	`95`	`+ var clientChannel = ((IClientChannel)browser.BrowserProcess);`
	`96`	`+`
	`97`	`+ if (clientChannel.State == CommunicationState.Opened)`
	`98`	`+ {`
	`99`	`+ clientChannel.Close();`
	`100`	`+ }`
	`101`	`+`
`99`	`102`	`browser.ChannelFactory = null;`
`100`		`- browser.CreateBrowserProxyDelegate = null;`
	`103`	`+ browser.BrowserProcess = null;`
`101`	`104`	`browser.JavascriptRootObject = null;`
`102`	`105`	`}`
`103`	`106`