[[ Bug 22005 ]] Fix memory leaks when using browser widget

runrevmark · runrevmark · commit 8c232568f1d0 · 2019-04-18T19:20:37.000+01:00
This patch fixes two different types of leak which occur in the
browser widget.

The first is from the wrapper classes used to host the platform
specific callback interfaces for events and javascript handlers.
A pair of these wrapper class instances leak for every instance
of the browser widget on all platforms. The leak is caused by
not releasing the (reference-counted) instances correctly.

The second is from a failure to free the c-string form of the
current javascript handlers list. The last set will leak when
the browser widget instance is destroyed. This affects Mac,
Windows, Linux and iOS. The leak has been fixed by freeing the
c-string in the per-platform destructors.
diff --git a/docs/notes/bugfix-22005.md b/docs/notes/bugfix-22005.md
@@ -0,0 +1 @@
+# Fix memory leaks from using the browser widget
diff --git a/libbrowser/src/libbrowser.cpp b/libbrowser/src/libbrowser.cpp
@@ -50,6 +50,21 @@ void MCBrowserRefCounted::Destroy()
 
 ////////////////////////////////////////////////////////////////////////////////
 
+MCBrowserBase::MCBrowserBase(void)
+    : m_event_handler(nil),
+      m_javascript_handler(nil)
+{
+}
+
+MCBrowserBase::~MCBrowserBase(void)
+{
+    if (m_event_handler)
+        m_event_handler->Release();
+    
+    if (m_javascript_handler)
+        m_javascript_handler->Release();
+}
+
 void MCBrowserBase::SetEventHandler(MCBrowserEventHandler *p_handler)
 {
 	if (p_handler)
@@ -470,6 +485,9 @@ bool MCBrowserSetRequestHandler(MCBrowserRef p_browser, MCBrowserRequestCallback
 		return false;
 	
 	p_browser->SetEventHandler(t_wrapper);
+    
+    t_wrapper->Release();
+    
 	return true;
 }
 
@@ -515,6 +533,9 @@ bool MCBrowserSetJavaScriptHandler(MCBrowserRef p_browser, MCBrowserJavaScriptCa
 		return false;
 	
 	p_browser->SetJavaScriptHandler(t_wrapper);
+    
+    t_wrapper->Release();
+    
 	return true;
 }
 
diff --git a/libbrowser/src/libbrowser_cef.cpp b/libbrowser/src/libbrowser_cef.cpp
@@ -1291,7 +1291,10 @@ void MCCefBrowserBase::Finalize()
 	// IM-2014-07-21: [[ Bug 12296 ]] Notify client of browser being closed
 	if (m_client)
 		m_client->OnOwnerClosed();
-	
+    
+    if (m_javascript_handlers != nil)
+        MCCStringFree(m_javascript_handlers);
+    
 	m_browser = nil;
 	m_client = nil;
 }
diff --git a/libbrowser/src/libbrowser_internal.h b/libbrowser/src/libbrowser_internal.h
@@ -30,13 +30,8 @@ typedef bool (*MCBrowserIterateCallback)(MCBrowser *p_browser, void *p_context);
 class MCBrowserBase : public MCBrowser
 {
 public:
-	MCBrowserBase() : m_event_handler(nil), m_javascript_handler(nil)
-	{
-	}
-	
-	virtual ~MCBrowserBase()
-	{
-	}
+    MCBrowserBase(void);
+    virtual ~MCBrowserBase(void);
 	
 	void SetEventHandler(MCBrowserEventHandler *p_handler);
 	void SetJavaScriptHandler(MCBrowserJavaScriptHandler *p_handler);
diff --git a/libbrowser/src/libbrowser_osx_webview.mm b/libbrowser/src/libbrowser_osx_webview.mm
@@ -425,6 +425,9 @@ inline void MCBrowserRunBlockOnMainFiber(void (^p_block)(void))
 		if (m_ui_delegate != nil)
 			[m_ui_delegate release];
 		
+        if (m_js_handlers != nil)
+            MCCStringFree(m_js_handlers);
+        
 		if (m_js_handler_list != nil)
 			[m_js_handler_list release];
 	});
diff --git a/libbrowser/src/libbrowser_uiwebview.mm b/libbrowser/src/libbrowser_uiwebview.mm
@@ -233,7 +233,10 @@ inline void MCBrowserRunBlockOnMainFiber(void (^p_block)(void))
 		
 		if (m_delegate != nil)
 			[m_delegate release];
-		
+        
+        if (m_js_handlers != nil)
+            MCCStringFree(m_js_handlers);
+        
 		if (m_js_handler_list != nil)
 			[m_js_handler_list release];
 	});

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+# Fix memory leaks from using the browser widget`