- Revision
- 91266
- Author
- [email protected]
- Date
- 2011-07-19 10:14:47 -0700 (Tue, 19 Jul 2011)
Log Message
Speculative fix for: Crash under WebPage::platformDragEnded when dragging on Mac
https://bugs.webkit.org/show_bug.cgi?id=64766
<rdar://problem/9548174>
Reviewed by Enrica Casucci.
I was unable to reproduce this bug, but Darin Adler and I discussed the probable issue. When starting the drag, we create
a WKPasteboardFilePromiseOwner, and a WKPasteboardOwner. When the drag is concluded, we call a method on the WKPasteboardFilePromiseOwner
which uses the WKPasteboardOwner. However, we are not guaranteeing that the WKPasteboardOwner will be around when the
WKPasteboardFilePromiseOwner method is called.
The fix is to retain both the WKPasteboardFilePromiseOwner and the WKPasteboardOwner that we need, making sure that we are keeping
both objects alive.
This patch also uses r91222 to replace WebPage::platformDragEnded, so WebPage doesn't need to know about the drag source.
* WebProcess/WebCoreSupport/WebDragClient.cpp:
(WebKit::WebDragClient::dragEnded): Add a non-Mac stub method, since the Mac is the only platform that does something here.
* WebProcess/WebCoreSupport/WebDragClient.h:
* WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:
(WebKit::WebDragClient::declareAndWriteDragImage): Use member variables instead of local variables.
(WebKit::WebDragClient::dragEnded): Move code from WebPageMac::platformDragEnded to here, and clear both member variables.
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::dragEnded): Don't call platformDragEnded anymore. WebCore::DragController::dragEnded calls WebDragClient::dragEnded,
which does the same thing.
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/mac/WebPageMac.mm: Remove platformDragEnded.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (91265 => 91266)
--- trunk/Source/WebKit2/ChangeLog 2011-07-19 17:08:30 UTC (rev 91265)
+++ trunk/Source/WebKit2/ChangeLog 2011-07-19 17:14:47 UTC (rev 91266)
@@ -1,3 +1,33 @@
+2011-07-18 Brian Weinstein <[email protected]>
+
+ Speculative fix for: Crash under WebPage::platformDragEnded when dragging on Mac
+ https://bugs.webkit.org/show_bug.cgi?id=64766
+ <rdar://problem/9548174>
+
+ Reviewed by Enrica Casucci.
+
+ I was unable to reproduce this bug, but Darin Adler and I discussed the probable issue. When starting the drag, we create
+ a WKPasteboardFilePromiseOwner, and a WKPasteboardOwner. When the drag is concluded, we call a method on the WKPasteboardFilePromiseOwner
+ which uses the WKPasteboardOwner. However, we are not guaranteeing that the WKPasteboardOwner will be around when the
+ WKPasteboardFilePromiseOwner method is called.
+
+ The fix is to retain both the WKPasteboardFilePromiseOwner and the WKPasteboardOwner that we need, making sure that we are keeping
+ both objects alive.
+
+ This patch also uses r91222 to replace WebPage::platformDragEnded, so WebPage doesn't need to know about the drag source.
+
+ * WebProcess/WebCoreSupport/WebDragClient.cpp:
+ (WebKit::WebDragClient::dragEnded): Add a non-Mac stub method, since the Mac is the only platform that does something here.
+ * WebProcess/WebCoreSupport/WebDragClient.h:
+ * WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:
+ (WebKit::WebDragClient::declareAndWriteDragImage): Use member variables instead of local variables.
+ (WebKit::WebDragClient::dragEnded): Move code from WebPageMac::platformDragEnded to here, and clear both member variables.
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::dragEnded): Don't call platformDragEnded anymore. WebCore::DragController::dragEnded calls WebDragClient::dragEnded,
+ which does the same thing.
+ * WebProcess/WebPage/WebPage.h:
+ * WebProcess/WebPage/mac/WebPageMac.mm: Remove platformDragEnded.
+
2011-07-18 Alexis Menard <[email protected]>
[Qt][WK2] Make QDesktopWebView::navigationAction method usable in QML.
Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebDragClient.cpp (91265 => 91266)
--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebDragClient.cpp 2011-07-19 17:08:30 UTC (rev 91265)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebDragClient.cpp 2011-07-19 17:14:47 UTC (rev 91266)
@@ -58,6 +58,12 @@
}
#endif
+#if !PLATFORM(MAC)
+void WebDragClient::dragEnded()
+{
+}
+#endif
+
void WebDragClient::dragControllerDestroyed()
{
delete this;
Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebDragClient.h (91265 => 91266)
--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebDragClient.h 2011-07-19 17:08:30 UTC (rev 91265)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebDragClient.h 2011-07-19 17:14:47 UTC (rev 91266)
@@ -28,6 +28,16 @@
#include <WebCore/DragClient.h>
+#if PLATFORM(MAC)
+#ifdef __OBJC__
+@class WKPasteboardFilePromiseOwner;
+@class WKPasteboardOwner;
+#else
+class WKPasteboardFilePromiseOwner;
+class WKPasteboardOwner;
+#endif
+#endif
+
namespace WebKit {
class WebPage;
@@ -50,9 +60,17 @@
#if PLATFORM(MAC)
virtual void declareAndWriteDragImage(NSPasteboard*, DOMElement*, NSURL*, NSString*, WebCore::Frame*);
#endif
+
+ virtual void dragEnded();
+
virtual void dragControllerDestroyed();
WebPage* m_page;
+
+#if PLATFORM(MAC)
+ RetainPtr<WKPasteboardFilePromiseOwner> m_filePromiseOwner;
+ RetainPtr<WKPasteboardOwner> m_pasteboardOwner;
+#endif
};
} // namespace WebKit
Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm (91265 => 91266)
--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm 2011-07-19 17:08:30 UTC (rev 91265)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm 2011-07-19 17:14:47 UTC (rev 91266)
@@ -148,16 +148,14 @@
RetainPtr<NSMutableArray> types(AdoptNS, [[NSMutableArray alloc] initWithObjects:NSFilesPromisePboardType, nil]);
[types.get() addObjectsFromArray:archive ? PasteboardTypes::forImagesWithArchive() : PasteboardTypes::forImages()];
- RetainPtr<WKPasteboardOwner> pasteboardOwner(AdoptNS, [[WKPasteboardOwner alloc] initWithImage:image]);
+ m_pasteboardOwner.adoptNS([[WKPasteboardOwner alloc] initWithImage:image]);
+ m_filePromiseOwner.adoptNS([(WKPasteboardFilePromiseOwner *)[WKPasteboardFilePromiseOwner alloc] initWithSource:m_pasteboardOwner.get()]);
- RetainPtr<WKPasteboardFilePromiseOwner> filePromiseOwner(AdoptNS, [(WKPasteboardFilePromiseOwner *)[WKPasteboardFilePromiseOwner alloc] initWithSource:pasteboardOwner.get()]);
- m_page->setDragSource(filePromiseOwner.get());
+ [pasteboard declareTypes:types.get() owner:m_pasteboardOwner.leakRef()];
- [pasteboard declareTypes:types.get() owner:pasteboardOwner.leakRef()];
-
[pasteboard setPropertyList:[NSArray arrayWithObject:extension] forType:NSFilesPromisePboardType];
- [filePromiseOwner.get() setTypes:[pasteboard propertyListForType:NSFilesPromisePboardType] onPasteboard:pasteboard];
+ [m_filePromiseOwner.get() setTypes:[pasteboard propertyListForType:NSFilesPromisePboardType] onPasteboard:pasteboard];
[URL writeToPasteboard:pasteboard];
@@ -173,6 +171,16 @@
[pasteboard setData:(NSData *)archive->rawDataRepresentation().get() forType:PasteboardTypes::WebArchivePboardType];
}
+void WebDragClient::dragEnded()
+{
+ // The drag source we care about here is NSFilePromiseDragSource, which doesn't look at
+ // the arguments. It's OK to just pass arbitrary constant values, so we just pass all zeroes.
+ [m_filePromiseOwner.get() draggedImage:nil endedAt:NSZeroPoint operation:NSDragOperationNone];
+
+ m_pasteboardOwner = nullptr;
+ m_filePromiseOwner = nullptr;
+}
+
} // namespace WebKit
@implementation WKPasteboardFilePromiseOwner
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp (91265 => 91266)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp 2011-07-19 17:08:30 UTC (rev 91265)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp 2011-07-19 17:14:47 UTC (rev 91266)
@@ -1796,8 +1796,7 @@
{
IntPoint adjustedClientPosition(clientPosition.x() + m_page->dragController()->dragOffset().x(), clientPosition.y() + m_page->dragController()->dragOffset().y());
IntPoint adjustedGlobalPosition(globalPosition.x() + m_page->dragController()->dragOffset().x(), globalPosition.y() + m_page->dragController()->dragOffset().y());
-
- platformDragEnded();
+
m_page->dragController()->dragEnded();
FrameView* view = m_page->mainFrame()->view();
if (!view)
@@ -2410,12 +2409,6 @@
m_page->setMemoryCacheClientCallsEnabled(memoryCacheMessagesEnabled);
}
-#if !PLATFORM(MAC)
-void WebPage::platformDragEnded()
-{
-}
-#endif
-
bool WebPage::canHandleRequest(const WebCore::ResourceRequest& request)
{
if (SchemeRegistry::shouldLoadURLSchemeAsEmptyDocument(request.url().protocol()))
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h (91265 => 91266)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h 2011-07-19 17:08:30 UTC (rev 91265)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h 2011-07-19 17:14:47 UTC (rev 91266)
@@ -418,10 +418,6 @@
void unmarkAllMisspellings();
void unmarkAllBadGrammar();
-#if PLATFORM(MAC)
- void setDragSource(NSObject *);
-#endif
-
#if PLATFORM(MAC) && !defined(BUILDING_ON_SNOW_LEOPARD)
void handleCorrectionPanelResult(const String&);
#endif
@@ -566,8 +562,6 @@
void didSelectItemFromActiveContextMenu(const WebContextMenuItemData&);
#endif
- void platformDragEnded();
-
void setCanStartMediaTimerFired();
static bool platformCanHandleRequest(const WebCore::ResourceRequest&);
@@ -612,8 +606,6 @@
RetainPtr<AccessibilityWebPageObject> m_mockAccessibilityElement;
- RetainPtr<NSObject> m_dragSource;
-
WebCore::KeyboardEvent* m_keyboardEventBeingInterpreted;
#elif PLATFORM(WIN)
Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm (91265 => 91266)
--- trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm 2011-07-19 17:08:30 UTC (rev 91265)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm 2011-07-19 17:14:47 UTC (rev 91266)
@@ -679,21 +679,6 @@
return request.url().protocolIs("applewebdata");
}
-void WebPage::setDragSource(NSObject *dragSource)
-{
- m_dragSource = dragSource;
-}
-
-void WebPage::platformDragEnded()
-{
- // The draggedImage method releases its responder; we retain here to balance that.
- [m_dragSource.get() retain];
- // The drag source we care about here is NSFilePromiseDragSource, which doesn't look at
- // the arguments. It's OK to just pass arbitrary constant values, so we just pass all zeroes.
- [m_dragSource.get() draggedImage:nil endedAt:NSZeroPoint operation:NSDragOperationNone];
- m_dragSource = nullptr;
-}
-
void WebPage::shouldDelayWindowOrderingEvent(const WebKit::WebMouseEvent& event, bool& result)
{
result = false;
