[webkit-changes] [WebKit/WebKit] 78fca1: Mail sometimes crashes under -beginDraggingSession...

Wenson Hsieh noreply at github.com
Tue Sep 13 12:22:04 PDT 2022


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 78fca16e3ae8ef10f630dc7c2b44afe887f86c5c
      https://github.com/WebKit/WebKit/commit/78fca16e3ae8ef10f630dc7c2b44afe887f86c5c
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2022-09-13 (Tue, 13 Sep 2022)

  Changed paths:
    M Source/WebKit/UIProcess/mac/WebViewImpl.h
    M Source/WebKit/UIProcess/mac/WebViewImpl.mm
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
    M Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h
    M Tools/TestWebKitAPI/cocoa/TestWKWebView.h
    M Tools/TestWebKitAPI/cocoa/TestWKWebView.mm
    M Tools/TestWebKitAPI/mac/DragAndDropSimulatorMac.mm

  Log Message:
  -----------
  Mail sometimes crashes under -beginDraggingSessionWithItems: when dragging attachments in compose
https://bugs.webkit.org/show_bug.cgi?id=245142
rdar://99662145

Reviewed by Tim Horton.

This is a speculative fix for occasional crashes in Mail when dragging attachment elements, due to
the `NSEvent` passed into `-[WKWebView beginDraggingSessionWithItems:event:source:]` being nil,
when AppKit attempts to add this `NSEvent` as a value in a dictionary. While I wasn't able to
reproduce the crash locally, there are two reasons why the crash might be happening, from code
inspection:

1.  `-acceptsFirstMouse:` or `-shouldDelayWindowOrderingForEvent:` is called after `-mouseDown:`,
    but before the drag has actually begun. This causes us to clear out `m_lastMouseDownEvent`
    before initiating the drag session.

2. `-mouseUp:` is called right after the drag hysteresis has been exceeded in the web process (and
    we're initiating the drag in `DragController`), but before the drag session has actually begun
    in the UI process.

This patch mitigates both potential causes by adjusting the implementations of `acceptsFirstMouse`
and `shouldDelayWindowOrderingForEvent` to set `m_lastMouseDownEvent` back to its previous value,
rather than nil; additionally, if `m_lastMouseDownEvent` is already nil when attempting to start the
attachment drag session, cancel the drag session and bail instead of attempting to proceed with a
nil event.

* Source/WebKit/UIProcess/mac/WebViewImpl.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::WebViewImpl::acceptsFirstMouse):
(WebKit::WebViewImpl::shouldDelayWindowOrderingForEvent):
(WebKit::WebViewImpl::startDrag):
(WebKit::WebViewImpl::setLastMouseDownEvent):

Make this return the previous `m_lastMouseDownEvent`, so that we can restore it when the event is
only being set temporarily (i.e. in `acceptsFirstMouse` and `shouldDelayWindowOrderingForEvent`).

* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(TestWebKitAPI::TEST):
* Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h:
* Tools/TestWebKitAPI/cocoa/TestWKWebView.h:
* Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView acceptsFirstMouseAtPoint:]):

Add a helper method that wraps calls to `-acceptsFirstMouse:`.

* Tools/TestWebKitAPI/mac/DragAndDropSimulatorMac.mm:
(-[DragAndDropTestWKWebView beginDraggingSessionWithItems:event:source:]):
(-[DragAndDropSimulator runFrom:to:]):
(-[DragAndDropSimulator willBeginDraggingHandler]):
(-[DragAndDropSimulator setWillBeginDraggingHandler:]):

Add a mechanism to run some testing logic right before synthesizing mouse drag events, when
simulating drag and drop.

Canonical link: https://commits.webkit.org/254450@main




More information about the webkit-changes mailing list