Diff
Modified: trunk/Tools/ChangeLog (249682 => 249683)
--- trunk/Tools/ChangeLog 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/ChangeLog 2019-09-10 01:31:06 UTC (rev 249683)
@@ -1,3 +1,65 @@
+2019-09-09 Fujii Hironori <[email protected]>
+
+ [Win][MiniBrowser] WebKitLegacyBrowserWindow is leaked by circular references
+ https://bugs.webkit.org/show_bug.cgi?id=201600
+
+ Reviewed by Brent Fulgham.
+
+ There were some circular references between
+ WebKitLegacyBrowserWindow and its delegation classes. For
+ example, WebKitLegacyBrowserWindow has a reference of
+ WebDownloadDelegate, and WebDownloadDelegate shares the ref
+ counter with WebKitLegacyBrowserWindow.
+
+ WebNotificationObserver was leaked because it wasn't unregistered
+ from the default notification center by using
+ IWebNotificationCenter::removeObserver.
+
+ If a new legacy window was created by mouse right click a link,
+ WebView was released twice because
+ PrintWebUIDelegate::createWebViewWithRequest didn't AddRef the
+ WebView.
+
+ This change does:
+ 1. Make delegation classes have own ref-counter to avoid circular references
+ 2. Do removeObserver notification observers
+ 3. AddRef WebView in PrintWebUIDelegate::createWebViewWithRequest
+
+ * MiniBrowser/win/AccessibilityDelegate.cpp:
+ (AccessibilityDelegate::AddRef):
+ (AccessibilityDelegate::Release):
+ * MiniBrowser/win/AccessibilityDelegate.h: Added m_refCount.
+ * MiniBrowser/win/MiniBrowserWebHost.cpp:
+ (MiniBrowserWebHost::QueryInterface):
+ (MiniBrowserWebHost::AddRef):
+ (MiniBrowserWebHost::Release):
+ * MiniBrowser/win/MiniBrowserWebHost.h: Added m_refCount.
+ * MiniBrowser/win/PrintWebUIDelegate.cpp:
+ (PrintWebUIDelegate::createWebViewWithRequest): Do AddRef for the returned IWebView.
+ (PrintWebUIDelegate::AddRef):
+ (PrintWebUIDelegate::Release):
+ * MiniBrowser/win/PrintWebUIDelegate.h: Added m_refCount.
+ * MiniBrowser/win/ResourceLoadDelegate.cpp:
+ (ResourceLoadDelegate::AddRef):
+ (ResourceLoadDelegate::Release):
+ * MiniBrowser/win/ResourceLoadDelegate.h: Added m_refCount.
+ * MiniBrowser/win/WebDownloadDelegate.cpp:
+ (WebDownloadDelegate::AddRef):
+ (WebDownloadDelegate::Release):
+ * MiniBrowser/win/WebDownloadDelegate.h: Added m_refCount.
+ * MiniBrowser/win/WebKitLegacyBrowserWindow.cpp:
+ (WebKitLegacyBrowserWindow::~WebKitLegacyBrowserWindow): Do removeObserver notification observers.
+ (WebKitLegacyBrowserWindow::init):
+ (WebKitLegacyBrowserWindow::setUIDelegate):
+ (WebKitLegacyBrowserWindow::setAccessibilityDelegate):
+ (WebKitLegacyBrowserWindow::setResourceLoadDelegate):
+ (WebKitLegacyBrowserWindow::setDownloadDelegate):
+ (WebKitLegacyBrowserWindow::AddRef): Deleted.
+ (WebKitLegacyBrowserWindow::Release): Deleted.
+ (WebKitLegacyBrowserWindow::setFrameLoadDelegate): Deleted.
+ (WebKitLegacyBrowserWindow::setFrameLoadDelegatePrivate): Deleted.
+ * MiniBrowser/win/WebKitLegacyBrowserWindow.h:
+
2019-09-09 Chris Dumez <[email protected]>
Stop using testRunner.setPrivateBrowsingEnabled_DEPRECATED() in http/tests/adClickAttribution/conversion-disabled-in-ephemeral-session.html
Modified: trunk/Tools/MiniBrowser/win/AccessibilityDelegate.cpp (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/AccessibilityDelegate.cpp 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/AccessibilityDelegate.cpp 2019-09-10 01:31:06 UTC (rev 249683)
@@ -53,12 +53,15 @@
ULONG AccessibilityDelegate::AddRef()
{
- return m_client.AddRef();
+ return ++m_refCount;
}
ULONG AccessibilityDelegate::Release()
{
- return m_client.Release();
+ ULONG newRef = --m_refCount;
+ if (!newRef)
+ delete this;
+ return newRef;
}
HRESULT AccessibilityDelegate::fireFrameLoadStartedEvents()
Modified: trunk/Tools/MiniBrowser/win/AccessibilityDelegate.h (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/AccessibilityDelegate.h 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/AccessibilityDelegate.h 2019-09-10 01:31:06 UTC (rev 249683)
@@ -41,6 +41,7 @@
virtual HRESULT STDMETHODCALLTYPE fireFrameLoadStartedEvents();
virtual HRESULT STDMETHODCALLTYPE fireFrameLoadFinishedEvents();
private:
+ ULONG m_refCount { 0 };
WebKitLegacyBrowserWindow& m_client;
};
Modified: trunk/Tools/MiniBrowser/win/MiniBrowserWebHost.cpp (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/MiniBrowserWebHost.cpp 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/MiniBrowserWebHost.cpp 2019-09-10 01:31:06 UTC (rev 249683)
@@ -82,6 +82,10 @@
*ppvObject = static_cast<IWebFrameLoadDelegate*>(this);
else if (IsEqualGUID(riid, IID_IWebFrameLoadDelegate))
*ppvObject = static_cast<IWebFrameLoadDelegate*>(this);
+ else if (IsEqualGUID(riid, IID_IWebFrameLoadDelegatePrivate))
+ *ppvObject = static_cast<IWebFrameLoadDelegatePrivate*>(this);
+ else if (IsEqualGUID(riid, IID_IWebNotificationObserver))
+ *ppvObject = static_cast<IWebNotificationObserver*>(this);
else
return E_NOINTERFACE;
@@ -91,12 +95,15 @@
ULONG MiniBrowserWebHost::AddRef()
{
- return m_client->AddRef();
+ return ++m_refCount;
}
ULONG MiniBrowserWebHost::Release()
{
- return m_client->Release();
+ ULONG newRef = --m_refCount;
+ if (!newRef)
+ delete this;
+ return newRef;
}
HRESULT MiniBrowserWebHost::didFinishLoadForFrame(_In_opt_ IWebView* webView, _In_opt_ IWebFrame* frame)
Modified: trunk/Tools/MiniBrowser/win/MiniBrowserWebHost.h (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/MiniBrowserWebHost.h 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/MiniBrowserWebHost.h 2019-09-10 01:31:06 UTC (rev 249683)
@@ -68,5 +68,6 @@
virtual HRESULT STDMETHODCALLTYPE onNotify(_In_opt_ IWebNotification*);
private:
+ ULONG m_refCount { 0 };
WebKitLegacyBrowserWindow* m_client { nullptr };
};
Modified: trunk/Tools/MiniBrowser/win/PrintWebUIDelegate.cpp (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/PrintWebUIDelegate.cpp 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/PrintWebUIDelegate.cpp 2019-09-10 01:31:06 UTC (rev 249683)
@@ -72,7 +72,7 @@
ShowWindow(newWindow.hwnd(), SW_SHOW);
auto& newBrowserWindow = *static_cast<WebKitLegacyBrowserWindow*>(newWindow.browserWindow());
- *newWebView = newBrowserWindow.webView();
+ *newWebView = newBrowserWindow.webView().Detach();
IWebFramePtr frame;
HRESULT hr;
hr = (*newWebView)->mainFrame(&frame.GetInterfacePtr());
@@ -151,12 +151,15 @@
ULONG PrintWebUIDelegate::AddRef()
{
- return m_client.AddRef();
+ return ++m_refCount;
}
ULONG PrintWebUIDelegate::Release()
{
- return m_client.Release();
+ ULONG newRef = --m_refCount;
+ if (!newRef)
+ delete this;
+ return newRef;
}
HRESULT PrintWebUIDelegate::webViewPrintingMarginRect(_In_opt_ IWebView* view, _Out_ RECT* rect)
Modified: trunk/Tools/MiniBrowser/win/PrintWebUIDelegate.h (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/PrintWebUIDelegate.h 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/PrintWebUIDelegate.h 2019-09-10 01:31:06 UTC (rev 249683)
@@ -106,6 +106,7 @@
virtual HRESULT STDMETHODCALLTYPE paintCustomScrollCorner(_In_opt_ IWebView*, _In_ HDC, RECT) { return E_NOTIMPL; }
private:
+ ULONG m_refCount { 0 };
WebKitLegacyBrowserWindow& m_client;
HWND m_modalDialogParent { nullptr };
};
Modified: trunk/Tools/MiniBrowser/win/ResourceLoadDelegate.cpp (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/ResourceLoadDelegate.cpp 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/ResourceLoadDelegate.cpp 2019-09-10 01:31:06 UTC (rev 249683)
@@ -55,12 +55,15 @@
ULONG ResourceLoadDelegate::AddRef()
{
- return m_client->AddRef();
+ return ++m_refCount;
}
ULONG ResourceLoadDelegate::Release()
{
- return m_client->Release();
+ ULONG newRef = --m_refCount;
+ if (!newRef)
+ delete this;
+ return newRef;
}
HRESULT ResourceLoadDelegate::identifierForInitialRequest(_In_opt_ IWebView*, _In_opt_ IWebURLRequest*, _In_opt_ IWebDataSource*, unsigned long identifier)
Modified: trunk/Tools/MiniBrowser/win/ResourceLoadDelegate.h (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/ResourceLoadDelegate.h 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/ResourceLoadDelegate.h 2019-09-10 01:31:06 UTC (rev 249683)
@@ -52,6 +52,7 @@
virtual HRESULT STDMETHODCALLTYPE plugInFailedWithError(_In_opt_ IWebView*, _In_opt_ IWebError*, _In_opt_ IWebDataSource*);
private:
+ ULONG m_refCount { 0 };
WebKitLegacyBrowserWindow* m_client;
};
Modified: trunk/Tools/MiniBrowser/win/WebDownloadDelegate.cpp (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/WebDownloadDelegate.cpp 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/WebDownloadDelegate.cpp 2019-09-10 01:31:06 UTC (rev 249683)
@@ -59,12 +59,15 @@
ULONG WebDownloadDelegate::AddRef()
{
- return m_client.AddRef();
+ return ++m_refCount;
}
ULONG WebDownloadDelegate::Release()
{
- return m_client.Release();
+ ULONG newRef = --m_refCount;
+ if (!newRef)
+ delete this;
+ return newRef;
}
HRESULT WebDownloadDelegate::decideDestinationWithSuggestedFilename(_In_opt_ IWebDownload* download, _In_ BSTR filename)
Modified: trunk/Tools/MiniBrowser/win/WebDownloadDelegate.h (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/WebDownloadDelegate.h 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/WebDownloadDelegate.h 2019-09-10 01:31:06 UTC (rev 249683)
@@ -54,6 +54,7 @@
virtual HRESULT STDMETHODCALLTYPE didFinish(_In_opt_ IWebDownload*);
private:
+ ULONG m_refCount { 0 };
WebKitLegacyBrowserWindow& m_client;
};
Modified: trunk/Tools/MiniBrowser/win/WebKitLegacyBrowserWindow.cpp (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/WebKitLegacyBrowserWindow.cpp 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/WebKitLegacyBrowserWindow.cpp 2019-09-10 01:31:06 UTC (rev 249683)
@@ -69,19 +69,12 @@
{
}
-ULONG WebKitLegacyBrowserWindow::AddRef()
+WebKitLegacyBrowserWindow::~WebKitLegacyBrowserWindow()
{
- ref();
- return refCount();
+ m_defaultNotificationCenter->removeObserver(m_notificationObserver, _bstr_t(WebViewProgressEstimateChangedNotification), m_webView);
+ m_defaultNotificationCenter->removeObserver(m_notificationObserver, _bstr_t(WebViewProgressFinishedNotification), m_webView);
}
-ULONG WebKitLegacyBrowserWindow::Release()
-{
- auto count = refCount();
- deref();
- return --count;
-}
-
HRESULT WebKitLegacyBrowserWindow::init()
{
HRESULT hr = WebKitCreateInstance(CLSID_WebView, 0, IID_IWebView, reinterpret_cast<void**>(&m_webView.GetInterfacePtr()));
@@ -109,8 +102,7 @@
if (FAILED(hr))
return hr;
- IWebNotificationCenterPtr defaultNotificationCenter;
- hr = notificationCenter->defaultCenter(&defaultNotificationCenter.GetInterfacePtr());
+ hr = notificationCenter->defaultCenter(&m_defaultNotificationCenter.GetInterfacePtr());
if (FAILED(hr))
return hr;
@@ -125,19 +117,20 @@
auto webHost = new MiniBrowserWebHost(this);
- hr = setFrameLoadDelegate(webHost);
+ hr = m_webView->setFrameLoadDelegate(webHost);
if (FAILED(hr))
return hr;
- hr = setFrameLoadDelegatePrivate(webHost);
+ hr = m_webViewPrivate->setFrameLoadDelegatePrivate(webHost);
if (FAILED(hr))
return hr;
- hr = defaultNotificationCenter->addObserver(webHost, _bstr_t(WebViewProgressEstimateChangedNotification), nullptr);
+ m_notificationObserver = webHost;
+ hr = m_defaultNotificationCenter->addObserver(m_notificationObserver, _bstr_t(WebViewProgressEstimateChangedNotification), m_webView);
if (FAILED(hr))
return hr;
- hr = defaultNotificationCenter->addObserver(webHost, _bstr_t(WebViewProgressFinishedNotification), nullptr);
+ hr = m_defaultNotificationCenter->addObserver(m_notificationObserver, _bstr_t(WebViewProgressFinishedNotification), m_webView);
if (FAILED(hr))
return hr;
@@ -153,9 +146,7 @@
if (FAILED(hr))
return hr;
- IWebDownloadDelegatePtr downloadDelegate;
- downloadDelegate.Attach(new WebDownloadDelegate(*this));
- hr = setDownloadDelegate(downloadDelegate);
+ hr = setDownloadDelegate(new WebDownloadDelegate(*this));
if (FAILED(hr))
return hr;
@@ -247,38 +238,23 @@
#endif
}
-HRESULT WebKitLegacyBrowserWindow::setFrameLoadDelegate(IWebFrameLoadDelegate* frameLoadDelegate)
-{
- m_frameLoadDelegate = frameLoadDelegate;
- return m_webView->setFrameLoadDelegate(frameLoadDelegate);
-}
-
-HRESULT WebKitLegacyBrowserWindow::setFrameLoadDelegatePrivate(IWebFrameLoadDelegatePrivate* frameLoadDelegatePrivate)
-{
- return m_webViewPrivate->setFrameLoadDelegatePrivate(frameLoadDelegatePrivate);
-}
-
HRESULT WebKitLegacyBrowserWindow::setUIDelegate(IWebUIDelegate* uiDelegate)
{
- m_uiDelegate = uiDelegate;
return m_webView->setUIDelegate(uiDelegate);
}
HRESULT WebKitLegacyBrowserWindow::setAccessibilityDelegate(IAccessibilityDelegate* accessibilityDelegate)
{
- m_accessibilityDelegate = accessibilityDelegate;
return m_webView->setAccessibilityDelegate(accessibilityDelegate);
}
HRESULT WebKitLegacyBrowserWindow::setResourceLoadDelegate(IWebResourceLoadDelegate* resourceLoadDelegate)
{
- m_resourceLoadDelegate = resourceLoadDelegate;
return m_webView->setResourceLoadDelegate(resourceLoadDelegate);
}
HRESULT WebKitLegacyBrowserWindow::setDownloadDelegate(IWebDownloadDelegatePtr downloadDelegate)
{
- m_downloadDelegate = downloadDelegate;
return m_webView->setDownloadDelegate(downloadDelegate);
}
Modified: trunk/Tools/MiniBrowser/win/WebKitLegacyBrowserWindow.h (249682 => 249683)
--- trunk/Tools/MiniBrowser/win/WebKitLegacyBrowserWindow.h 2019-09-10 01:28:57 UTC (rev 249682)
+++ trunk/Tools/MiniBrowser/win/WebKitLegacyBrowserWindow.h 2019-09-10 01:31:06 UTC (rev 249683)
@@ -64,9 +64,6 @@
friend class WebDownloadDelegate;
friend class ResourceLoadDelegate;
- ULONG AddRef();
- ULONG Release();
-
HRESULT init();
HRESULT prepareViews(HWND mainWnd, const RECT& clientRect);
@@ -81,8 +78,6 @@
bool seedInitialDefaultPreferences();
bool setToDefaultPreferences();
- HRESULT setFrameLoadDelegate(IWebFrameLoadDelegate*);
- HRESULT setFrameLoadDelegatePrivate(IWebFrameLoadDelegatePrivate*);
HRESULT setUIDelegate(IWebUIDelegate*);
HRESULT setAccessibilityDelegate(IAccessibilityDelegate*);
HRESULT setResourceLoadDelegate(IWebResourceLoadDelegate*);
@@ -116,6 +111,7 @@
void setPreference(UINT menuID, bool enable);
WebKitLegacyBrowserWindow(BrowserWindowClient&, HWND mainWnd, bool useLayeredWebView);
+ ~WebKitLegacyBrowserWindow();
void subclassForLayeredWindow();
bool setCacheFolder();
@@ -129,13 +125,9 @@
IWebInspectorPtr m_inspector;
IWebPreferencesPtr m_standardPreferences;
IWebPreferencesPrivatePtr m_prefsPrivate;
+ IWebNotificationCenterPtr m_defaultNotificationCenter;
+ IWebNotificationObserverPtr m_notificationObserver;
- IWebFrameLoadDelegatePtr m_frameLoadDelegate;
- IWebUIDelegatePtr m_uiDelegate;
- IAccessibilityDelegatePtr m_accessibilityDelegate;
- IWebResourceLoadDelegatePtr m_resourceLoadDelegate;
- IWebDownloadDelegatePtr m_downloadDelegate;
-
IWebCoreStatisticsPtr m_statistics;
IWebCachePtr m_webCache;
