Title: [136631] trunk/Source/WebKit/chromium
- Revision
- 136631
- Author
- [email protected]
- Date
- 2012-12-04 20:55:38 -0800 (Tue, 04 Dec 2012)
Log Message
[Chromium] Detach the frame and destroy the page immediately upon request to closeHelperPlugin().
https://bugs.webkit.org/show_bug.cgi?id=103947
Patch by David Dorwin <[email protected]> on 2012-12-04
Reviewed by Kent Tamura.
In the case that the host page is being destroyed, some of the objects
the page depends on, specifically RenderViewImpl, may be destroyed by
the time close() is called asynchronously.
The frameDetached() calling code was originally copied from
WebViewImpl::close(), but WebViewImpl does not have an asynchronous
closing mechanism like the WebWidgets.
This is the fix for http://crbug.com/160650
* src/WebHelperPluginImpl.cpp:
(WebKit::WebHelperPluginImpl::init):
(WebKit::WebHelperPluginImpl::closeHelperPlugin):
(WebKit::WebHelperPluginImpl::destoryPage):
(WebKit):
(WebKit::WebHelperPluginImpl::close):.
(WebKit::WebHelperPlugin::create):
* src/WebHelperPluginImpl.h:
(WebHelperPluginImpl):
* src/WebPagePopupImpl.cpp: Made similar changes since it follows the same model. (I think the way popups are closed has prevented the race condition from occurring for popups.)
(WebKit::WebPagePopupImpl::destoryPage):
(WebKit):
(WebKit::WebPagePopupImpl::close):
(WebKit::WebPagePopupImpl::closePopup):
* src/WebPagePopupImpl.h:
(WebPagePopupImpl):
Modified Paths
Diff
Modified: trunk/Source/WebKit/chromium/ChangeLog (136630 => 136631)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-12-05 04:49:51 UTC (rev 136630)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-12-05 04:55:38 UTC (rev 136631)
@@ -1,5 +1,38 @@
2012-12-04 David Dorwin <[email protected]>
+ [Chromium] Detach the frame and destroy the page immediately upon request to closeHelperPlugin().
+ https://bugs.webkit.org/show_bug.cgi?id=103947
+
+ Reviewed by Kent Tamura.
+
+ In the case that the host page is being destroyed, some of the objects
+ the page depends on, specifically RenderViewImpl, may be destroyed by
+ the time close() is called asynchronously.
+ The frameDetached() calling code was originally copied from
+ WebViewImpl::close(), but WebViewImpl does not have an asynchronous
+ closing mechanism like the WebWidgets.
+
+ This is the fix for http://crbug.com/160650
+
+ * src/WebHelperPluginImpl.cpp:
+ (WebKit::WebHelperPluginImpl::init):
+ (WebKit::WebHelperPluginImpl::closeHelperPlugin):
+ (WebKit::WebHelperPluginImpl::destoryPage):
+ (WebKit):
+ (WebKit::WebHelperPluginImpl::close):.
+ (WebKit::WebHelperPlugin::create):
+ * src/WebHelperPluginImpl.h:
+ (WebHelperPluginImpl):
+ * src/WebPagePopupImpl.cpp: Made similar changes since it follows the same model. (I think the way popups are closed has prevented the race condition from occurring for popups.)
+ (WebKit::WebPagePopupImpl::destoryPage):
+ (WebKit):
+ (WebKit::WebPagePopupImpl::close):
+ (WebKit::WebPagePopupImpl::closePopup):
+ * src/WebPagePopupImpl.h:
+ (WebPagePopupImpl):
+
+2012-12-04 David Dorwin <[email protected]>
+
[Chromium] Removed obsolete local variable from WebViewImpl.cpp.
https://bugs.webkit.org/show_bug.cgi?id=104050
Modified: trunk/Source/WebKit/chromium/src/WebHelperPluginImpl.cpp (136630 => 136631)
--- trunk/Source/WebKit/chromium/src/WebHelperPluginImpl.cpp 2012-12-05 04:49:51 UTC (rev 136630)
+++ trunk/Source/WebKit/chromium/src/WebHelperPluginImpl.cpp 2012-12-05 04:55:38 UTC (rev 136631)
@@ -87,6 +87,9 @@
private:
virtual void closeWindowSoon() OVERRIDE
{
+ // This should never be called since the only way to close the
+ // invisible page is via closeHelperPlugin().
+ ASSERT_NOT_REACHED();
m_widget->closeHelperPlugin();
}
@@ -120,7 +123,6 @@
if (!initPage(webView, pluginType))
return false;
m_widgetClient->show(WebNavigationPolicy());
-
setFocus(true);
return true;
@@ -133,6 +135,12 @@
m_page->mainFrame()->loader()->stopAllLoaders();
m_page->mainFrame()->loader()->stopLoading(UnloadEventPolicyNone);
}
+
+ // We must destroy the page now in case the host page is being destroyed, in
+ // which case some of the objects the page depends on may have been
+ // destroyed by the time this->close() is called asynchronously.
+ destoryPage();
+
// m_widgetClient might be 0 because this widget might be already closed.
if (m_widgetClient) {
// closeWidgetSoon() will call this->close() later.
@@ -201,6 +209,17 @@
return true;
}
+void WebHelperPluginImpl::destoryPage()
+{
+ if (!m_page)
+ return;
+
+ if (m_page->mainFrame())
+ m_page->mainFrame()->loader()->frameDetached();
+
+ m_page.clear();
+}
+
void WebHelperPluginImpl::setCompositorSurfaceReady()
{
}
@@ -225,17 +244,7 @@
void WebHelperPluginImpl::close()
{
- RefPtr<WebFrameImpl> mainFrameImpl;
-
- if (m_page) {
- // Initiate shutdown. This will cause a lot of notifications to be sent.
- if (m_page->mainFrame()) {
- mainFrameImpl = WebFrameImpl::fromFrame(m_page->mainFrame());
- m_page->mainFrame()->loader()->frameDetached();
- }
- m_page.clear();
- }
-
+ ASSERT(!m_page); // Should only be called via closePopup().
m_widgetClient = 0;
deref();
}
@@ -248,10 +257,10 @@
CRASH();
// A WebHelperPluginImpl instance usually has two references.
// - One owned by the instance itself. It represents the visible widget.
- // - One owned by a WebViewImpl. It's released when the WebViewImpl ask the
- // WebHelperPluginImpl to close.
+ // - One owned by the hosting element. It's released when the hosting
+ // element asks the WebHelperPluginImpl to close.
// We need them because the closing operation is asynchronous and the widget
- // can be closed while the WebViewImpl is unaware of it.
+ // can be closed while the hosting element is unaware of it.
return adoptRef(new WebHelperPluginImpl(client)).leakRef();
}
Modified: trunk/Source/WebKit/chromium/src/WebHelperPluginImpl.h (136630 => 136631)
--- trunk/Source/WebKit/chromium/src/WebHelperPluginImpl.h 2012-12-05 04:49:51 UTC (rev 136630)
+++ trunk/Source/WebKit/chromium/src/WebHelperPluginImpl.h 2012-12-05 04:55:38 UTC (rev 136631)
@@ -64,6 +64,7 @@
private:
explicit WebHelperPluginImpl(WebWidgetClient*);
bool initPage(WebKit::WebViewImpl*, const String& pluginType);
+ void destoryPage();
// WebWidget methods:
virtual void setCompositorSurfaceReady() OVERRIDE;
Modified: trunk/Source/WebKit/chromium/src/WebPagePopupImpl.cpp (136630 => 136631)
--- trunk/Source/WebKit/chromium/src/WebPagePopupImpl.cpp 2012-12-05 04:49:51 UTC (rev 136630)
+++ trunk/Source/WebKit/chromium/src/WebPagePopupImpl.cpp 2012-12-05 04:55:38 UTC (rev 136631)
@@ -225,6 +225,17 @@
return true;
}
+void WebPagePopupImpl::destoryPage()
+{
+ if (!m_page)
+ return;
+
+ if (m_page->mainFrame())
+ m_page->mainFrame()->loader()->frameDetached();
+
+ m_page.clear();
+}
+
WebSize WebPagePopupImpl::size()
{
return m_popupClient->contentSize();
@@ -313,9 +324,7 @@
void WebPagePopupImpl::close()
{
m_closing = true;
- if (m_page && m_page->mainFrame())
- m_page->mainFrame()->loader()->frameDetached();
- m_page.clear();
+ destoryPage(); // In case closePopup() was not called.
m_widgetClient = 0;
deref();
}
@@ -329,6 +338,9 @@
DOMWindowPagePopup::uninstall(m_page->mainFrame()->document()->domWindow());
}
m_closing = true;
+
+ destoryPage();
+
// m_widgetClient might be 0 because this widget might be already closed.
if (m_widgetClient) {
// closeWidgetSoon() will call this->close() later.
Modified: trunk/Source/WebKit/chromium/src/WebPagePopupImpl.h (136630 => 136631)
--- trunk/Source/WebKit/chromium/src/WebPagePopupImpl.h 2012-12-05 04:49:51 UTC (rev 136630)
+++ trunk/Source/WebKit/chromium/src/WebPagePopupImpl.h 2012-12-05 04:55:38 UTC (rev 136631)
@@ -87,6 +87,7 @@
explicit WebPagePopupImpl(WebWidgetClient*);
bool initPage();
+ void destoryPage();
WebWidgetClient* m_widgetClient;
WebRect m_windowRectInScreen;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
