Changeset 91266 in webkit


Ignore:
Timestamp:
Jul 19, 2011 10:14:47 AM (13 years ago)
Author:
bweinstein@apple.com
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.
Location:
trunk/Source/WebKit2
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit2/ChangeLog

    r91248 r91266  
     12011-07-18  Brian Weinstein  <bweinstein@apple.com>
     2
     3        Speculative fix for: Crash under WebPage::platformDragEnded when dragging on Mac
     4        https://bugs.webkit.org/show_bug.cgi?id=64766
     5        <rdar://problem/9548174>
     6
     7        Reviewed by Enrica Casucci.
     8
     9        I was unable to reproduce this bug, but Darin Adler and I discussed the probable issue. When starting the drag, we create
     10        a WKPasteboardFilePromiseOwner, and a WKPasteboardOwner. When the drag is concluded, we call a method on the WKPasteboardFilePromiseOwner
     11        which uses the WKPasteboardOwner. However, we are not guaranteeing that the WKPasteboardOwner will be around when the
     12        WKPasteboardFilePromiseOwner method is called.
     13       
     14        The fix is to retain both the WKPasteboardFilePromiseOwner and the WKPasteboardOwner that we need, making sure that we are keeping
     15        both objects alive.
     16       
     17        This patch also uses r91222 to replace WebPage::platformDragEnded, so WebPage doesn't need to know about the drag source.
     18
     19        * WebProcess/WebCoreSupport/WebDragClient.cpp:
     20        (WebKit::WebDragClient::dragEnded): Add a non-Mac stub method, since the Mac is the only platform that does something here.
     21        * WebProcess/WebCoreSupport/WebDragClient.h:
     22        * WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:
     23        (WebKit::WebDragClient::declareAndWriteDragImage): Use member variables instead of local variables.
     24        (WebKit::WebDragClient::dragEnded): Move code from WebPageMac::platformDragEnded to here, and clear both member variables.
     25        * WebProcess/WebPage/WebPage.cpp:
     26        (WebKit::WebPage::dragEnded): Don't call platformDragEnded anymore. WebCore::DragController::dragEnded calls WebDragClient::dragEnded,
     27            which does the same thing.
     28        * WebProcess/WebPage/WebPage.h:
     29        * WebProcess/WebPage/mac/WebPageMac.mm: Remove platformDragEnded.
     30
    1312011-07-18  Alexis Menard  <alexis.menard@openbossa.org>
    232

  • trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebDragClient.cpp

    r89582 r91266  
    5959#endif
    6060
     61#if !PLATFORM(MAC)
     62void WebDragClient::dragEnded()
     63{
     64}
     65#endif
     66
    6167void WebDragClient::dragControllerDestroyed()
    6268{

  • trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebDragClient.h

    r77870 r91266  
    2929#include <WebCore/DragClient.h>
    3030
     31#if PLATFORM(MAC)
     32#ifdef __OBJC__
     33@class WKPasteboardFilePromiseOwner;
     34@class WKPasteboardOwner;
     35#else
     36class WKPasteboardFilePromiseOwner;
     37class WKPasteboardOwner;
     38#endif
     39#endif
     40
    3141namespace WebKit {
    3242
    … …  
    5161    virtual void declareAndWriteDragImage(NSPasteboard*, DOMElement*, NSURL*, NSString*, WebCore::Frame*);
    5262#endif
     63
     64    virtual void dragEnded();
     65
    5366    virtual void dragControllerDestroyed();
    5467
    5568    WebPage* m_page;
     69   
     70#if PLATFORM(MAC)
     71    RetainPtr<WKPasteboardFilePromiseOwner> m_filePromiseOwner;
     72    RetainPtr<WKPasteboardOwner> m_pasteboardOwner;
     73#endif
    5674};
    5775

  • trunk/Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm

    r86451 r91266  
    149149    [types.get() addObjectsFromArray:archive ? PasteboardTypes::forImagesWithArchive() : PasteboardTypes::forImages()];
    150150
    151     RetainPtr<WKPasteboardOwner> pasteboardOwner(AdoptNS, [[WKPasteboardOwner alloc] initWithImage:image]);
    152 
    153     RetainPtr<WKPasteboardFilePromiseOwner> filePromiseOwner(AdoptNS, [(WKPasteboardFilePromiseOwner *)[WKPasteboardFilePromiseOwner alloc] initWithSource:pasteboardOwner.get()]);
    154     m_page->setDragSource(filePromiseOwner.get());
    155 
    156     [pasteboard declareTypes:types.get() owner:pasteboardOwner.leakRef()];   
     151    m_pasteboardOwner.adoptNS([[WKPasteboardOwner alloc] initWithImage:image]);
     152    m_filePromiseOwner.adoptNS([(WKPasteboardFilePromiseOwner *)[WKPasteboardFilePromiseOwner alloc] initWithSource:m_pasteboardOwner.get()]);
     153
     154    [pasteboard declareTypes:types.get() owner:m_pasteboardOwner.leakRef()];   
    157155
    158156    [pasteboard setPropertyList:[NSArray arrayWithObject:extension] forType:NSFilesPromisePboardType];
    159157
    160     [filePromiseOwner.get() setTypes:[pasteboard propertyListForType:NSFilesPromisePboardType] onPasteboard:pasteboard];
     158    [m_filePromiseOwner.get() setTypes:[pasteboard propertyListForType:NSFilesPromisePboardType] onPasteboard:pasteboard];
    161159
    162160    [URL writeToPasteboard:pasteboard];
    … …  
    172170    if (archive)
    173171        [pasteboard setData:(NSData *)archive->rawDataRepresentation().get() forType:PasteboardTypes::WebArchivePboardType];
     172}
     173
     174void WebDragClient::dragEnded()
     175{
     176    // The drag source we care about here is NSFilePromiseDragSource, which doesn't look at
     177    // the arguments. It's OK to just pass arbitrary constant values, so we just pass all zeroes.
     178    [m_filePromiseOwner.get() draggedImage:nil endedAt:NSZeroPoint operation:NSDragOperationNone];
     179   
     180    m_pasteboardOwner = nullptr;
     181    m_filePromiseOwner = nullptr;
    174182}
    175183

  • trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

    r91232 r91266  
    17971797    IntPoint adjustedClientPosition(clientPosition.x() + m_page->dragController()->dragOffset().x(), clientPosition.y() + m_page->dragController()->dragOffset().y());
    17981798    IntPoint adjustedGlobalPosition(globalPosition.x() + m_page->dragController()->dragOffset().x(), globalPosition.y() + m_page->dragController()->dragOffset().y());
    1799    
    1800     platformDragEnded();
     1799
    18011800    m_page->dragController()->dragEnded();
    18021801    FrameView* view = m_page->mainFrame()->view();
    … …  
    24112410}
    24122411
    2413 #if !PLATFORM(MAC)
    2414 void WebPage::platformDragEnded()
    2415 {
    2416 }
    2417 #endif
    2418 
    24192412bool WebPage::canHandleRequest(const WebCore::ResourceRequest& request)
    24202413{

  • trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h

    r91064 r91266  
    419419    void unmarkAllBadGrammar();
    420420
    421 #if PLATFORM(MAC)
    422     void setDragSource(NSObject *);
    423 #endif
    424 
    425421#if PLATFORM(MAC) && !defined(BUILDING_ON_SNOW_LEOPARD)
    426422    void handleCorrectionPanelResult(const String&);
    … …  
    567563#endif
    568564
    569     void platformDragEnded();
    570 
    571565    void setCanStartMediaTimerFired();
    572566
    … …  
    612606
    613607    RetainPtr<AccessibilityWebPageObject> m_mockAccessibilityElement;
    614 
    615     RetainPtr<NSObject> m_dragSource;
    616608
    617609    WebCore::KeyboardEvent* m_keyboardEventBeingInterpreted;

  • trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm

    r88667 r91266  
    680680}
    681681
    682 void WebPage::setDragSource(NSObject *dragSource)
    683 {
    684     m_dragSource = dragSource;
    685 }
    686 
    687 void WebPage::platformDragEnded()
    688 {
    689     // The draggedImage method releases its responder; we retain here to balance that.
    690     [m_dragSource.get() retain];
    691     // The drag source we care about here is NSFilePromiseDragSource, which doesn't look at
    692     // the arguments. It's OK to just pass arbitrary constant values, so we just pass all zeroes.
    693     [m_dragSource.get() draggedImage:nil endedAt:NSZeroPoint operation:NSDragOperationNone];
    694     m_dragSource = nullptr;
    695 }
    696 
    697682void WebPage::shouldDelayWindowOrderingEvent(const WebKit::WebMouseEvent& event, bool& result)
    698683{
Note: See TracChangeset for help on using the changeset viewer.