Title: [114987] trunk/Source/WebKit/chromium
- Revision
- 114987
- Author
- tk...@chromium.org
- Date
- 2012-04-23 20:15:08 -0700 (Mon, 23 Apr 2012)
Log Message
Crash when the page with a calendar picker is scrolled
https://bugs.webkit.org/show_bug.cgi?id=84287
Reviewed by Hajime Morita.
Use the same ownership model as WebPopupMenuImpl's.
* src/WebPagePopupImpl.cpp:
(WebKit::WebPagePopupImpl::close):
Clear m_widgetClient to avoid furthur access to it. deref instead of delete.
(WebKit::WebPagePopupImpl::closePopup):
Do not call closeWidgetSoon() if close() was already called.
(WebKit::WebPagePopup::create):
Add a reference. Add explanation of the ownership.
* src/WebPagePopupImpl.h:
(WebPagePopupImpl): Make this RefCounted. Make the destuctor public.
* src/WebViewImpl.cpp:
(WebKit::WebViewImpl::WebViewImpl): No need to clear m_pagePopup explicitly.
(WebKit::WebViewImpl::openPagePopup): Need to use .get() because m_pagePopup is a RefPtr.
(WebKit::WebViewImpl::closePagePopup): ditto.
(WebKit::WebViewImpl::hidePopups): ditto.
* src/WebViewImpl.h: Make m_pagePopup a RefPtr.
Modified Paths
Diff
Modified: trunk/Source/WebKit/chromium/ChangeLog (114986 => 114987)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-04-24 02:11:07 UTC (rev 114986)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-04-24 03:15:08 UTC (rev 114987)
@@ -1,3 +1,28 @@
+2012-04-23 Kent Tamura <tk...@chromium.org>
+
+ Crash when the page with a calendar picker is scrolled
+ https://bugs.webkit.org/show_bug.cgi?id=84287
+
+ Reviewed by Hajime Morita.
+
+ Use the same ownership model as WebPopupMenuImpl's.
+
+ * src/WebPagePopupImpl.cpp:
+ (WebKit::WebPagePopupImpl::close):
+ Clear m_widgetClient to avoid furthur access to it. deref instead of delete.
+ (WebKit::WebPagePopupImpl::closePopup):
+ Do not call closeWidgetSoon() if close() was already called.
+ (WebKit::WebPagePopup::create):
+ Add a reference. Add explanation of the ownership.
+ * src/WebPagePopupImpl.h:
+ (WebPagePopupImpl): Make this RefCounted. Make the destuctor public.
+ * src/WebViewImpl.cpp:
+ (WebKit::WebViewImpl::WebViewImpl): No need to clear m_pagePopup explicitly.
+ (WebKit::WebViewImpl::openPagePopup): Need to use .get() because m_pagePopup is a RefPtr.
+ (WebKit::WebViewImpl::closePagePopup): ditto.
+ (WebKit::WebViewImpl::hidePopups): ditto.
+ * src/WebViewImpl.h: Make m_pagePopup a RefPtr.
+
2012-04-18 James Robinson <jam...@chromium.org>
[chromium] Use TextureLayerChromium for WebGL content instead of a dedicated layer type
Modified: trunk/Source/WebKit/chromium/src/WebPagePopupImpl.cpp (114986 => 114987)
--- trunk/Source/WebKit/chromium/src/WebPagePopupImpl.cpp 2012-04-24 02:11:07 UTC (rev 114986)
+++ trunk/Source/WebKit/chromium/src/WebPagePopupImpl.cpp 2012-04-24 03:15:08 UTC (rev 114987)
@@ -282,7 +282,8 @@
void WebPagePopupImpl::close()
{
m_page.clear();
- delete this;
+ m_widgetClient = 0;
+ deref();
}
void WebPagePopupImpl::closePopup()
@@ -292,8 +293,11 @@
m_page->mainFrame()->loader()->stopAllLoaders();
m_page->mainFrame()->loader()->stopLoading(UnloadEventPolicyNone);
}
- // closeWidgetSoon() will call this->close() later.
- m_widgetClient->closeWidgetSoon();
+ // m_widgetClient might be 0 because this widget might be already closed.
+ if (m_widgetClient) {
+ // closeWidgetSoon() will call this->close() later.
+ m_widgetClient->closeWidgetSoon();
+ }
m_popupClient->didClosePopup();
}
@@ -307,7 +311,13 @@
#if ENABLE(PAGE_POPUP)
if (!client)
CRASH();
- return new WebPagePopupImpl(client);
+ // A WebPagePopupImpl 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
+ // WebPagePopupImpl to close.
+ // We need them because the closing operation is asynchronous and the widget
+ // can be closed while the WebViewImpl is unaware of it.
+ return adoptRef(new WebPagePopupImpl(client)).leakRef();
#else
UNUSED_PARAM(client);
return 0;
Modified: trunk/Source/WebKit/chromium/src/WebPagePopupImpl.h (114986 => 114987)
--- trunk/Source/WebKit/chromium/src/WebPagePopupImpl.h 2012-04-24 02:11:07 UTC (rev 114986)
+++ trunk/Source/WebKit/chromium/src/WebPagePopupImpl.h 2012-04-24 03:15:08 UTC (rev 114987)
@@ -37,6 +37,7 @@
#include "PageWidgetDelegate.h"
#include "WebPagePopup.h"
#include <wtf/OwnPtr.h>
+#include <wtf/RefCounted.h>
namespace WebCore {
class Page;
@@ -51,11 +52,13 @@
class WebPagePopupImpl : public WebPagePopup,
public PageWidgetEventHandler,
- public WebCore::PagePopup {
+ public WebCore::PagePopup,
+ public RefCounted<WebPagePopupImpl> {
WTF_MAKE_NONCOPYABLE(WebPagePopupImpl);
WTF_MAKE_FAST_ALLOCATED;
public:
+ virtual ~WebPagePopupImpl();
bool init(WebViewImpl*, WebCore::PagePopupClient*, const WebCore::IntRect& originBoundsInRootView);
bool handleKeyEvent(const WebCore::PlatformKeyboardEvent&);
void closePopup();
@@ -82,7 +85,6 @@
#endif
explicit WebPagePopupImpl(WebWidgetClient*);
- virtual ~WebPagePopupImpl();
bool initPage();
WebWidgetClient* m_widgetClient;
Modified: trunk/Source/WebKit/chromium/src/WebViewImpl.cpp (114986 => 114987)
--- trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2012-04-24 02:11:07 UTC (rev 114986)
+++ trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2012-04-24 03:15:08 UTC (rev 114987)
@@ -372,9 +372,6 @@
, m_dragOperation(WebDragOperationNone)
, m_autofillPopupShowing(false)
, m_autofillPopup(0)
-#if ENABLE(PAGE_POPUP)
- , m_pagePopup(0)
-#endif
, m_isTransparent(false)
, m_tabsToLinks(false)
, m_dragScrollTimer(adoptPtr(new DragScrollTimer))
@@ -1215,15 +1212,15 @@
if (Frame* frame = focusedWebCoreFrame())
frame->selection()->setCaretVisible(false);
- return m_pagePopup;
+ return m_pagePopup.get();
}
void WebViewImpl::closePagePopup(PagePopup* popup)
{
ASSERT(popup);
WebPagePopupImpl* popupImpl = static_cast<WebPagePopupImpl*>(popup);
- ASSERT(m_pagePopup == popupImpl);
- if (m_pagePopup != popupImpl)
+ ASSERT(m_pagePopup.get() == popupImpl);
+ if (m_pagePopup.get() != popupImpl)
return;
m_pagePopup->closePopup();
m_pagePopup = 0;
@@ -2797,7 +2794,7 @@
hideAutofillPopup();
#if ENABLE(PAGE_POPUP)
if (m_pagePopup)
- closePagePopup(m_pagePopup);
+ closePagePopup(m_pagePopup.get());
#endif
}
Modified: trunk/Source/WebKit/chromium/src/WebViewImpl.h (114986 => 114987)
--- trunk/Source/WebKit/chromium/src/WebViewImpl.h 2012-04-24 02:11:07 UTC (rev 114986)
+++ trunk/Source/WebKit/chromium/src/WebViewImpl.h 2012-04-24 03:15:08 UTC (rev 114987)
@@ -711,7 +711,7 @@
#if ENABLE(PAGE_POPUP)
// The popup associated with an input element.
- WebPagePopupImpl* m_pagePopup;
+ RefPtr<WebPagePopupImpl> m_pagePopup;
#endif
OwnPtr<WebDevToolsAgentPrivate> m_devToolsAgent;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
