diff --git a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp index 05ec2fbef..a2382b34e 100644 --- a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp +++ b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp @@ -55,6 +55,24 @@ namespace CefSharp return this; }; + ref struct CefBrowserWrapperUpdater + { + CefBrowserWrapper^ _capture; + CefAppUnmanagedWrapper& _wrapper; + CefBrowserWrapperUpdater(CefBrowserWrapper^ capture, CefAppUnmanagedWrapper& wrapper) : + _capture(capture), + _wrapper(wrapper) { + } + + public: + CefBrowserWrapper^ update(int key, CefBrowserWrapper^ oldValue) + { + _wrapper._onBrowserDestroyed->Invoke(oldValue); + delete oldValue; + return _capture; + } + }; + // CefRenderProcessHandler void CefAppUnmanagedWrapper::OnBrowserCreated(CefRefPtr browser, CefRefPtr extraInfo) { @@ -63,7 +81,14 @@ namespace CefSharp //Multiple CefBrowserWrappers created when opening popups auto browserId = browser->GetIdentifier(); - _browserWrappers->TryAdd(browserId, wrapper); + + // In some cases CEF recreates the browser with identical id instead of making a new one + // If this happens it will call OnBrowserCreated a second time before calling OnBrowserDestroyed for the first browser + // We need to check if the browser wrapper with given id already exists and if it does destroy it before adding the new one + // https://github.com/chromiumembedded/cef/blob/ffa86e361d9c210136b539953663526add56191e/include/cef_render_process_handler.h#L67 + auto updater = gcnew CefBrowserWrapperUpdater(wrapper, *this); + auto updateFunc = gcnew Func(updater, &CefBrowserWrapperUpdater::update); + _browserWrappers->AddOrUpdate(browserId, wrapper, updateFunc); static gcroot^> factory = gcnew Func(CefAppUnmanagedWrapper::JavascriptBindingSettingsFactory); @@ -136,11 +161,20 @@ namespace CefSharp void CefAppUnmanagedWrapper::OnBrowserDestroyed(CefRefPtr browser) { - CefBrowserWrapper^ wrapper; - if (_browserWrappers->TryRemove(browser->GetIdentifier(), wrapper)) - { - _onBrowserDestroyed->Invoke(wrapper); - delete wrapper; + int browserId = browser->GetIdentifier(); + CefBrowserWrapper^ currentValue; + if (_browserWrappers->TryGetValue(browserId, currentValue)) { + // Check if another wrapper hasn't already taken our place + if (currentValue->IsSameBrowser(browser)) { + CefBrowserWrapper^ wrapper; + // This is technically a race condition since the entry for browser id might have changed between TryGetValue + // and TryRemove. No clear way to fix this with ConcurrentDictionary + if (_browserWrappers->TryRemove(browserId, wrapper)) + { + _onBrowserDestroyed->Invoke(wrapper); + delete wrapper; + } + } } // Don't remove javascript settings because cef is unreliable in calling OnBrowserCreated/OnBrowserDestroyed consistently: diff --git a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h index a6d90755d..cdb8cb4c1 100644 --- a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h +++ b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h @@ -22,6 +22,7 @@ namespace CefSharp // This class is the native subprocess level CEF object wrapper. private class CefAppUnmanagedWrapper : SubProcessApp, CefRenderProcessHandler { + friend ref struct CefBrowserWrapperUpdater; private: gcroot _handler; gcroot^> _onBrowserCreated; diff --git a/CefSharp.BrowserSubprocess.Core/CefBrowserWrapper.h b/CefSharp.BrowserSubprocess.Core/CefBrowserWrapper.h index 934725eae..b95d71f09 100644 --- a/CefSharp.BrowserSubprocess.Core/CefBrowserWrapper.h +++ b/CefSharp.BrowserSubprocess.Core/CefBrowserWrapper.h @@ -46,6 +46,9 @@ namespace CefSharp { this->!CefBrowserWrapper(); } + bool IsSameBrowser(CefRefPtr other) { + return this->_cefBrowser->IsSame(other); + } property int BrowserId; property bool IsPopup;